feat(ast): semantic article-role tagging — Related Sections / References / Submittals - #273
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deterministic role classification from the article heading (ADR-033). No DB column — derived, like editability/conflicts. Tolerant of CSI numbering prefixes; only 'article' nodes are tagged. Unknown headings carry no role. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the ast tagArticleRoles transform once in parse() so every freshly-parsed SpecTree (.sec/.docx/.txt) carries meta.articleRole. DOCX applies before ref extraction so refs see unchanged text. Update parse.test.ts toBe → toStrictEqual for tree assertions: withArticleRoles always returns a new object (immutable), so reference equality no longer holds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… toBe tests) Make tagArticleRoles return the same array when no node changes and withArticleRoles return the same tree when parts are unchanged, so parse() allocates nothing when there's nothing to tag. Reverts the parse.test.ts toBe→toStrictEqual loosening — the identity invariant holds again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
So DB-reconstructed trees — the path get_spec and GET /specs/:id/tree use — carry the role, using the same pure deriver as the parser. Article rows only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Gated by skipIf (copyrighted fixtures); CI exercises them. Asserts CPI ilvl offset does not break classification and no non-article node is tagged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e terminator
Final-review hardening: NUMBER_PREFIX_RE required a terminator so a digit glued
to a letter ("3D MODELING") is no longer treated as a CSI numbering prefix.
Safe-by-construction rather than safe only because no role title currently
begins after a digit. Pinned with a regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 5 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds derived ChangesSemantic article-role tagging
Sequence Diagram(s)sequenceDiagram
participant parse
participant tagArticleRoles
participant deriveArticleRole
participant buildNodeTree
parse->>tagArticleRoles: post-process parsed trees
tagArticleRoles->>deriveArticleRole: derive articleRole from heading text
buildNodeTree->>deriveArticleRole: derive articleRole from stored article text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ast/article-role.ts`:
- Around line 53-55: The normalizeHeading helper in article-role.ts is stripping
NUMBER_PREFIX_RE before trimming, so headings with leading whitespace can keep
their numeric prefix and fail lookup. Update normalizeHeading to trim the input
first, then remove the numbering prefix, then normalize internal whitespace and
uppercase the result. Keep the fix localized to normalizeHeading and preserve
its existing normalization behavior otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c8c03cc-88aa-4e8f-a9a5-11141c8be8c5
📒 Files selected for processing (14)
docs/adr/033-article-role-tagging.mddocs/superpowers/plans/2026-06-24-article-role-tagging.mdopenapi.yamlsrc/ast/article-role.test.tssrc/ast/article-role.tssrc/ast/index.tssrc/ast/schemas.tssrc/ast/types.tssrc/db/queries/specs.test.tssrc/db/queries/specs.tssrc/parser/article-role.test.tssrc/parser/docx/arcat.integration.test.tssrc/parser/docx/cpi.integration.test.tssrc/parser/index.ts
…DR-033) buildSubtree (the PATCH /specs/:id/paragraphs/:nodeId response path) shaped meta but did not derive meta.articleRole the way buildNodeTree does. Editing an article heading to or from a recognized CSI title left the immediate PATCH response without the role until a full-tree refetch — inconsistent across SpecNode responses. Mirror the same node_type='article' → deriveArticleRole derivation here so the field is present wherever the API returns an article. Regression test added at the API boundary: PATCH an article heading to "1.2 REFERENCES" and assert meta.articleRole === 'references' in the response (RED before the fix, GREEN after). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex review (GPT-5.5, xhigh) — additional eyes: 1 finding (P2), fixed.
No P1 findings; parse path, full-tree read path, and now the subtree response path all derive the role from the single |
NUMBER_PREFIX_RE is ^-anchored, so a heading with incidental leading
whitespace (" 1.1 REFERENCES") slipped past the strip and was left as
"1.1 REFERENCES" — unmatchable, so the article role was not derived. Trim
first, then strip the prefix, then collapse internal whitespace and trim
again. The "3D MODELING" anchor is unaffected: the digit-glued-to-letter
guard lives in the regex's terminator requirement, not the strip ordering.
Regression test pins both: " 1.1 REFERENCES"/"\t1.02 SUBMITTALS" now
classify, and " 3D MODELING" still derives no role.
CodeRabbit finding (src/ast/article-role.ts:55).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review loop complete ✅CI: Build · Lint · Test (unit + integration) · LOC-delta · contract gate — all green on CodeRabbit (genuine review, not a no-op — "Actionable comments posted: 1"):
Codex (GPT-5.5, xhigh — additional eyes): 1 finding (P2), fixed.
Both fixes reuse the single |
…033) changedSpecs() fingerprints revision snapshots with JSON.stringify( SpecTreeSchema.parse(tree)). buildNodeTree now adds meta.articleRole to article nodes, but snapshots frozen BEFORE article-role tagging have no such key — so an addendum comparing a post-change target against a pre-change base saw every role-bearing section as "changed" and listed unchanged sections in the addendum's Affected Sections. articleRole is a pure function of the heading text, which is already in the fingerprint, so it adds nothing but cross-version coupling. Strip all derived meta before fingerprinting (one place to extend for future derived fields) — content-change detection compares authored content only. Regression test: simulate a pre-tagging base snapshot by stripping articleRole from the stored base trees, then diff against a fresh target; assert only the genuinely-edited section is affected (the unchanged concrete/controls sections must not reappear). RED before the fix — the addendum rendered all three sections — GREEN after. Codex finding (src/db/queries/specs.ts:178 via revisions.ts treeFingerprint). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex re-review (GPT-5.5, xhigh) on
This is why I re-ran Codex against the final HEAD after the first two fixes — the deriver's reach into the persisted revision-snapshot path wasn't visible from the original diff against the pre-tagging base. All CI + the contract gate stay green. |
NUMBER_PREFIX_RE used (?:\.\d+)* (zero-or-more dotted groups), so the \s+ terminator branch also stripped a BARE integer or year — "1 REFERENCES" and "2024 REFERENCES" both normalized to "REFERENCES" and were classified 'references'. A bare integer/year is not a CSI article number (those are dotted: 1.1, 1.02, 1.1.1), so this misclassifies non-CSI headings, violating ADR-033's "absent rather than wrong" contract and risking downstream coordination checks selecting the wrong article. Require at least one dotted group ((?:\.\d+)+), so only a true CSI article number is stripped; bare integers/years are left intact and derive no role. All real CSI/CPI forms (1.1, 1.02, 1.1.1, with -/./)/ws terminators) still strip; the "3D MODELING" glued-digit guard is unchanged. Regression test pins it: "2024 REFERENCES"/"1 REFERENCES"/"1 SUMMARY" derive no role, while "1.1 REFERENCES"/"1.1.1 REFERENCES" still classify. RED before the fix, GREEN after. Codex finding (src/ast/article-role.ts:51). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex re-review (GPT-5.5, xhigh) on
Note: the existing Three Codex passes total on this PR (initial + two re-reviews as fixes landed), each catching a distinct reach of the deriver: subtree PATCH response, revision fingerprint, and now the prefix grammar. Running one more pass on |
Review loop converged ✅ — final Codex pass cleanAfter the dotted-prefix fix, a 4th Codex pass on Total findings handled on this PR — 4 (1 CodeRabbit + 3 Codex), all valid, all fixed + pinned with RED→GREEN regression tests:
Each Codex pass (run synchronously after fixes landed) caught a distinct reach of the new deriver that the prior diff couldn't see — subtree response, revision snapshot, prefix grammar — which is exactly why I re-ran it to convergence rather than once. Status: all CI green on |
…eb demo (#281) * feat(examples): surface recently-landed backend capabilities in the web demo Bake four capabilities that had landed on `main` but had zero demo surface into the web UI demo (one cohesive change — they share api.js/tree.js/app.js): - Article-role chips: render meta.articleRole (#258/#273, ADR-033) as a small humanized chip on each recognized article heading. Pure render. - Reversible paragraph removal: setParagraphRemoved() → PATCH .../removal (#251). A ⊘/↩ toggle on removable body paragraphs (pr1–pr7/continuation); removed nodes render greyed with a VANISH tag and a Restore affordance. 422 on structural/note nodes is surfaced as a clear warning. New API_FEATURES.paragraphRemoval flag (hard-delete paragraphDelete stays false). - Open-comments panel: getOpenComments/getProjectOpenComments (#262/#272). A Report-view panel grouped by section plus a masthead OPEN CMTS indicator (open count). New API_FEATURES.openComments flag. - Dangling-ref snippet: render the new snippet excerpt (#269) under each dangling_ref coordination row, with the source paragraph as locator context. Per-paragraph affordances are now wired per capability flag, so tree.js shows only the buttons the connected build actually serves. Verified end-to-end against the live stack (API + demo proxy + Postgres) with Playwright: chips render on the right articles, the removal round-trip flips vanish state and the Restore affordance both ways, the 422 path is handled, and both report panels render. No src/ or openapi.yaml changes. Closes #280 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(examples): refresh open-comments after spec membership & paragraph mutations The project-scoped open-comments diagnostic (OPEN CMTS masthead cell + report panel) was only refreshed on initial workspace load and inside refreshDiagnostics(). Spec membership mutations (upload, add-from-library/TOC, remove-from-project, library removal, project-settings save) and paragraph mutations (soft removal, hard delete, text edit) call refreshBrokenCount() and refreshCoordination() directly, so the open-comments view kept showing the previous membership's comments until a full workspace reload. Add refreshOpenComments() alongside the other two diagnostics at every site that refreshes after a state change, so the open-comments count and panel stay in sync with the rest of the diagnostics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#325) * docs(readme): sync capabilities to last month of merged PRs Reflect shipped work in the README's "Included Today", "API Surface", and MCP tool table, validated against the merged diffs and current main: - PDF ingest (text-layer + OCR + font-encoding recovery) accepted by POST /parse (#287, #290, #311) - coordination / E&O report + submittal register (#241, #269, #277, #282, #283, #284) and article-role tagging (#273) - onboarding pipeline: library import, editability review/override, reclassify, finalize/reopen, open-comments (#243, #247, #248, #249, #272) - spec/project soft-delete + restore (#257, #313), document concurrency (#197), revision/addendum manual rendering (#221), numbering profiles (#317, #322) - add missing MCP tools get_numbering_profile, submittal_register, open_comments_report; document GET /docs (Scalar) (#213, #285) - add Example Client pointer to examples/web_ui_demo (#225) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(roadmap): move shipped work to done; re-date to 2026-07-01 Reconcile the roadmap with merged reality (was stamped 2026-06-17). Moved from planned/in-progress to Included, each validated against the diff: - PDF ingest (#287, #290, #311) — remove from "Later" - deep paragraph nesting pr6/pr7 (#215) - revision nomenclature (#216) + revision/addendum manual rendering (#221) — the two "Near Term" Phase 2e items are done - coordination / E&O report, required-sections, article-role, submittal register (#239, #241, #269, #273, #277, #282, #283, #284) — new "Coordination and Semantics" section; removed "coordination report" from planned Phase 4 - onboarding APIs (#243, #247, #248, #249, #272) — API done; UI remains planned - soft-delete/withdraw (#257, #313), section-number format (#266, #271), external-content associations (#242), structural numbering profiles (#317) Kept as planned (foundation only): header/footer composition (#222, #314) and keynote surfacing (#315) — DB/AST exist, no resolution/render/export yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(architecture): reflect merged structural changes Update the architecture spec for shipped work, validated against the diffs and current schema/routes: - Tech Stack + Data Flow: Parse — PDF text-layer (unpdf/pdfjs-dist) + OCR (tesseract.js/@napi-rs/canvas) path and numberingProfileId override (#287, #290, #311, #317; ADR-034, ADR-039) - DB schema — specs.onboarding_status/withdrawn_at, projects.section_number_format /deleted_at/deleted_by, paragraphs.source_facts/classification/ editability_override; "Additional tables" summary for editing_conventions, paragraph_associations, required_sections, keynotes, header_footer_configs, numbering_profiles, revision_nomenclature_profiles (foundation-only tables flagged) (ADR-021/022/023/028/031/032; #187, #242) - new Coordination Report / E&O section (finding vocabulary) and Document Concurrency section (locks/optimistic/lifecycle) (#197, #241, #269, #277, #282, #283, #284; ADR-018, ADR-033/035/036/037) - AST meta.articleRole (#273, ADR-033); API-surface note pointing at the CI-enforced openapi.yaml + GET /docs; refreshed MCP tool list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes #258.
Why
The canonical CSI AST was semantic-light: every PART child was a generic
articlenode with no role tag, so there was no deterministic way to ask "which article is the Related Sections article? References? Submittals?". This is the keystone (Task-0) of the #256 wishlist: it unblocks the article↔body coordination checks (A2/A3 — Related Sections ↔ body refs, B2 — References ↔ cited standards) and the deterministic submittal register (item D). It lands first because it is the single most reusable foundation.What
Classifies PART articles by role deterministically from the article heading and surfaces it as an optional
meta.articleRoleonSpecNode, without mutating any existing field:summary,references,definitions,related-sections,submittals,quality-assurance,delivery-storage-handling,warranty— matched against a data table of canonical CSI titles + documented variants ("Related Requirements", "Reference Standards", "Section Includes", …).1.1,1.02,1.1.1), uppercases, and collapses whitespace before an exact-title lookup. Robust whether or not a parser already stripped the prefix.get_specautomatically — both serialize theSpecTreefromgetSpecTree, and the new field rides that serialization (no MCPtools.tschange needed). Documented inopenapi.yaml'sSpecNode.metaschema (CI contract gate green).Design decisions
parser/index.ts) so freshly-parsed trees carry it, and on DB read (db/queries/specs.ts buildNodeTree) so reconstructed trees carry it. Same pure function, two call sites — no drift, and editing a heading re-classifies for free. This mirrors howmeta.editability/conflictsare already shaped on read.src/ast/(the foundational leaf bothparser/anddb/import from via the barrel), so the module boundary stays clean.node_type='article'before the deriver runs, so ARCAT and CPI headings classify identically. Guarded by gated ARCAT + CPI fixture tests.articlenodes are tagged.note/continuationnever get a role (anotereading "REFERENCES" stays anotewith no role) — enforced at both chokepoints.tagArticleRoles/withArticleRolesare identity-preserving on a no-op (return the same reference when nothing is tagged), soparse()allocates nothing when there's nothing to classify — preserving the existing reference-identity invariant inparse.test.ts.// KNOWN AMBIGUITY:test per the OOXML ambiguity rule.article_rolecolumn (adds a migration + staleness risk for no benefit) and a dedicatedrolenode-type (forks every existingarticleconsumer).Testing
pnpm test1030/1030 (incl. 32 new: deriver classification, prefix-strip safety, immutability, KNOWN AMBIGUITY, parse-path.SEC,buildNodeTreeread-path).pnpm lint(eslint + tsc + prettier).contract.integration.test.ts) 8/8 — theopenapi.yamlarticleRoleaddition is validated against the live routes. (The 2docs.integration.test.tsfailures observed locally are an express-sendFiledotfile artifact of the.worktreescheckout path — they pass in CI / a non-dot checkout and are untouched by this branch.)skipIf; run in CI where the copyrighted fixtures are present) — assert References classifies and the CPI offset doesn't break it.🤖 Co-authored by Claude Opus 4.8 (1M context). Closes #258.
Summary by CodeRabbit
New Features
articleRolevalue in metadata.Bug Fixes