Skip to content

feat(db): keynote master table + project-filtered keynote query (ADR-016) - #315

Merged
thewrz merged 1 commit into
mainfrom
feat/issue-98
Jul 1, 2026
Merged

feat(db): keynote master table + project-filtered keynote query (ADR-016)#315
thewrz merged 1 commit into
mainfrom
feat/issue-98

Conversation

@thewrz

@thewrz thewrz commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Why

ADR-016 D1/D2. Keynotes connect drawing annotations to specification content: a drawing callout carries a keynote code that resolves to a CSI section (and optionally a specific paragraph). Annotating a drawing with a keynote that points at a section the project manual does not contain is a coordination error with errors-and-omissions exposure — so a project's valid keynote set is a filter over the firm's master keynotes, not a copy. This lands the storage + read primitive that the export endpoint (#99) and Revit sync (Phase 4) build on.

What

  • Migration 037_create_keynotes.ts — the keynotes master table per ADR-016 D1: library_id (FK → libraries, CASCADE), code, parent_code, description, target_section, optional target_paragraph_id (FK → paragraphs, SET NULL), UNIQUE (library_id, code). Reversible up/down.
  • getProjectKeynotes(projectId) (src/db/queries/keynotes.ts) — ADR-016 D2 filter: keynotes whose source library feeds the project (project_sources) and whose target_section is present in the project TOC (project_specs). Computed, never copied.
  • Barrel export through src/db/index.ts (getProjectKeynotes, ProjectKeynote).
  • Integration tests covering both acceptance criteria plus source-library filtering, priority resolution, and deep-link pass-through.

Design decisions

Calls I made where the issue left room, and why:

  1. Duplicate code across source libraries resolves by priority (one row per code). A Revit keynote table is keyed by code, and ADR-016 D2 explicitly says "keynotes from the project's source libraries (project_sources order)". So when two source libraries both carry the same code, the query returns a single row from the highest-priority library (DISTINCT ON (code) ... ORDER BY code, src.priority) — the same copy-on-derive precedence as ADR-015. Output is ordered by code for a deterministic, directly-renderable result (feat(api): project keynote export — keynote table file + MCP tool #99 renders it as-is).
  2. TOC = project_specs only. "Present in the project TOC" is read literally as the specs actually in the project manual, matching the existing "present spec" definition in coordination-implied.ts. Authored-but-not-yet-derived required_sections (ADR-028 intent) are deliberately not counted — a keynote is valid only against sections the manual really contains.
  3. Unknown project → [], not an error. A non-existent project has no project_sources and no project_specs, so the filter naturally yields empty. The 404 is the API caller's concern (feat(api): project keynote export — keynote table file + MCP tool #99, out of scope here) — mirrors listLibrarySpecs's "existence is the caller's concern" philosophy and keeps this read query from inventing an error class feat(db): keynote master table + project-filtered keynote query #98 doesn't ask for.
  4. Non-empty CHECKs on code and description; no shape CHECK. code is firm convention, opaque to SpecR (ADR-016: validate only uniqueness + target-section existence, never a numbering scheme), so no regex. target_section mirrors specs.section (varchar(20), no regex) — target-section existence is enforced at query time by TOC membership. The non-empty guards on code/description follow the established paragraph_associations house style (migration 032) — a blank code/description is meaningless.

Testing

  • Unit tests pass (pnpm test — 1216 passed)
  • Integration tests pass (getProjectKeynotes + table constraints — 10 passed locally against Postgres 16; full integration suite 716 passed, 0 failed)
  • Lint clean (pnpm lint — eslint + tsc + prettier)
  • Build clean (pnpm build)
  • Migration up → down → up verified clean (table created, dropped, recreated)
  • CI green

Both acceptance criteria are covered by tests: a keynote whose target_section is absent from the TOC is excluded, and UNIQUE (library_id, code) is enforced with a clean reversible migration.

🤖 Co-authored by Claude Opus 4.8. Closes #98.

Summary by CodeRabbit

  • New Features

    • Added support for retrieving project keynotes through the public database API.
    • Introduced a new keynote data structure including section and paragraph targeting details.
  • Bug Fixes

    • Project keynotes are now filtered to match valid project source libraries and included sections.
    • Duplicate keynote codes are handled consistently, with results returned in a stable order.
  • Chores

    • Added database constraints and tests to improve data integrity and coverage for keynote records.

…016)

Add the `keynotes` master table (ADR-016 D1) and `getProjectKeynotes`, the
project-validity filter (ADR-016 D2). Keynotes connect drawing annotations to
spec content; a master keynote is valid on a project only when its source
library feeds the project AND its target_section is present in the project TOC.
The set is computed, never copied — nothing to keep in sync.

- Migration 037: `keynotes` (library_id, code, parent_code, description,
  target_section, optional target_paragraph_id) with UNIQUE (library_id, code),
  reversible up/down. `code` stays opaque (no shape CHECK per ADR-016); non-empty
  guards on code/description match the paragraph_associations house style.
- getProjectKeynotes(projectId): joins project_sources + filters on project_specs
  TOC membership; a code carried by two source libraries resolves to the
  highest-priority one (DISTINCT ON code, ORDER BY code, src.priority).
- Barrel export through src/db/index.ts.

Export file format (#99) and Revit sync (Phase 4) are out of scope.

Closes #98

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new keynotes PostgreSQL table via migration 037, implements getProjectKeynotes with DISTINCT ON filtering against project TOC membership, exports the ProjectKeynote type and function from the db barrel, and covers both table constraints (D1) and query filtering (D2) with integration tests.

Changes

Keynotes table, query, and public API

Layer / File(s) Summary
keynotes table migration and getProjectKeynotes query
src/db/migrations/037_create_keynotes.ts, src/db/queries/keynotes.ts, src/db/index.ts
Migration creates the keynotes table with FKs to libraries and paragraphs, UNIQUE (library_id, code), CHECK constraints for non-blank code/description, and two indexes. keynotes.ts defines the ProjectKeynote interface, mapRow, and getProjectKeynotes using DISTINCT ON (k.code) with joins against project_sources, project_specs, and specs. Both symbols are re-exported from src/db/index.ts.
Integration tests — D1 constraints and D2 filtering
src/db/queries/keynotes.integration.test.ts
Raw SQL helpers seed libraries, projects, TOC specs, paragraphs, and keynotes. Tests assert TOC-section exclusion, non-source-library exclusion, result ordering, parent code and target_paragraph_id propagation, duplicate-code resolution, unknown-project empty result, and database-level constraint enforcement for uniqueness and blank fields.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #98: This PR directly implements the keynotes migration, getProjectKeynotes query, and barrel export described in the issue, satisfying all listed acceptance criteria.
  • #99: The API/MCP export feature depends on getProjectKeynotes and ProjectKeynote, both of which are introduced here.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main DB keynote table and filtered query work.
Linked Issues check ✅ Passed The PR implements the ADR-016 table, filtered query, barrel export, and required uniqueness/reversibility checks.
Out of Scope Changes check ✅ Passed The changes stay within keynote storage, lookup, export, and test coverage without introducing unrelated scope.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-98

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thewrz

thewrz commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Adversarial review pass (Codex + local checks)

Ran Codex (GPT-5.5, xhigh) adversarially against main plus an independent local verification sweep. Clean — no fixes needed; PR stays draft.

Codex verdict: "No actionable correctness issues were identified in the migration, query, barrel export, or integration coverage for the requested scope."

Independent SQL correctness/security check (the only place a real bug could hide):

  • Both filters bind $1 (projectId) — parameterized, no injection.
  • project_sources PK is (project_id, library_id), so the join can't fan out a keynote into duplicate rows.
  • DISTINCT ON (k.code) … ORDER BY k.code, src.priority resolves a cross-library duplicate code to the lowest-priority-number (highest-precedence) source library — the intended ADR-015/ADR-016 D2 precedence.
  • target_section is NOT NULL, so no NULL surprises in the TOC IN (…) membership test.
  • Migration UNIQUE (library_id, code) + CASCADE (library) / SET NULL (paragraph); up → down → up verified clean.

Local checks (Postgres 16):

  • pnpm lint (eslint + tsc + prettier) — green
  • pnpm build — green
  • Unit: 1216 passed
  • Integration (getProjectKeynotes + table constraints): 10 passed; full integration suite 716 passed, 0 failed

Nothing flagged to decline — Codex raised no findings, and no scope creep into #99 (export format) or Phase-4 Revit sync was introduced.

@thewrz
thewrz marked this pull request as ready for review June 30, 2026 15:24

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/db/queries/keynotes.ts`:
- Around line 3-4: Introduce a keynote-query-specific error type for this module
instead of throwing the shared DatabaseError directly. Add a typed error class
owned by keynotes.ts that extends SpecrError, then update getProjectKeynotes()
to wrap database failures with that new error while preserving the original
cause. Keep the existing error chaining behavior and ensure any catch path in
this module uses the new error surface so callers can distinguish keynote-query
failures from other DB errors.
🪄 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: 1701bcd5-aa1e-4d4d-9442-fe325091abd9

📥 Commits

Reviewing files that changed from the base of the PR and between e655c77 and 1798a23.

📒 Files selected for processing (4)
  • src/db/index.ts
  • src/db/migrations/037_create_keynotes.ts
  • src/db/queries/keynotes.integration.test.ts
  • src/db/queries/keynotes.ts

Comment thread src/db/queries/keynotes.ts
@thewrz

thewrz commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Codex (GPT-5.5, xhigh) adversarial review — additional eyes: No actionable correctness issues found. The new migration, project-filtered keynote query, barrel exports, and tests align with the documented ADR-016 scope. (CodeRabbit reviewed normally; Codex ran as a second independent pass.)

@thewrz
thewrz merged commit 68ddd27 into main Jul 1, 2026
11 checks passed
@thewrz
thewrz deleted the feat/issue-98 branch July 1, 2026 01:02
thewrz added a commit that referenced this pull request Jul 1, 2026
#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(db): keynote master table + project-filtered keynote query

1 participant