fix(frontend): preserve structured footnote roles - #388
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 |
|
@claude Please review exact current head |
|
Fixed at 81cd12f. Root cause: closing HTML footnote tags do not repeat the class that identifies the container, so the old depth counter leaked the footnote role into later ordinary paragraphs. The parser now tracks matching open tags, including nested lists and OOXML containers, and decrements only when the corresponding container closes. Evidence: frontend test suite 145 passed, TypeScript/Vite build passed, oxlint passed, and the focused post-body parser test passed (14 tests). |
|
@claude Please review exact current head ee5882a after the normal stack update. PR #388 now includes current parent head 4bf0613 through a regular merge commit, while retaining the footnote-container and empty-marker fixes. Review the parent-child boundary, frontend behavior, and any merge-conflict or regression risk at this exact head; report actionable findings with file and line evidence. |
|
@opencode-agent @cwl-noema-review Please re-review exact head |
|
@claude Please review exact current head 1b680a2. A concurrent repair now bounds footnote detection to class/role attributes and fixes anchor-tag name matching (so abbr is not stripped), with two regression tests. The current branch retains the parent-stack merge and prior footnote fixes; frontend lint, 148 tests, and build pass. Review exact-head parser safety and buyer-visible text boundaries; report actionable findings. |
Exact-head frontend evidenceCurrent head:
Hosted Checks and independent approval remain required. No force push or bypass. |
|
Fixed on exact head |
…nce' into fix/pr388-tag-boundaries
|
@opencode-agent @cwl-noema-review 재검토 요청: exact head 86ac1d4. 동시 wrapper-footnote 수정과 최신 PR #387 부모를 보존해 fast-forward stack으로 재적층했고 frontend lint, 149 tests, production build, Storybook build가 통과했습니다. |
068ed6a
into
feat/event-lineage-channel-evidence
There was a problem hiding this comment.
📝 Info: Marker hides footnote-line indentation from unit inference
inferIndentationUnit measures leading whitespace without stripping the footnote marker, so a marker-prefixed footnote line reports width 0. indentationLevel strips markers first, then measures. The two paths disagree on the same line's indentation. Impact is small since footnotes are rarely indented.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| const opensFootnote = isOoxmlContainer || isContainer; | ||
| if (!selfClosing) { | ||
| openTags.push({ name, isFootnote: opensFootnote }); | ||
| } | ||
| if (opensFootnote) { | ||
| if (!selfClosing) footnoteDepth += 1; | ||
| return `${tag}${FOOTNOTE_MARKER}`; | ||
| } | ||
| if ( | ||
| isWordParagraph || | ||
| (footnoteDepth > 0 && (name === "li" || name === "p" || name === "w:p")) | ||
| ) { | ||
| return `${tag}${FOOTNOTE_MARKER}`; | ||
| } |
There was a problem hiding this comment.
🔍 Unclosed footnote container mislabels all later paragraphs
markFootnoteTags raises footnoteDepth on an opening footnote container and only lowers it on a matching close. If a container is never closed in malformed markup, the depth stays positive and every later <li>/<p>/<w:p> is tagged role: "footnote". Balanced generator/Word/OOXML output avoids this; hand-authored or truncated HTML does not.
Was this helpful? React with 👍 or 👎 to provide feedback.
* 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>
Summary
MsoFootnoteText, and OOXML footnote/endnote containersDelivery
This is intentionally stacked on PR #387 (
4faf9a31371195c5ec63fca42a5afbb93a95369b) so the existing exact-head frontend build fixes remain in the base. Merge the base PR first, then rebase this change onto the resulting main head.Verification
corepack pnpm run lintcorepack pnpm run test— 144 passedcorepack pnpm run buildgit diff --checkNo real records or identifiers are included.