fix(frontend): render markdown tables in post bodies - #389
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@claude Please review exact current head 0d8b6f4 after the current-head repair. It now requires a Markdown separator row before pipe tables, applies table rendering in the persisted-structure path, and gives rendered tables stable React keys. Frontend lint, 149 tests, build, and diff-check pass. Review only this exact head for parsing false positives, buyer rendering, accessibility, and stack boundaries; report actionable findings. |
|
@claude Re-review exact current head 7b551bf after the normal stack update. PR #389 now includes current PR #388 head ee5882a through a regular merge commit and retains the Markdown table hardening. Verify parent-child boundaries and frontend behavior at this exact head; report actionable findings only. |
|
@opencode-agent @cwl-noema-review Please re-review exact head |
|
@claude Please review exact current head 3880376. A concurrent remote commit now preserves separator-free pipe rows only for persisted OCR image evidence while Markdown source text still requires an explicit separator row; stable React keys and persisted-structure rendering remain fixed. Frontend lint, 150 tests, build, and diff-check pass. Review parser boundaries, buyer display, and accessibility at this exact head; report actionable findings. |
Exact-head verificationCurrent head:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… into fix/pr389-preserve-ocr-pipe-table
|
@opencode-agent @cwl-noema-review Please review restacked exact head |
|
@claude Please review exact current head 0c2f706. The branch now includes current #388 head 1b680a2 via a normal merge, retaining Markdown-table hardening and separator-free OCR support. Frontend lint, 152 tests, build, and diff-check pass. Review exact parent-child boundaries and buyer-visible parser behavior; report actionable findings only. |
|
@opencode-agent @cwl-noema-review Please re-review exact head |
Exact-head verificationCurrent head:
No force push or bypass was used. Hosted Checks and independent approval remain required. |
| if (trimmed.includes("|")) { | ||
| const cells = trimmed.replace(/^\|/, "").replace(/\|$/, "").split("|"); | ||
| if (cells.length >= 2 && cells.some((cell) => cell.trim())) { | ||
| pipeTableRows.push(line); | ||
| continue; | ||
| } | ||
| } | ||
| if (pipeTableRows.length > 0) flushPipeTableRows(); |
There was a problem hiding this comment.
🔍 Pipe-bearing list items merge into one paragraph
A line containing | with two or more non-empty cells is accumulated as a table-candidate row and continues, bypassing the LIST_ITEM_START flush at postBodyDisplay.ts. When these lines do not form a table (no separator row), flushPipeTableRows appends them back to lines (postBodyDisplay.ts), joining them into one paragraph. Consecutive plain-text list items whose text contains a pipe (e.g. - Owner | Alice / - Reviewer | Bob on single-newline lines) previously stayed separate via the list-item flush; they now collapse into a single run-on paragraph. No test covers this.
Was this helpful? React with 👍 or 👎 to provide feedback.
… into codex/pr389-stray-pipe
… into fix/pr389-preserve-ocr-pipe-table
|
@opencode-agent @cwl-noema-review 재검토 요청: exact head 788bacd. 최신 PR #388 부모와 동시 wrapper-footnote 수정까지 보존해 재적층했고 frontend lint, 155 tests, production build, Storybook build가 통과했습니다. |
778c5df
into
feat/post-body-footnote-display
github-project-automation
Bot
moved this from In Progress
to Done
in naruon Platform Roadmap
| const flushPipeTableRows = () => { | ||
| const hasSeparator = pipeTableRows.some((row) => { | ||
| const cells = row.trim().replace(/^\|/, "").replace(/\|$/, "").split("|"); | ||
| return cells.length >= 2 && cells.every((cell) => /^\s*:?-{3,}:?\s*$/.test(cell)); | ||
| }); | ||
| if (pipeTableRows.length >= 2 && hasSeparator) { | ||
| flush(); | ||
| paragraphs.push(pipeTableRows.map((row) => row.trim()).join("\n")); | ||
| } else { | ||
| lines.push(...pipeTableRows); | ||
| } | ||
| pipeTableRows = []; | ||
| }; |
There was a problem hiding this comment.
📝 Info: Separator accepted anywhere when grouping, required at row 1 when rendering
flushPipeTableRows groups pipe lines into a table paragraph when any row is a separator, but parsePipeDelimitedTable (frontend/src/PostBody.tsx:18) only renders when the separator is at row index 1. A table whose separator sits elsewhere is grouped, fails to parse, then renders as one <p> with newlines flattened to spaces. Affects only malformed Markdown tables.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@claude Please review the exact PR head 788bacd (the PR ref is distinct from the moving branch tip; verify the PR ref directly). Exact-head local verification passed 21 focused backend tests, 155 frontend tests, lint, production build, and diff-check. Verify current hosted Checks and stack base before a formal review verdict. |
* feat: persist and explain Event Lineage channel evidence Reconstruct already computed per-channel scores, but live Event Lineage collapsed each edge to a fused score. Persist the active signals beside each edge, return them on GET /api/lineage, and disclose exact inferred values in the Buyer DAG. * test(schema): retain project event migration fixture * fix(frontend): keep admin panel behind authentication * test: apply lineage evidence migration to API fixtures * feat: route rebuild adjudication through orchestrator * fix: preserve adjudication during PostgreSQL imports * fix: order persisted lineage weights deterministically * docs: record orchestrated lineage rebuild policy * fix(frontend): preserve structured footnote roles * fix(lineage): budget signal rounding tolerance * fix(frontend): close HTML footnote containers reliably * fix: hide empty footnote markers * fix(lineage): offload synchronous reconstruction * perf(lineage): bound channel evidence reads * fix(frontend): bound footnote and anchor tag detection * fix: close rebuild transaction before reconstruction * fix(frontend): recognize wrapped footnote lists * fix: release lineage pool during reconstruction * fix(frontend): preserve structured footnote roles (#388) * fix(frontend): preserve structured footnote roles * fix(frontend): close HTML footnote containers reliably * fix: hide empty footnote markers * fix(frontend): bound footnote and anchor tag detection * fix(frontend): recognize wrapped footnote lists * fix(frontend): render markdown tables in post bodies (#389) * fix(frontend): render markdown tables in post bodies * fix(frontend): harden markdown table rendering * fix(frontend): preserve separator-free OCR tables * fix(frontend): confirm markdown tables before splitting prose * fix: make lineage evidence disclosure interactive * fix(frontend): preserve nested list indentation (#391) * fix(frontend): preserve nested list indentation * fix(frontend): indent block children in nested lists * fix(db): make tenant settings migration replay-safe * fix: bound live lineage adjudication work * chore: nudge CI re-review (opencode-agent's prior REQUEST_CHANGES was against a transient coverage-evidence flake on this same head; later reruns of the same head passed, but opencode-review only posts once per head SHA) * fix: guard corpus lineage rebuild against orchestrator failures; drop Buyer wording - Wrap rebuild_lineage_from_pool() in main.py's /api/lineage/rebuild in the same except (HttpClientError, OSError) -> HTTPException(503) pattern used at this file's other orchestrator call sites, so a transient hiccup during a corpus-wide (up to 5,000-call) rebuild degrades cleanly instead of discarding the whole reconstruction as a raw 500. Adds a focused endpoint test covering the new 503 path. - Reword the new ADR 0124 prose and CHANGELOG/CHANGELOG.d entries added by this PR to say "reader" / "Event Lineage DAG" instead of "buyer"/"Buyer", so this PR doesn't reintroduce naming PR #474 (ADR 0119) is retiring. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14 * fix: allocate migration 0174 for lineage signals * fix: close lineage evidence review gaps * Batch Event Lineage persistence writes * fix(frontend): restore shared OIDC return-URL helpers on login The login button had regressed to a raw window.location.pathname + window.location.search concat for the OIDC state.returnUrl, dropping the hash fragment and the isSafeReturnUrl validation that returnUrlFromLocation() already provides, and never called rememberOidcReturnUrl() to persist a storage fallback for restoreOidcReturnUrl() in main.tsx's onSigninCallback. Use the shared helpers again, matching the existing recurring-bug pattern already fixed elsewhere in this stack (the admin-panel-on-unauthenticated-branch half of the same class of regression). * fix: reconcile merged accessibility roles, script normalization, and migration-replay tests Post-merge fixes surfaced by full test verification after merging origin/main into this branch: - LineageDag's SVG now uses role="group" (this PR's own accessibility fix for the interactive per-edge evidence buttons), superseding main's older role="img". Updated the tests that still queried role="img" for the lineage graph, and switched the ambiguous getAllByRole("group") assertions to precise `svg[role="group"]` queries since <details> (added by this PR's channel-evidence panel) also carries an implicit ARIA group role. - postBodyDisplay.ts: adopted main's normalizeScriptText for <sup>/<sub> handling (real Unicode superscripts, later re-rendered by splitScriptRuns) instead of this PR's older ad hoc "^N" caret regex, and dropped the bare-marker FOOTNOTE_START heuristic main had already removed as a false-positive source (a bullet list starting with "*" was being misread as a footnote). Restored the FOOTNOTE_START constant only where still needed transitively, then removed it entirely once isMarkedFootnote (this PR's own container-aware footnote detection) proved sufficient on its own. Updated the one test still asserting the old "^1" caret text. - migrate.sh's replay gate: this PR's test asserted the old explicit per-file allowlist main had already replaced with ADR 0166's general four-digit filename pattern. Updated the assertion to check the new mechanism covers 0103/0174 without individual entries. - scripts/import_postgresql_posts.py's rebuild_lineage stub now accepts the llm= keyword this PR's own call site passes. - lineage_persistence.py: added the missing docstring on the nested sort_key helper the AST docstring-coverage audit flagged. Verified: backend `uv run pytest -q` 931 passed / 16 skipped / 0 failed (0:08:37); frontend `pnpm run lint`, `pnpm run build`, and `pnpm exec vitest run` (245 passed / 245). * fix: resolve post-merge test breakage from combined LineageDag changes - LineageDag.test.tsx declared a module-level `graph` const twice (once for the channel-evidence tests, once for the hit-target tests), which the line-based merge could not detect since both additions landed in non-overlapping hunks. Rename the second to `nodeHitTargetGraph`. - The merged LineageDag now keeps the lineage svg's `role="group"` (needed because its edges/nodes are interactive button-role descendants), so the mobile-scroll viewport test's `getByRole("img", ...)` query no longer matches; update it to `getByRole("group", ...)`. * fix(lineage): persist estimated fusion weights * fix(lineage): load weights for the active channel set * fix: align lineage weights with budgeted channels * fix: fail closed on adjudication errors * fix(lineage): wire the adjudication client into corpus-wide rebuild Issue #289: rebuild_lineage accepted no adjudication client, so the optional LLM channel never contributed on the corpus-wide path. Add an optional adjudication_client parameter: an available client adds llm to the active-channel set (failing closed until a four-channel estimate exists per ADR 0200) and reaches reconstruct as the reasoning channel; a missing or unavailable client keeps the three-channel path. The POST /api/lineage/rebuild endpoint now passes _adjudication_client(). Unavailable clients are normalized to None at this boundary so reconstruct receives a single canonical unavailable value. * fix(ask): retain lineage reconstruction profile * fix(lineage): preserve worker retries and parser edge cases * fix(ui): stop WorkspaceCalendar's fail-closed placeholder announcing as role=status Its resolved empty/unavailable state carried role="status" like sibling panels' transient loading text does, so mounting it inside the Board's collapsed Advanced Review Tools details collided with every other status region on the page (4 failing App.test.tsx assertions). RankingsPanel's own resolved placeholders carry no ARIA role for the same reason -- only the "Loading..." state announces. * fix(measurement): reject duplicated lineage channels * fix(lineage): name connection direction correctly * fix(lineage): name follows direction correctly --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: seonghobae <seonghobae@users.noreply.github.com>
1 participant
Summary
Verification
pnpm run test -- --no-file-parallelism --maxWorkers=1(147 passed)pnpm run lintpnpm run buildgit diff --checkThis PR is stacked on #388 and must be revalidated after its parent changes.