feat: external content association (paragraph ↔ external document reference) (#109) - #242
Conversation
Keyed on paragraph_id (stable w:sdt UUID) so external content links survive spec regeneration. Dual identity: DMS connector pair (ADR-014 D5) or url+hash. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire listAssociationsForSpec into getSpecTree (via attachAssociations recursive helper) and listAssociationsForParagraph into getParagraphWithAncestors so meta.associations surfaces on tree nodes and paragraph reads that have external content links. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GET/POST /specs/:id/paragraphs/:nodeId/associations + DELETE .../:associationId. openapi.yaml documents the routes + ParagraphAssociation schema (contract gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ons (#109) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughImplements External Content Association ( ChangesExternal Content Association
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over HTTPClient,DB: Create Association
participant HTTPClient as HTTP Client
participant Router as Express Router
participant createAssociationHandler
participant resolveIds
participant DB as DB Query Layer
HTTPClient->>Router: POST /specs/:id/paragraphs/:nodeId/associations
Router->>createAssociationHandler: req, res
createAssociationHandler->>resolveIds: specId, nodeId UUIDs
resolveIds->>DB: SELECT spec_id FROM paragraphs WHERE id=nodeId
DB-->>resolveIds: spec_id or null
alt paragraph not owned by spec
resolveIds-->>HTTPClient: 404
end
createAssociationHandler->>DB: createAssociation(nodeId, body)
DB-->>createAssociationHandler: ParagraphAssociation
createAssociationHandler-->>HTTPClient: 201 { success, data }
end
rect rgba(144, 238, 144, 0.5)
Note over Consumer,DB: Read Associations via Spec Tree
participant Consumer
participant getSpecTree
participant listAssociationsForSpec
participant attachAssociations
Consumer->>getSpecTree: specId
getSpecTree->>listAssociationsForSpec: specId
listAssociationsForSpec-->>getSpecTree: Map paragraphId → ParagraphAssociation[]
getSpecTree->>attachAssociations: SpecNode[], associationMap
attachAssociations-->>getSpecTree: SpecNode[] with meta.associations
getSpecTree-->>Consumer: enriched SpecTree
end
rect rgba(255, 223, 186, 0.5)
Note over MCP,DB: MCP get_paragraph includes associations
participant MCP as MCP Tool
participant handleGetParagraph
participant getParagraphWithAncestors
participant listAssociationsForParagraph
MCP->>handleGetParagraph: { paragraphId }
handleGetParagraph->>getParagraphWithAncestors: paragraphId
getParagraphWithAncestors->>listAssociationsForParagraph: paragraphId
listAssociationsForParagraph-->>getParagraphWithAncestors: ParagraphAssociation[]
getParagraphWithAncestors-->>handleGetParagraph: node with associations
handleGetParagraph-->>MCP: JSON with association label
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
🧹 Nitpick comments (3)
docs/superpowers/plans/2026-06-23-external-content-association.md (2)
26-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSmooth out repetitive sentence starters in the file structure section.
Lines 26–40 contain many consecutive bullet points starting with "Create" or "Modify". While the content is clear and actionable, LanguageTool flags the repetition as a style issue. Consider grouping by category or varying the phrasing (e.g., "Introduce", "Extend", "Wire") to improve readability without losing clarity.
🤖 Prompt for 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. In `@docs/superpowers/plans/2026-06-23-external-content-association.md` around lines 26 - 40, The bullet points in the file structure section contain repetitive sentence starters with "Create" and "Modify" appearing consecutively throughout lines 26–40, which impacts readability. To fix this, vary the phrasing by replacing some instances of "Create" with alternatives like "Introduce" or "Set up", and replace some "Modify" instances with "Extend", "Wire", or "Update" depending on the context of each task. Consider grouping related tasks (e.g., all new file creation together, then all modifications together) to naturally reduce consecutive repetition while maintaining clarity and actionability of the checklist.Source: Linters/SAST tools
943-951: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify OpenAPI schema generation for ParagraphAssociation and CreateAssociation schemas.
Task 5, Step 5 (lines 943–951) defers the exact OpenAPI schema definitions to "follow existing
components.schemasentries" but does not provide the complete YAML structure. While the DTO shape is fully specified in the locked row schema (lines 44–61) and Task 2 (ParagraphAssociationinterface), consider adding a brief YAML snippet showing theParagraphAssociationandCreateAssociationschemas as examples, so implementers know precisely which fields are required and optional, array nesting, etc.🤖 Prompt for 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. In `@docs/superpowers/plans/2026-06-23-external-content-association.md` around lines 943 - 951, The OpenAPI schema definitions for ParagraphAssociation and CreateAssociation in Step 5 lack concrete YAML structure examples, leaving ambiguity about required vs optional fields and proper formatting. Add complete YAML schema snippets under Step 5 (around lines 943-951) showing the ParagraphAssociation schema with required fields (id, label, externalMetadata, createdAt) and optional fields (externalProvider, externalId, url, contentHash), and the CreateAssociation schema mirroring the DTO structure from lines 44-61. Include explicit field types, descriptions, and required array specifications so implementers have precise guidance on schema structure.src/api/contract.integration.test.ts (1)
81-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer explicit response-schema assertions for new association endpoints instead of allowlisting.
Allowlisting these new JSON operations reduces contract guardrails for response-shape drift. Add explicit
assertResponse(...)checks insrc/api/associations.integration.test.tsand remove these allowlist entries once covered.As per coding guidelines,
src/api/**/*.ts:openapi.yamlis the live, authoritative API contract, so response conformance should stay directly tested.🤖 Prompt for 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. In `@src/api/contract.integration.test.ts` around lines 81 - 82, Remove the two allowlist entries for 'get /specs/{}/paragraphs/{}/associations' and 'post /specs/{}/paragraphs/{}/associations' from the allowlist in src/api/contract.integration.test.ts, then add explicit assertResponse(...) checks in src/api/associations.integration.test.ts to validate the response schemas for these two endpoints against the openapi.yaml contract. This ensures proper response-shape validation instead of bypassing it through allowlisting.Source: Coding guidelines
🤖 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 `@openapi.yaml`:
- Around line 272-290: Add a 400 response reference to the GET and DELETE
association operations in the OpenAPI specification to reflect that these
endpoints return HTTP 400 for invalid UUID path parameters. After the existing
404 and 500 response definitions in both operations (around the areas at lines
272-290 and 337-343), insert a new 400 response entry that references an
appropriate BadRequest error response component, ensuring the OpenAPI contract
matches the actual behavior of the association endpoints in
src/api/associations.ts.
- Around line 4238-4265: The ParagraphAssociation and CreateAssociation schemas
currently allow any combination of optional properties but the server enforces a
constraint that either url must be present, or both externalProvider and
externalId must be present together. Update both schema definitions to encode
this invariant using OpenAPI's oneOf construct to create mutually exclusive
groups: one group requiring url, and another group requiring both
externalProvider and externalId together. This ensures the schema accurately
reflects the server's validation logic and prevents clients from creating
invalid requests.
In `@src/ast/association-schemas.ts`:
- Around line 19-29: The current validation in the .check() method only rejects
when both hasDmsPair and hasUrl are false, but it permits partial DMS identity
(externalProvider without externalId or vice versa) whenever url is present.
Update the validation logic to ensure that externalProvider and externalId are
both defined or both undefined regardless of whether url exists. Add an
additional condition to check for mismatched DMS fields (where exactly one of
externalProvider or externalId is defined) and push an error to ctx.issues when
this partial state is detected, preventing mixed identity states from passing
validation.
In `@src/ast/associations.test.ts`:
- Around line 23-29: Add a new test case in the associations.test.ts file to
cover the regression scenario where a url is provided along with an incomplete
DMS pair (either externalProvider without externalId, or externalId without
externalProvider). The test should use CreateAssociationBodySchema.safeParse()
to verify that such combinations are properly rejected, similar to the existing
test structure, but with a url field included to ensure the schema rejects this
specific invalid combination of partially present identity credentials.
In `@src/db/migrations/032_create_paragraph_associations.ts`:
- Around line 25-27: The CHECK constraint in the pgm.addConstraint call for
paragraph_associations_identity_check is logically incomplete. It currently
allows partial external provider/id configurations when url is set, violating
the identity invariant. Modify the check predicate to enforce that either both
external_provider AND external_id are NOT NULL together, or both
external_provider AND external_id are NULL with url NOT NULL. The corrected
constraint should be: (external_provider IS NOT NULL AND external_id IS NOT
NULL) OR (external_provider IS NULL AND external_id IS NULL AND url IS NOT
NULL).
In `@src/db/queries/associations.ts`:
- Around line 67-83: The createAssociation function has a race condition where
resolveSpecId checks if the paragraph exists, then performs a separate INSERT
operation, allowing the paragraph to be deleted between these steps and
resulting in a generic 500 error instead of a proper 404. Replace the separate
resolveSpecId call and INSERT statement with an atomic INSERT...SELECT query
that joins against the spec table in a single operation, eliminating the race
condition. When the INSERT returns no rows (meaning the paragraph was not
found), throw AssociationParagraphNotFoundError instead of falling through to
generic error handling. Apply this same atomic pattern to the other similar
INSERT operation referenced in the range 88-93.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-06-23-external-content-association.md`:
- Around line 26-40: The bullet points in the file structure section contain
repetitive sentence starters with "Create" and "Modify" appearing consecutively
throughout lines 26–40, which impacts readability. To fix this, vary the
phrasing by replacing some instances of "Create" with alternatives like
"Introduce" or "Set up", and replace some "Modify" instances with "Extend",
"Wire", or "Update" depending on the context of each task. Consider grouping
related tasks (e.g., all new file creation together, then all modifications
together) to naturally reduce consecutive repetition while maintaining clarity
and actionability of the checklist.
- Around line 943-951: The OpenAPI schema definitions for ParagraphAssociation
and CreateAssociation in Step 5 lack concrete YAML structure examples, leaving
ambiguity about required vs optional fields and proper formatting. Add complete
YAML schema snippets under Step 5 (around lines 943-951) showing the
ParagraphAssociation schema with required fields (id, label, externalMetadata,
createdAt) and optional fields (externalProvider, externalId, url, contentHash),
and the CreateAssociation schema mirroring the DTO structure from lines 44-61.
Include explicit field types, descriptions, and required array specifications so
implementers have precise guidance on schema structure.
In `@src/api/contract.integration.test.ts`:
- Around line 81-82: Remove the two allowlist entries for 'get
/specs/{}/paragraphs/{}/associations' and 'post
/specs/{}/paragraphs/{}/associations' from the allowlist in
src/api/contract.integration.test.ts, then add explicit assertResponse(...)
checks in src/api/associations.integration.test.ts to validate the response
schemas for these two endpoints against the openapi.yaml contract. This ensures
proper response-shape validation instead of bypassing it through allowlisting.
🪄 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: 1e0f0e2c-d354-49c5-afb2-1708422224cf
📒 Files selected for processing (17)
docs/superpowers/plans/2026-06-23-external-content-association.mdopenapi.yamlsrc/api/associations.integration.test.tssrc/api/associations.tssrc/api/contract.integration.test.tssrc/api/router.tssrc/ast/association-schemas.tssrc/ast/associations.test.tssrc/ast/index.tssrc/ast/types.tssrc/db/index.tssrc/db/migrations/032_create_paragraph_associations.tssrc/db/queries/associations.integration.test.tssrc/db/queries/associations.tssrc/db/queries/paragraphs.tssrc/db/queries/specs.tssrc/mcp/associations.integration.test.ts
If the paragraph is deleted between resolveSpecId and the INSERT, the FK violation (pg 23503) now surfaces as AssociationParagraphNotFoundError — the same typed not-found the resolveSpecId miss throws — instead of a generic DatabaseError → 500. Extracted toCreateAssociationError to keep the function within the complexity cap. (PR #242 review, corroborated by CodeRabbit.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A {label, url, externalProvider} body (provider WITHOUT externalId) previously
validated and stored a dangling, unusable DMS identity. externalProvider and
externalId are now both-or-neither, INDEPENDENT of url, enforced consistently in:
- the Zod CreateAssociationBodySchema (.check pair-consistency rule),
- migration 032's identity CHECK ((external_provider IS NULL) = (external_id IS NULL)),
- openapi CreateAssociation (dependentRequired both ways).
The existing at-least-one-identity rule is unchanged. Regression tests at every
layer: {url,provider}/{url,id} rejected; {provider,id}/{url} accepted; raw
half-pair INSERT hits the DB CHECK (pg 23514).
(PR #242 review, corroborated by Codex gpt-5.5 + CodeRabbit.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
listAssociations and deleteAssociation validate UUID path params and return 400 on a malformed id, but openapi only documented 404/500. Add the 400 (BadRequest) response so the contract matches the handlers. (PR #242 review, CodeRabbit.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves conflicts after #242 (external content association) merged to main. All three conflicts were independent additions to shared surfaces, resolved by keeping both sides: - openapi.yaml: union the editability/reclassify/accept-as-note paths (#136) with the associations paths (#109); node schema carries both `editability` and `associations` properties. - src/api/router.ts: keep both the editability and associations handler imports. - src/api/contract.integration.test.ts: response-coverage set lists both PRs' ops. Verified on the merged tree: tsc + eslint clean, openapi prettier-clean, and the contract gate + association/editability/reclassify/MCP integration suites pass (56 tests, incl. bidirectional route↔spec coverage). Co-Authored-By: Claude Opus 4.8 <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>
Why
ADR-019 affirms external content association as in-scope: a firm links its own
collateral (e.g. a PDF datasheet) to a Part 2 product paragraph. SpecR stores the
link + provenance only — never the licensed bytes (the DMS owns transport, ADR-014).
This lets a spec writer point a reviewer at the authoritative manufacturer document for a
product paragraph without SpecR becoming a content host. Closes #109.
What
A new
paragraph_associationstable keyed on the paragraph UUID (the stablew:sdtround-trip anchor) so associations survive spec regeneration. Each row carries a dual-mode
external reference and is surfaced read-only inside the spec tree and MCP
get_paragraph,and managed through a small REST sub-resource.
paragraph_associations—paragraph_id(FK→paragraphsON DELETE CASCADE, the survival key) + denormalizedspec_id;label; dual identityexternal_provider+external_id(ADR-014 D5) orurl+content_hash; opaqueexternal_metadataJSONB. A CHECK enforces at least one identity; no bytes column.ParagraphAssociationDTO +SpecNodeMeta.associations?+ the ZodCreateAssociationBodySchema(v4 idiom,.check()).src/db/queries/associations.ts: create / list-for-paragraph /list-for-spec / delete + typed
AssociationParagraphNotFoundError.getSpecTreeattachesmeta.associationsper node (immutable rebuild);getParagraphWithAncestorsattaches them to the leaf node only. MCPget_paragraph/get_specride the same DB reads — no handler change needed.GET/POST /specs/:id/paragraphs/:nodeId/associationsandDELETE …/:associationId, documented in the CI-enforcedopenapi.yaml(contract gate green).Design decisions
opaque
external_provider+external_id; ADR-019 requires link + provenance. A firm with aDMS connector links by the connector pair; a firm without one (the common near-term case)
links by
url+content_hash. One nullable-column table with a presence CHECK serves both —no fork into two tables. The same "url OR provider+id" rule is encoded consistently in the DB
CHECK, the Zod refinement, and the openapi schema.
paragraph_id, not content. Thew:sdtparagraph UUID is stable acrossAST→DOCX regeneration, so an association keyed on it survives a regenerate/merge that rewrites
text and bumps
content_version.spec_idis denormalized purely so the spec-tree read is oneindexed query; the survival guarantee rides on
paragraph_id. Pinned by an explicitregeneration test.
:nodeIddoesn't belong to:id, the associationsub-resource simply doesn't exist under that path, so 404 is the honest status.
SpecR never dereferences
url. Transport stays with the DMS (ADR-014), keeping the platformcontent-neutral (ADR-019) and MIT-redistributable.
Scope (held)
Out of scope per the issue and ADRs, and not built: storing/serving file bytes, any DMS
transport, scheduling. No update/PATCH endpoint (YAGNI).
Testing
pnpm test— 988/988)pnpm test:integration— 534 pass; the only 2 failures arepre-existing & unrelated
src/api/docs.integration.test.tsScalar/openapi-serving cases thatfail at the branch base too)
pnpm lint— eslint + tsc strict + prettier)(
src/api/associations.integration.test.ts,src/mcp/associations.integration.test.ts)(
src/db/queries/associations.integration.test.ts)🤖 Co-authored by Claude Opus 4.8. Closes #109.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Summary by CodeRabbit
Release Notes