Files
dbx/docs/superpowers/plans/2026-08-02-database-documentation-annotations.md
T
f30d59b989 feat(docs): database documentation viewer and DBML export
* feat(docs): add SchemaSnapshot model

* feat(docs): infer relationship cardinality from foreign keys

* fix(docs): resolve foreign keys within the source schema

* feat(docs): convert group hue to sRGB hex for DBML

* feat(docs): add DBML lexical primitives

* feat(docs): render DBML table blocks

* feat(docs): render DBML refs, enums and table groups

* feat(docs): assemble complete DBML documents

* feat(docs): collect schema snapshots with bounded fan-out

* fix(docs): gate FK warning on engine capability and reference synthesized enums

* fix(docs): qualify synthesized enum references in multi-schema output

* test(docs): anchor the multi-schema enum reference assertion

* feat(docs): add snapshot collection route

* feat(docs): add collect_docs_snapshot to DbxBackend

* feat(docs): add dbx dbml command

* test(docs): add live snapshot and DBML verification

Runs collect_snapshot + to_dbml against a real PostgreSQL database
(organon, 47 tables) and asserts structural DBML validity: Project
header, every table present, balanced braces, trailing newline.

* refactor(docs): use sort_by_key for snapshot table ordering

* docs(docs): add database documentation design and implementation plan

Records the Part 1 design (SchemaSnapshot + DBML export) and the plan that
produced it. The plan carries an appendix listing the thirteen assumptions
that proved wrong during execution, so the corrected facts are not re-derived
from the surrounding prose.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKcC15gEuQBHMidPCFFYwn

* fix(docs): use native enum names, canonical engine labels, and surface metadata failures

Final review fix wave covering four findings:

- PostgreSQL named enums (e.g. `ConversationStatus`) now keep their own
  type name and synthesized: false, instead of being renamed to
  `{table}_{column}` and losing identity. A type shared by several
  columns now dedupes to a single Enum block instead of colliding or
  duplicating. `synthesize_enum` and `render_type` route through one
  shared `enum_type_name` helper so the two can't drift apart again.
- `database_type` (and the CommentsUnsupported/NoForeignKeyMetadata
  warnings) now use the same canonical engine label already used
  throughout table_structure_sql's own warning prose, instead of a raw
  Rust Debug string (`Postgres`, `SqlServer`, `MongoDb`).
- An index-fetch failure during collection now surfaces as a
  TableSkipped warning instead of silently degrading to an empty index
  list, which relations.rs uses to infer relationship cardinality.
- A schema-enumeration failure now surfaces as a warning instead of
  silently proceeding against schema "".
- Removed the redundant Arc<Semaphore>; buffer_unordered already caps
  concurrency at MAX_CONCURRENT_TABLES.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKcC15gEuQBHMidPCFFYwn

* feat(docs): add annotation file model

* test(docs): assert full round-trip fidelity for the annotation model

* feat(docs): add per-engine annotation key folding

* fix(docs): do not fold identifiers on case-sensitive engines

* feat(docs): load and validate the notes file

* fix(docs): report a version mismatch before unknown-field errors

* feat(docs): merge annotations into the schema snapshot

* feat(docs): detect orphaned annotations without deleting them

* feat(docs): add --notes to dbx dbml

* feat(docs): remember a notes file path per connection

Adds docs_notes_path to ConnectionConfig so the desktop app (Part 3)
can persist where a connection's documentation notes file lives. The
CLI is unaffected — it takes an explicit --notes path.

ConnectionConfig has a hand-written Deserialize impl that delegates to
a mirror struct, ConnectionConfigData, and converts via From. Adding
the field only to ConnectionConfig would compile but never populate
from stored JSON, since ConnectionConfigData's fields are what serde
actually reads. The field is threaded through all three places:
ConnectionConfig, ConnectionConfigData, and the From impl, mirroring
the existing `color` field.

* test(docs): verify annotations against a live database

* test(docs): assert exactly one orphaned annotation

* feat(docs): add snapshot types for the docs viewer

* fix(docs): correct snapshot type nullability and add missing column fields

* feat(docs): add a real-output fixture and drift conformance test

The fixture is generated by dump_docs_fixture.rs from a live collect_snapshot
run against the keycloak database in the shared local-infra stack, with
annotations applied so a LOCAL note, column note, group and orphanedNotes
warning are all present. fixtureConformance.spec.ts asserts against that real
JSON rather than a hand-written literal, so a change to the Rust snapshot
shape breaks the test instead of silently drifting from the hand-maintained
types.ts.

Keycloak is used because its schema is public open-source knowledge, so the
committed fixture carries no private schema. The kept tables are an explicit
allowlist rather than an alphabetical slice, because the conformance test
needs a connected foreign-key subgraph: protocol_mapper has two foreign keys
to different tables and composite_role has two to the same one.

* feat(docs): add index grouping for the docs viewer

* feat(docs): describe snapshot warnings for the viewer

* feat(docs): add client-side search for the docs viewer

* test(docs): make the search fixture able to fail

* feat(docs): expose group hue as a CSS custom property

* docs(plan): add annotations and viewer plans, correct Task 7

Parts 2 and 3a were planned after the first plan was committed and were never
tracked. docs/superpowers/ is gitignored, so both needed -f, matching how the
existing specs and plans in that directory were added.

The viewer plan's Task 7 is corrected against the installed marked@18.0.4. Its
original text carried four defects, found by probing the library rather than by
review: an assertion that fails against a correct implementation, a pre-escaping
approach that double-escapes entities, a javascript: blocklist with live
bypasses (entity-encoded, vbscript:, data:text/html, and <img src>, which was
never covered at all), and a link renderer whose text property is raw markdown
source rather than parsed HTML — an XSS hole found while verifying the fix for
the previous defect.

The viewer plan also gains a corrections appendix grouping defects by failure
mode. The annotations plan is committed as written; its defects are recorded in
a follow-up.

* feat(docs): render note markdown with raw HTML escaped

* fix(docs): drop protocol-relative URLs in note markdown

The URL allowlist permitted anything starting with / so relative paths work,
and //evil.com qualifies. Over https that grants nothing a note author could
not do with an ordinary https link, but the Part 3b standalone export is
opened via file://, where //host/path is a UNC path. On Windows that opens an
SMB connection and leaks an NTLM hash, with no click required since images
auto-load, and it is plantable from a COMMENT ON value.

Found by an adversarial probe of the committed module rather than by review;
the backslash form was already dropped, only the slash form slipped through.

* docs(plan): add corrections appendix to the annotations plan

Groups the defects found executing Part 2 by failure mode, matching the form
of the viewer plan's appendix. The notable one is Mode A: docs_notes_path had
to be added in three places because ConnectionConfig has a serde mirror
struct, and adding it in one place compiles, passes a round-trip test written
against the same struct, and then reads None forever after every load.

Also records a controller hypothesis that turned out to be wrong, since
checking it cost one read.

* fix(docs): reject backslash protocol-relative URLs in notes

The // guard from the previous commit was itself a blocklist: /\evil.com
starts with a single slash, so it passed, and the WHATWG URL spec treats /\ identically to // for special schemes. Browsers normalise the backslash, so
it reaches the same file:// UNC path and the same no-click NTLM leak.

Rejecting both separator characters in both positions closes it. Found by
probing what String(raw).trim() leaves unnormalised before the prefix checks;
the third defect on this file found by probing rather than by reading.

* feat(docs): add docs viewer components

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKcC15gEuQBHMidPCFFYwn

* test(docs): pin each theme's legacy colour base independently

The ordering check indexOf(hsl) < indexOf(@supports) quantified over any
occurrence, so deleting the light .docs-group block left the dark block's hsl
satisfying it. The test passed while every table group rendered colourless on
light-theme WebViews without oklch.

Asserting each selector's own base block catches deleting either one. Found by
the task implementer, which deleted one block and then both to show the guard
pinned 'some base exists' rather than 'each selector has a base'.

* fix(docs): escape single quotes in note attribute values

escapeHtml covered & < > and double quotes but not single quotes. Not
exploitable today because every attribute in this file is double-quoted, but
that is a formatting convention enforced nowhere and living in a different
part of the file from the escaper. A future edit writing title='...' would
turn a formatting choice into an attribute breakout.

Raised by review as latent fragility rather than a defect; fixed because the
escaper should be correct on its own rather than correct-given-an-invariant.

* test(docs): match single-quoted v-html bindings in the contract guard

The guard matched /v-html\s*=\s*"([^"]*)"/ — double quotes only. A binding
written `v-html='table.note'` produced zero matches and passed, handing a
database COMMENT ON value straight to the DOM with the renderNote sanitiser
bypassed. Both quote styles are valid Vue and nothing in the repo enforces
one, so the guard had a hole exactly where it mattered most.

Verified by temporarily rewriting a real WikiIndex binding as
`v-html='table.note'`: the test now fails with "WikiIndex.vue: v-html must
render renderNote output: expected 'table.note' to contain 'renderNote'".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKcC15gEuQBHMidPCFFYwn

* fix(docs): cap search results per kind instead of overall

DocsSearch sliced searchDocs output to 40 AFTER concatenation, and the
concatenation order is tables -> columns -> groups -> enums. Columns always
flooded the list, so the cap deleted the tail — every group and enum hit.
Against the real fixture, "e" produced 155 hits (9 table, 133 column, 1 group,
12 enum) and rendered 9 tables, 31 columns and nothing else; groups and enums
were structurally unreachable through search.

Cap each kind against its own limit inside docsSearch.ts, where the logic is
tested, and drop the slice from the template so exactly one place limits
results. Ranking is unchanged: tables still precede columns.

Verified by reverting to a single .slice(0, 40) over the concatenation: the
new tests fail with "enums must survive a column flood: expected false to be
true" and "expected 30 to be 20".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKcC15gEuQBHMidPCFFYwn

* fix(docs): move the index card note out of its button

The card rendered renderNote output with v-html inside the <button>. A note
containing a markdown link — [spec](https://example.com) — put an <a> inside a
<button>: invalid nesting, and the anchor was not keyboard reachable because
the button swallows it in the tab order.

The <li> now carries the card's border, background and hover, the button holds
only the table name and kind, and the note is its sibling. Visually identical —
same padding, same 0.5 gap, previously mt-0.5 — and the name row still spans
the full width as the click target.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKcC15gEuQBHMidPCFFYwn

* test(docs): pin every fixture struct's key set in both directions

The guard pinned ColumnInfo and IndexInfo plus three DocTable keys, and only
in the "no missing key" direction. Relationship, FieldRef, DocEnum,
ProjectMeta, ColumnNote, ForeignKeyInfo, TableGroup and 10 of 13 DocTable keys
were unchecked. Renaming Relationship::to to `target` in Rust kept the suite at
67/67 and vue-tsc at exit 0 while RelationshipList read `field.table` on
undefined and every table page rendered blank.

Every struct in the fixture is now pinned both ways — no missing key, no
unexpected key — over every instance rather than element [0]. Each direction
catches a different half of a rename. Object.hasOwn throughout, so a key that
is present and null stays distinguishable from one skip_serializing_if omitted.

ForeignKeyInfo, TableGroup and the SchemaSnapshot root are included beyond the
list the review gave: they are equally present in the fixture and equally
unpinned.

Verified against a modified copy of the fixture outside the repo with
Relationship::to renamed: "Relationship[0] must always carry to: expected
false to be true".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKcC15gEuQBHMidPCFFYwn

* fix(cli): error when an explicit --notes path does not exist

load_annotations returns Ok(None) for a missing file, which is right for the
implicit per-connection notes path — it may legitimately not exist yet. It is
wrong for --notes, where the user named a specific file: a mistyped path
produced DBML with every note silently absent and no diagnostic at all,
indistinguishable from a database that has no documentation.

Both final reviewers independently ruled this must-fix-before-merge.

* fix(docs): treat an explicit FK ref_schema as authoritative

find_target tried the explicit ref_schema, then fell through to the source
table's own schema on failure. When the referenced schema was not collected —
routine, since users select schemas — an FK from sales.orders to
archive.customers resolved to sales.customers instead: a different table, and
a diagram that is confidently wrong rather than visibly incomplete.

The function's own doc comment already promised that keys pointing outside the
collected set are dropped. Now it does that.

This is a regression of the defect found in Part 1: the three-tier lookup was
added then, but tier one was written to fall through rather than to decide.

* fix(docs): corroborate engine capability warnings against what was collected

supports_comments and supports_foreign_keys delegate to the structure
editor's DDL-generation capabilities, not to introspection support. IRIS is
the proven divergence: it reports %DESCRIPTION on introspection but DBX
cannot ALTER an existing one, so the flag is false while the collector reads
and includes those comments — producing a snapshot that warned comments were
unsupported alongside the comments themselves.

Each warning now fires only when the flag says the engine cannot AND
collection found nothing of the kind to contradict it. ClickHouse and Doris,
which genuinely report no foreign keys, still warn. The doc comments now say
what the functions actually measure.

Found by final review, which traced every other caller to establish the
flag's real semantics.

* docs(fixture): finish repointing the fixture source to keycloak

Follows the rebase that replaced the fixture at its origin commit. Repoints
the live annotation test's project identity and both plans, and records the
one capability keycloak costs us.

Keycloak declares no PostgreSQL enum types, so the fixture cannot exercise
DocEnum — a Rust-side rename of a DocEnum field would pass every test here
and break the viewer's enum rendering silently. Rather than delete the test,
it now asserts the gap, so it fails the day the fixture source gains an enum
and prompts restoring the pin.

* docs(spec): design for in-app database documentation (Part 3b)

Mounts the Part 3a viewer in DBX and makes it editable: table/column notes,
table groups, and per-group colour, autosaved to a notes file that can live
in the user's repository.

Two findings shaped the scope. DBX already ships SchemaDiagramDialog, so the
viewer links to it rather than building a second ER diagram — only the Part 3c
export needs its own minimal renderer, because that dialog reaches into stores
and cannot be inlined. And nothing currently reads docs_notes_path or writes
annotations at all, so 'in-app editing' needs new Rust rather than frontend
wiring alone.

The standalone export, dbx docs verb and hash routing are deferred to 3c.

* docs(plan): implementation plan for in-app database documentation

Ten tasks: atomic annotation save and path resolution, Tauri commands, web
route parity, the frontend facade, pure edit transforms, the i18n namespace
with a parity guard, editing components, the enum page, edit plumbing through
the viewer, and the dialog with debounced autosave.

Self-review caught three defects before dispatch, all the same class that cost
Part 3a ten fix rounds: a return type named DescribedWarning that does not
exist (it is WarningNotice), a test calling emptySnapshot() which does not
exist, and a table() helper invoked with columns when its real signature takes
a groupId. Every identifier the plan names is a claim about the codebase.

* docs(plan): resolve the data directory without a dbx-mcp dependency

Tasks 2 and 3 called dbx_mcp::paths::app_data_dir(), but neither src-tauri nor
dbx-web depends on dbx-mcp, so neither would have compiled. Both already have
a better source: AppState.storage.data_dir() honours a custom data dir, and
WebState already carries data_dir.

Found by the pre-flight scan before any implementer saw it.

* docs(plan): fix Task 1 against the real crate (no Default, no tempfile)

ConnectionConfig has no Default derive and ~60 fields, so resolve_notes_path
now takes the connection id and the optional override directly — the two
fields it actually reads. The test pain was pointing at the signature.

dbx-core has no dev-dependencies, so the tests use the temp_dir + uuid idiom
already present in annotations.rs rather than tempfile.

* feat(docs): add atomic annotation save and notes path resolution

* docs(plan): guard the autosave against concurrent writes

flush() cleared the debounce timer but not an in-flight write, so closing the
dialog while a debounced save was awaiting the backend started a second one.
Two concurrent saves of the same file waste a round trip, race to land stale,
and are the exact concurrency that corrupts the notes file when the temp path
is not unique per writer.

Found while adjudicating the Task 1 review, which demonstrated the Rust half
of the same problem.

* fix(docs): make temp paths unique to prevent concurrent save corruption

* fix(docs): replace vacuous atomicity test with inode-based verification

* docs(plan): correct the apply_annotations import path

dbx_core::docs re-exports collector, color, dbml, keys, relations and snapshot
but NOT annotations, so apply_annotations and friends are only reachable at
dbx_core::docs::annotations. Tasks 2 and 3 both used the shorter path and
would not have compiled.

Verified against crates/dbx-core/src/docs/mod.rs before either was dispatched.

* fix(docs): truncate temp filename to stay within 255-byte filesystem limit

* feat(docs): add Tauri commands for docs snapshot and annotations

* docs(plan): propagate the resolve_notes_path signature to its callers

Pre-flight changed resolve_notes_path to take (connection_id, docs_notes_path,
data_dir) instead of a ConnectionConfig, but only Task 1 was updated. Tasks 2
and 3 still called it with the old signature and would not have compiled.

The Task 2 implementer caught it and used the correct form from its dispatch
note. Task 3 had the identical stale call and had not been dispatched yet.

The plan's self-review checks signature consistency across tasks; this changed
AFTER that review, during pre-flight, and nothing re-ran the check.

* docs(plan): pin Task 4's http.ts idiom and the Tauri argument names

http.ts uses a post<T>(url, body) helper at line 222; the plan said only
'match the existing idiom', which is delegating verification to someone with
less context. Written out concretely now.

Also records what Task 2's review flagged as unverifiable from its own diff:
Tauri serialises command arguments by name, so the invoke object keys must
match the Rust parameter names. That mismatch compiles cleanly on both sides
and fails only when a user clicks — and it falls in the gap between two
task-scoped reviews, since neither diff contains both halves.

* feat(docs): add web routes for annotation load, apply and save

Mirrors the Tauri commands (docs_load_annotations, docs_apply_annotations,
docs_save_annotations) added in the previous task: collect returns the raw
snapshot, apply is separate so the shadowedNote rule stays in one place.

* docs(plan): make the i18n parity guard actually observable

Every non-English locale is export default withEnglishFallback({...}) — the
fallback is applied at module level, inside the locale file. Only en.ts is a
bare object. So importing a locale's default export yields the ALREADY-MERGED
object, and the parity test would have found every key present in every locale
and passed while translations were missing.

The test written to catch silent English fallback would have been silently
defeated by that fallback.

Task 6 now puts the new namespace in per-locale modules under locales/docs/,
which the test imports directly and unwrapped. Scoped entirely to the new
namespace; the existing 315 KB of keys are untouched. Step 6 also asks the
implementer to demonstrate the trap: point the imports back at the merged
modules and watch a missing key pass.

* feat(docs): expose docs snapshot and annotations to the frontend

* docs(plan): locate the duplicated table-key rule correctly in Task 8

The plan said docsIndex.ts builds the qualified table key inline. It does not
— it groups by table.schema, a section key. The table key rule lives in a
private qualified() in docsSearch.ts and in a hand-rolled tableKey() in
DocsApp.vue, DocsSidebar.vue and WikiIndex.vue.

Part 3a's final review flagged that duplication as a Minor and it was
deferred. Task 8 was about to add a fifth copy, so it now extracts
docsKeys.ts first and replaces the existing ones. This is the key annotations
are stored under, so two call sites disagreeing attaches a note to the wrong
table.

* feat(docs): add pure annotation edit transforms

* docs(plan): ground the group hue picker in DBX's existing swatch idiom

ConnectionDialog.vue already has a swatch row — h-6 w-6 rounded-full buttons,
ring-2 selected state, i18n titles. Task 7 now points at it so the new picker
looks native.

With an explicit warning not to copy the fill mechanism: connection colours
are hex painted via Tailwind classes, group colours are hues rendered through
docs.css. A naive copy introduces hex literals and fails the contract test —
correctly, since a hardcoded hex cannot stay legible on both grounds, which is
why groups store a hue at all.

* docs(plan): make Task 10's dialog wiring concrete

Five exact locations, all verified: the store ref and its export, the
useDialogSources watcher, the AppDialogs import and render, and the
ObjectBrowser trigger plus its context-menu entry.

Records the non-obvious part: the watcher clears the source back to null
after firing, and that clearing is what makes the dialog re-openable —
without it, setting the same value twice does not re-trigger the watch.

ObjectBrowser is the entry point rather than the connection tree, because the
tree has no diagram entry either and ObjectBrowser already supplies exactly
the prefills the docs dialog needs.

* feat(docs): add the docs i18n namespace with a parity guard

* fix(docs): ban vue-i18n from standalone-exportable docs components

WarningBanner.vue used useI18n() directly, which throws with no Vue app
instance -- exactly the standalone HTML export case describeWarning's
translator parameter exists to avoid. Thread translate as a prop from
DocsApp instead, and add vue-i18n/useI18n( to the component contract's
forbidden list so the constraint is enforced, not just documented.

* docs(plan): note that DocsApp already has snapshot and translate

Task 6's fix added translate to DocsApp when describeWarning started taking a
translator. Task 9 said to add it, which would have been a duplicate prop.
It now says to add only annotations and readonly to the existing defineProps.

* feat(docs): add note editor, group editor and group picker

* docs(plan): give Task 10 the dialog shell from SchemaDiagramDialog

Exact Dialog primitives, the get/set computed every dialog here uses to bridge
the open prop, and the sizing class copied verbatim from
SchemaDiagramDialog.vue:827 — the docs viewer is the same kind of full-window
workspace, not a form, so it should not invent dimensions.

Also states explicitly that this component lives outside src/docs/ and so may
and must use useI18n(): it is what supplies the translate prop the viewer
components need, since they are banned from importing vue-i18n themselves.

* test(docs): guard the light-ground group tokens too

* feat(docs): add the enum page and a shared table-key helper

EnumPage renders an enum's values and every column declared with that type.
It is read-only on purpose: AnnotationFile has no `enums` key, so an edited
note would have nowhere to be saved.

qualifiedTableKey moves the `schema.name` rule — bare name on schema-less
engines like SQLite and MySQL — into docsKeys, where the call sites that had
each copied it can share one definition. It is the key annotations are stored
under, so two call sites disagreeing would attach a note to the wrong table.

columnsUsingEnum matches data_type exactly rather than by substring: an enum
named `state` would otherwise claim every `estado` and `statement` column.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VTBWnT4goXLA9iqDrfTPqk

* refactor(docs): share one qualified table key across the viewer

Six call sites had each copied the `schema.name` rule; they now import
qualifiedTableKey from docsKeys instead. Two were not in the plan's list:
TablePage.vue and RelationshipList.vue. RelationshipList passes a remapped
FieldRef rather than a DocTable, which is why the helper takes
Pick<DocTable, "schema" | "name"> — that widening is what let every call site
be adapted directly instead of keeping a thin delegating wrapper.

Also strengthens the columnsUsingEnum substring guard, which was not guarding
anything. Its only column had type `integer`, and "integer".includes("state")
is false, so replacing the exact match with includes() left all 8 tests
green. Adds a `statement` column — the only type here that really contains
`state` — and drops `estado` from the doc comment, since it does not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VTBWnT4goXLA9iqDrfTPqk

* feat(docs): thread editing through the viewer as emitted events

DocsApp gains `annotations` and `readonly` and re-emits a DocsEdit for every
change its children request. Nothing under src/docs/ persists anything, which
is what keeps the directory bundleable into the standalone HTML export; a new
contract guard now pins that by rejecting any component that names
save/load/applyDocsAnnotations.

NoteEditor is fed the MERGED snapshot note, not the local annotation layer. It
renders and edits one value, so seeding it locally would show nothing for a
note that came from a database comment. Writing over one shadows it, which is
what noteSource and shadowedNote already exist to disclose.

`annotations` is threaded for what the merge erases: `groups` carries the
editable GroupAnnotation records, while snapshot.groups carries resolved
TableGroups that GroupPicker and GroupEditor cannot write back to.

Also makes enums reachable. EnumPage rendered nowhere and search returned enum
hits that DocsSearch deliberately disabled, since enums carry no table key;
they now navigate by bare name, which is how columnsUsingEnum resolves them
too. Groups remain unclickable — they still have no page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VTBWnT4goXLA9iqDrfTPqk

* feat(docs): mount the documentation viewer in DBX with autosaved editing

DatabaseDocsDialog hosts DocsApp outside src/docs/, which is what lets it use
useI18n() and supply the `translate` prop the viewer components are banned from
importing for themselves. It collects the snapshot, loads the notes file
(falling back to emptyAnnotations), and holds the raw snapshot so every edit can
re-derive the merged view through applyDocsAnnotations.

createAutosave debounces writes and, above all, makes a failure visible: a
silently swallowed write is the worst outcome here, because the user keeps
typing and believes their notes are saved. It also refuses to run two writes at
once — flush() clearing the timer does not stop a write already awaiting save,
and two concurrent writes of the same file is the exact race that corrupted the
notes file before the Rust side used a unique temp path. Both properties are
pinned by tests I confirmed fail when the guard is removed.

Loads and re-derivations carry a generation number so a slow response cannot
overwrite a newer one, and closing flushes the debounce rather than dropping a
note typed a moment earlier.

Trigger mirrors the schema diagram's wiring: docsSource on connectionStore, a
watch in useDialogSources that clears the source so the dialog is re-openable,
an async component in AppDialogs, and openDocs in ObjectBrowser. The context
entry is added at BOTH object menus — the plan named only the table one, but
views offer diagram.open too and documentation is no less relevant there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VTBWnT4goXLA9iqDrfTPqk

* docs(plan): record Tasks 1-10 as done

The plan carried 58 unticked boxes after ten completed tasks, so progress had
to be reconstructed from the commit trail instead of read off the document.

Tasks 1-7 are ticked from that commit evidence rather than from step-by-step
observation — they landed in earlier sessions. Tasks 8-10 were executed and
verified directly.

Task 8 Step 2 stays open on purpose. `columnsUsingEnum` was already implemented
and committed before that step was reached, so its failure was never observed;
the exact-match guard was verified by Step 5 instead, which is what exposed
that the test could not detect a substring match at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VTBWnT4goXLA9iqDrfTPqk

* feat(docs): print snapshot warnings as prose from the CLI

`dbx dbml` printed `{warning:?}`, so a skipped table surfaced as
`TableSkipped { table: "public.orders", reason: "permission denied" }` —
the struct shape, reading like a panic rather than like advice.

The prose lives in Rust rather than in the `docs.warnings` i18n namespace
because the CLI has no i18n runtime. That is the same constraint that made
`describeWarning` take a translator instead of importing vue-i18n: the
viewer translates, the CLI cannot, so each needs its own source for the
same text.

A second test asserts the rendering is not the Debug form, because
reverting the CLI to `{warning:?}` is a one-character edit that still
compiles and still prints something.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ro4mfGEmsbbH32WYsvxsfH

* feat(docs): let a connection point its notes file at a repository

`docs_notes_path` has existed on `ConnectionConfig` since Part 2 and has
been read by `resolve_notes_path` since Part 3b, but nothing ever set it —
so every connection silently used the app data directory default and the
override was unreachable.

The field is what makes schema documentation reviewable: pointing it at a
file inside a repository puts notes in the same diff as the migration that
changed the schema.

Gated on `isSchemaAware`, matching the row above it, since documentation is
a relational-only feature. A cleared field is normalised to absent rather
than "" — `resolve_notes_path` treats blank as unset, but an empty string
would still be persisted as though a path had been chosen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ro4mfGEmsbbH32WYsvxsfH

* docs: document database documentation and DBML export

Covers opening the viewer, notes and groups, the LOCAL/database-comment
rule, the notes file format and where it lives, every warning the viewer
can raise, and the `dbx dbml` verb including the CI drift check.

States the boundaries explicitly — relational engines only, no triggers or
procedures, and DBML export is one-way — because each of those is a
question the feature invites and would otherwise be answered by trying it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ro4mfGEmsbbH32WYsvxsfH

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: t8y2 <t8y2@users.noreply.github.com>
2026-08-07 03:35:19 +08:00

50 KiB

Database Documentation Part 2: Annotation Store — Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Let users author table notes, column notes, project notes and table groups in a git-committable file, and have dbx dbml --notes <path> merge them into the generated DBML.

Architecture: A new crates/dbx-core/src/docs/annotations.rs owns the notes file: load, validate, and merge into the SchemaSnapshot that Part 1's collector produces. Keys are schema.table / schema.table.column, case-folded per engine by a small docs/keys.rs. Merge precedence is local ?? database_comment, recorded in the note_source and shadowed_note fields Part 1 already defined but never populated. Notes whose target no longer exists are reported as SnapshotWarning::OrphanedNotes, never dropped.

Tech Stack: Rust (dbx-core, dbx-cli), serde, serde_json. No new dependencies.

Branch: continue on feature/docs-snapshot-dbml (Part 1's branch, 19 commits, unmerged) — Part 2 builds directly on its types. Worktree already exists at /Users/possebon/workspaces/dbx.feature-docs-snapshot-dbml.

Spec: docs/superpowers/specs/2026-08-02-database-documentation-dbml-design.md, Section 4.

Global Constraints

  • NO new dependencies. serde, serde_json, chrono, futures are already present in dbx-core.
  • cargo is NOT on PATH. Every cargo command must be prefixed with export PATH="$HOME/.rustup/toolchains/stable-aarch64-apple-darwin/bin:$PATH". A bare cargo fails instantly with "Failed to spawn process: No such file or directory (os error 2)" — that is not a hang.
  • A notes file that is ABSENT is fine; a notes file that is MALFORMED is a hard error. User prose is at stake; proceeding silently would render apparently-complete docs while discarding writing.
  • Never silently drop a note. A note whose target no longer exists becomes a warning, not a deletion.
  • format_version in the notes file is 1.
  • Group colour is hue: u16 in 0..=359. Never a hex string.
  • Run cargo fmt before every commit. Conventional Commits.
  • Report mismatches, do not work around them. If a signature differs from this plan, stop and report the actual compiler error. Part 1 had thirteen plan defects; every one was caught this way.

Pre-resolved facts (verified against the source — these override any assumption in the spec)

  1. schema/normalization.rs is NOT where key folding goes. Every function in it is pub(super) and it does table-list filtering (YashanDB recyclebin, object-type filters, column dedup) — not identifier case folding, and not reachable from docs::. Part 2 creates its own crates/dbx-core/src/docs/keys.rs.

  2. schema_diff::detect_renames cannot be used for orphan suggestions. Signature is pub fn detect_renames(removed: &[String], added: &[String], source_details: &[TableSchemaDetail], target_details: &[TableSchemaDetail], threshold: f64) -> Vec<RenameCandidate> (schema_diff.rs:817). It needs TableSchemaDetail for BOTH sides. The notes file stores prose only — it never retained the old schema — so there is nothing to diff against. Rename suggestions are deferred until snapshot history exists (spec Section 12). Part 2 detects and reports orphans; it does not suggest re-maps.

  3. Per-connection persistence. DesktopSettings.saved_sql_sync_dir is a GLOBAL setting, not per-connection — it is not the precedent the spec implies. ConnectionConfig already carries per-connection preferences (note, color, init_script, visible_schemas) and is serde-persisted into the connections table. Add docs_notes_path: Option<String> to ConnectionConfig with #[serde(default)] — non-breaking, no migration, no new table.

  4. MySQL lower_case_table_names is read in transfer.rs:705-727 via SHOW VARIABLES, but that helper is private and costs a round-trip. Part 2 does NOT query it. MySQL keys fold to lowercase unconditionally (see Task 2 rationale).

  5. Part 1 fields this plan populates (all exist, currently hardcoded None/NoteSource::None at collector.rs:206-209): DocTable::note, note_source, shadowed_note, column_notes, group_id, and SchemaSnapshot::groups. column_notes is already consumed by the serializer at dbml.rs:126; groups already render via render_group with an empty-member guard. No Part 1 type changes are required.

  6. Annotations are applied CLI-side, NOT inside the collector. The notes file lives on the machine running dbx, and WebBackend talks to a remote DBX instance that cannot read it. Applying the merge after the snapshot returns keeps both transports identical — a property a Part 1 review specifically verified. Adding an annotations field to CollectOptions would therefore create a field written by nothing and read by nothing, which is exactly the "produced but never consumed" defect the Part 1 final review flagged. collector.rs is deliberately not modified by this plan.


File Structure

File Responsibility
crates/dbx-core/src/docs/keys.rs Per-engine identifier folding; AnnotationKey construction and matching
crates/dbx-core/src/docs/annotations.rs Notes-file model, load/save, merge into snapshot, orphan detection
crates/dbx-core/src/models/connection.rs docs_notes_path field
crates/dbx-cli/src/main.rs --notes flag, wiring into run_dbml
crates/dbx-core/tests/live_postgres_docs_annotations.rs Live verification with a real notes file

Task 0: Confirm the starting point

  • Step 1: Verify the branch and a green baseline
cd /Users/possebon/workspaces/dbx.feature-docs-snapshot-dbml
git branch --show-current    # must print: feature/docs-snapshot-dbml
git status --short           # must be empty
export PATH="$HOME/.rustup/toolchains/stable-aarch64-apple-darwin/bin:$PATH"
cargo test -p dbx-core docs::

Expected: feature/docs-snapshot-dbml, clean tree, 60 tests passing. If any differs, stop and report — Part 2 builds on Part 1 and a dirty starting point makes every later failure ambiguous.


Task 1: Notes-file model

Files:

  • Create: crates/dbx-core/src/docs/annotations.rs
  • Modify: crates/dbx-core/src/docs/mod.rs

Interfaces:

  • Produces: AnnotationFile, ProjectAnnotation, TableAnnotation, ColumnAnnotation, GroupAnnotation

  • Step 1: Write the failing test

Create crates/dbx-core/src/docs/annotations.rs containing only this test module:

#[cfg(test)]
mod tests {
    use super::*;

    const SAMPLE: &str = r#"{
      "formatVersion": 1,
      "project": { "name": "Ecommerce", "note": "# Overview" },
      "groups": [
        { "id": "order-management", "name": "Order Management", "hue": 28, "note": "Checkout to handoff." }
      ],
      "tables": {
        "core.orders": {
          "group": "order-management",
          "note": "One row per checkout.",
          "columns": { "status": { "note": "State machine." } }
        }
      }
    }"#;

    #[test]
    fn parses_a_complete_notes_file() {
        let parsed: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        assert_eq!(parsed.format_version, 1);
        assert_eq!(parsed.project.as_ref().unwrap().name.as_deref(), Some("Ecommerce"));
        assert_eq!(parsed.groups.len(), 1);
        assert_eq!(parsed.groups[0].hue, 28);
        assert_eq!(parsed.tables.len(), 1);

        let orders = parsed.tables.get("core.orders").expect("orders");
        assert_eq!(orders.group.as_deref(), Some("order-management"));
        assert_eq!(orders.note.as_deref(), Some("One row per checkout."));
        assert_eq!(orders.columns.get("status").unwrap().note, "State machine.");
    }

    #[test]
    fn a_minimal_file_needs_only_the_format_version() {
        let parsed: AnnotationFile = serde_json::from_str(r#"{"formatVersion": 1}"#).expect("parse");
        assert!(parsed.tables.is_empty());
        assert!(parsed.groups.is_empty());
        assert!(parsed.project.is_none());
    }

    #[test]
    fn round_trips_through_json() {
        let parsed: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");
        let written = serde_json::to_string(&parsed).expect("serialize");
        let reparsed: AnnotationFile = serde_json::from_str(&written).expect("reparse");
        assert_eq!(reparsed.tables.len(), 1);
        assert_eq!(reparsed.groups[0].name, "Order Management");
    }

    #[test]
    fn rejects_a_file_with_an_unknown_top_level_field() {
        // Typos in a hand-edited file must not be silently ignored — a
        // misspelled "tabels" key would otherwise discard every note in it.
        let result: Result<AnnotationFile, _> =
            serde_json::from_str(r#"{"formatVersion": 1, "tabels": {}}"#);
        assert!(result.is_err(), "unknown fields must be rejected");
    }
}
  • Step 2: Run the test to verify it fails

Add to crates/dbx-core/src/docs/mod.rs, keeping alphabetical order (it currently lists color, collector, dbml, relations, snapshot):

pub mod annotations;

Run: cargo test -p dbx-core docs::annotations Expected: FAIL — cannot find type AnnotationFile.

  • Step 3: Write the types

At the top of crates/dbx-core/src/docs/annotations.rs:

use std::collections::BTreeMap;

use serde::{Deserialize, Serialize};

/// The on-disk notes file. This IS the store — not a cache of anything.
/// It is meant to be committed to a repository and reviewed in pull
/// requests, so it must stay small, readable, and stable in key order.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct AnnotationFile {
    pub format_version: u32,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub project: Option<ProjectAnnotation>,
    #[serde(default, skip_serializing_if = "Vec::is_empty")]
    pub groups: Vec<GroupAnnotation>,
    /// Keyed by `schema.table` (or bare `table` on schema-less engines).
    #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
    pub tables: BTreeMap<String, TableAnnotation>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct ProjectAnnotation {
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub name: Option<String>,
    /// Markdown. Becomes the documentation landing page.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub note: Option<String>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct GroupAnnotation {
    /// Stable slug, referenced by `TableAnnotation::group`.
    pub id: String,
    pub name: String,
    /// 0..=359. Lightness and chroma are theme-controlled, so any hue is
    /// legible on both light and dark grounds by construction.
    pub hue: u16,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub note: Option<String>,
}

#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct TableAnnotation {
    /// References `GroupAnnotation::id`.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub group: Option<String>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub note: Option<String>,
    /// Keyed by bare column name.
    #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
    pub columns: BTreeMap<String, ColumnAnnotation>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct ColumnAnnotation {
    pub note: String,
}

BTreeMap rather than HashMap is deliberate: it serializes in sorted key order, so a committed notes file produces a stable diff instead of a reshuffled one on every save.

  • Step 4: Run the tests

Run: cargo test -p dbx-core docs::annotations Expected: PASS, 4 tests.

  • Step 5: Commit
cargo fmt
git add crates/dbx-core/src/docs/
git commit -m "feat(docs): add annotation file model"

Task 2: Per-engine key folding

Files:

  • Create: crates/dbx-core/src/docs/keys.rs
  • Modify: crates/dbx-core/src/docs/mod.rs

Interfaces:

  • Consumes: crate::models::connection::DatabaseType
  • Produces: pub fn fold_identifier(db_type: DatabaseType, value: &str) -> String, pub fn table_key(db_type: DatabaseType, schema: Option<&str>, table: &str) -> String, pub fn column_key(db_type: DatabaseType, schema: Option<&str>, table: &str, column: &str) -> String

Why this module exists: the spec said folding belongs in schema/normalization.rs. It does not — everything there is pub(super) and it filters table lists rather than folding identifiers.

MySQL rationale: MySQL's real behaviour depends on the server's lower_case_table_names variable. transfer.rs:705 reads it via SHOW VARIABLES, but that helper is private and costs a round-trip per collection. Folding MySQL keys to lowercase unconditionally is correct for the common configurations (1 and 2) and, on the rare 0, only risks matching a note to a table differing solely by case — which is a matching hit, never a wrong note on an unrelated table. The cost is bounded and the round-trip is not.

  • Step 1: Write the failing test

Create crates/dbx-core/src/docs/keys.rs with only this test module:

#[cfg(test)]
mod tests {
    use super::*;
    use crate::models::connection::DatabaseType;

    #[test]
    fn postgres_folds_to_lowercase() {
        assert_eq!(fold_identifier(DatabaseType::Postgres, "Orders"), "orders");
        assert_eq!(fold_identifier(DatabaseType::Postgres, "ORDERS"), "orders");
    }

    #[test]
    fn mysql_folds_to_lowercase() {
        assert_eq!(fold_identifier(DatabaseType::MySQL, "Orders"), "orders");
    }

    #[test]
    fn table_keys_are_qualified_when_a_schema_is_present() {
        assert_eq!(table_key(DatabaseType::Postgres, Some("Core"), "Orders"), "core.orders");
        assert_eq!(table_key(DatabaseType::Postgres, None, "Orders"), "orders");
        assert_eq!(table_key(DatabaseType::Postgres, Some(""), "Orders"), "orders");
    }

    #[test]
    fn column_keys_extend_the_table_key() {
        assert_eq!(
            column_key(DatabaseType::Postgres, Some("core"), "orders", "Status"),
            "core.orders.status"
        );
    }

    #[test]
    fn folding_is_idempotent() {
        let once = table_key(DatabaseType::Postgres, Some("Core"), "Orders");
        let twice = table_key(DatabaseType::Postgres, Some(&once), "x");
        assert!(twice.starts_with("core.orders"), "got {twice}");
        assert_eq!(fold_identifier(DatabaseType::Postgres, &once), once);
    }
}
  • Step 2: Run the test to verify it fails

Add pub mod keys; to crates/dbx-core/src/docs/mod.rs (alphabetically, after dbml).

Run: cargo test -p dbx-core docs::keys Expected: FAIL — cannot find function fold_identifier.

Note: the exact DatabaseType variant spellings above (Postgres, MySQL) may differ. Check crates/dbx-core/src/models/connection.rs and use the real ones; report which you used.

  • Step 3: Write the implementation
use crate::models::connection::DatabaseType;

/// Fold an identifier to its canonical case for note matching.
///
/// PostgreSQL folds unquoted identifiers to lower case, Oracle to upper.
/// MySQL depends on the server's `lower_case_table_names`; we fold to lower
/// unconditionally rather than pay a `SHOW VARIABLES` round-trip per
/// collection — on the rare case-sensitive configuration the only risk is
/// matching a note to a table differing solely by case, never attaching a
/// note to an unrelated table.
pub fn fold_identifier(db_type: DatabaseType, value: &str) -> String {
    match db_type {
        DatabaseType::Oracle => value.to_uppercase(),
        _ => value.to_lowercase(),
    }
}

/// `schema.table`, or bare `table` when there is no schema.
pub fn table_key(db_type: DatabaseType, schema: Option<&str>, table: &str) -> String {
    match schema.filter(|value| !value.is_empty()) {
        Some(schema) => {
            format!("{}.{}", fold_identifier(db_type, schema), fold_identifier(db_type, table))
        }
        None => fold_identifier(db_type, table),
    }
}

/// `schema.table.column`, or `table.column` when there is no schema.
pub fn column_key(db_type: DatabaseType, schema: Option<&str>, table: &str, column: &str) -> String {
    format!("{}.{}", table_key(db_type, schema, table), fold_identifier(db_type, column))
}

Add pub use keys::{column_key, fold_identifier, table_key}; to docs/mod.rs.

  • Step 4: Run the tests

Run: cargo test -p dbx-core docs::keys Expected: PASS, 5 tests. Adjust the DatabaseType::Oracle arm if that variant is spelled differently.

  • Step 5: Commit
cargo fmt
git add crates/dbx-core/src/docs/
git commit -m "feat(docs): add per-engine annotation key folding"

Task 3: Load the notes file

Files:

  • Modify: crates/dbx-core/src/docs/annotations.rs

Interfaces:

  • Produces: pub fn load_annotations(path: &std::path::Path) -> Result<Option<AnnotationFile>, String>

The rule this task encodes: absent is fine, malformed is fatal.

  • Step 1: Write the failing test

Append to the tests module in annotations.rs:

    use std::io::Write;

    fn temp_notes(contents: &str) -> std::path::PathBuf {
        let mut path = std::env::temp_dir();
        path.push(format!("dbx-notes-test-{}.json", contents.len()));
        let mut file = std::fs::File::create(&path).expect("create");
        file.write_all(contents.as_bytes()).expect("write");
        path
    }

    #[test]
    fn an_absent_file_is_not_an_error() {
        let missing = std::path::Path::new("/nonexistent/dbx-notes-does-not-exist.json");
        assert!(matches!(load_annotations(missing), Ok(None)));
    }

    #[test]
    fn a_valid_file_loads() {
        let path = temp_notes(SAMPLE);
        let loaded = load_annotations(&path).expect("load").expect("some");
        assert_eq!(loaded.tables.len(), 1);
        let _ = std::fs::remove_file(&path);
    }

    #[test]
    fn a_malformed_file_is_a_hard_error_naming_the_path() {
        let path = temp_notes("{ this is not json");
        let error = load_annotations(&path).expect_err("must fail");
        assert!(error.contains(&path.display().to_string()), "error must name the file: {error}");
        let _ = std::fs::remove_file(&path);
    }

    #[test]
    fn an_unsupported_format_version_is_rejected() {
        let path = temp_notes(r#"{"formatVersion": 99}"#);
        let error = load_annotations(&path).expect_err("must fail");
        assert!(error.contains("99"), "error must name the version: {error}");
        let _ = std::fs::remove_file(&path);
    }
  • Step 2: Run the test to verify it fails

Run: cargo test -p dbx-core docs::annotations Expected: FAIL — cannot find function load_annotations.

  • Step 3: Write the implementation
use std::path::Path;

/// The only format version this build understands.
pub const ANNOTATION_FORMAT_VERSION: u32 = 1;

/// Load the notes file.
///
/// An ABSENT file returns `Ok(None)` — that is the normal first-run and
/// first-CI-run state. A MALFORMED file is a hard error: someone's prose is
/// in there, and rendering apparently-complete documentation while silently
/// discarding it is worse than failing.
pub fn load_annotations(path: &Path) -> Result<Option<AnnotationFile>, String> {
    let contents = match std::fs::read_to_string(path) {
        Ok(contents) => contents,
        Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
        Err(error) => return Err(format!("Failed to read notes file {}: {error}", path.display())),
    };

    let parsed: AnnotationFile = serde_json::from_str(&contents)
        .map_err(|error| format!("Failed to parse notes file {}: {error}", path.display()))?;

    if parsed.format_version != ANNOTATION_FORMAT_VERSION {
        return Err(format!(
            "Notes file {} has formatVersion {}, but this build understands {}.",
            path.display(),
            parsed.format_version,
            ANNOTATION_FORMAT_VERSION
        ));
    }

    Ok(Some(parsed))
}
  • Step 4: Run the tests

Run: cargo test -p dbx-core docs::annotations Expected: PASS, 8 tests.

  • Step 5: Commit
cargo fmt
git add crates/dbx-core/src/docs/
git commit -m "feat(docs): load and validate the notes file"

Task 4: Merge annotations into the snapshot

Files:

  • Modify: crates/dbx-core/src/docs/annotations.rs

Interfaces:

  • Consumes: AnnotationFile (Task 1), table_key/column_key (Task 2), SchemaSnapshot/DocTable/ColumnNote/NoteSource/TableGroup/SnapshotWarning (Part 1)
  • Produces: pub fn apply_annotations(snapshot: &mut SchemaSnapshot, annotations: &AnnotationFile, db_type: DatabaseType)

Merge precedence: local_annotation ?? database_comment. When a local note shadows a database comment, the comment is preserved in shadowed_note so the UI can show what is being overridden — otherwise a DBA later writing a better COMMENT ON would have their improvement silently hidden.

  • Step 1: Write the failing test

Append to the tests module:

    use crate::docs::{DocTable, NoteSource, ProjectMeta, SchemaSnapshot, TableKind};
    use crate::models::connection::DatabaseType;

    fn snapshot_with(tables: Vec<DocTable>) -> SchemaSnapshot {
        SchemaSnapshot {
            format_version: 1,
            project: ProjectMeta {
                name: "conn".to_string(),
                database_type: "postgres".to_string(),
                database: None,
                schemas: vec!["core".to_string()],
                generated_at: String::new(),
                note: None,
            },
            tables,
            relationships: vec![],
            groups: vec![],
            enums: vec![],
            warnings: vec![],
        }
    }

    fn table_named(schema: &str, name: &str, comment: Option<&str>) -> DocTable {
        DocTable {
            schema: Some(schema.to_string()),
            name: name.to_string(),
            kind: TableKind::Table,
            columns: vec![],
            indexes: vec![],
            foreign_keys: vec![],
            group_id: None,
            note: comment.map(ToOwned::to_owned),
            note_source: if comment.is_some() { NoteSource::Database } else { NoteSource::None },
            shadowed_note: None,
            column_notes: BTreeMap::new(),
            estimated_rows: None,
            view_definition: None,
        }
    }

    #[test]
    fn a_local_note_shadows_the_database_comment_and_preserves_it() {
        let mut snapshot = snapshot_with(vec![table_named("core", "orders", Some("Old DB comment."))]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);

        let table = &snapshot.tables[0];
        assert_eq!(table.note.as_deref(), Some("One row per checkout."));
        assert_eq!(table.note_source, NoteSource::Local);
        assert_eq!(table.shadowed_note.as_deref(), Some("Old DB comment."));
    }

    #[test]
    fn a_database_comment_survives_when_there_is_no_local_note() {
        let mut snapshot = snapshot_with(vec![table_named("core", "users", Some("From the database."))]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);

        let table = &snapshot.tables[0];
        assert_eq!(table.note.as_deref(), Some("From the database."));
        assert_eq!(table.note_source, NoteSource::Database);
        assert_eq!(table.shadowed_note, None);
    }

    #[test]
    fn keys_match_case_insensitively_on_postgres() {
        // The notes file says "core.orders"; the live schema reports "Core"/"Orders".
        let mut snapshot = snapshot_with(vec![table_named("Core", "Orders", None)]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);

        assert_eq!(snapshot.tables[0].note.as_deref(), Some("One row per checkout."));
    }

    #[test]
    fn column_notes_are_applied_and_marked_local() {
        let mut table = table_named("core", "orders", None);
        table.columns.push(crate::types::ColumnInfo {
            name: "status".to_string(),
            data_type: "text".to_string(),
            ..Default::default()
        });
        let mut snapshot = snapshot_with(vec![table]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);

        let note = snapshot.tables[0].column_notes.get("status").expect("column note");
        assert_eq!(note.note, "State machine.");
        assert_eq!(note.source, NoteSource::Local);
    }

    #[test]
    fn the_project_note_and_name_are_applied() {
        let mut snapshot = snapshot_with(vec![]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);

        assert_eq!(snapshot.project.name, "Ecommerce");
        assert_eq!(snapshot.project.note.as_deref(), Some("# Overview"));
    }

    #[test]
    fn groups_are_copied_and_membership_is_assigned() {
        let mut snapshot = snapshot_with(vec![table_named("core", "orders", None)]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);

        assert_eq!(snapshot.groups.len(), 1);
        assert_eq!(snapshot.groups[0].id, "order-management");
        assert_eq!(snapshot.groups[0].hue, 28);
        assert_eq!(snapshot.tables[0].group_id.as_deref(), Some("order-management"));
    }

    #[test]
    fn a_table_referencing_an_undefined_group_is_left_ungrouped() {
        let mut snapshot = snapshot_with(vec![table_named("core", "orders", None)]);
        let mut annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");
        annotations.groups.clear();

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);

        assert_eq!(snapshot.tables[0].group_id, None, "a dangling group reference must not be assigned");
    }
  • Step 2: Run the test to verify it fails

Run: cargo test -p dbx-core docs::annotations Expected: FAIL — cannot find function apply_annotations.

  • Step 3: Write the implementation
use crate::docs::{ColumnNote, NoteSource, SchemaSnapshot, TableGroup};
use crate::docs::keys::{column_key, table_key};
use crate::models::connection::DatabaseType;

/// Merge a notes file into a collected snapshot.
///
/// Precedence is `local ?? database_comment`. When a local note shadows a
/// database comment the comment is kept in `shadowed_note`, so a later
/// `COMMENT ON` improvement stays visible rather than being silently hidden.
pub fn apply_annotations(
    snapshot: &mut SchemaSnapshot,
    annotations: &AnnotationFile,
    db_type: DatabaseType,
) {
    if let Some(project) = annotations.project.as_ref() {
        if let Some(name) = project.name.as_deref().filter(|value| !value.trim().is_empty()) {
            snapshot.project.name = name.to_string();
        }
        if project.note.is_some() {
            snapshot.project.note = project.note.clone();
        }
    }

    let known_groups: std::collections::HashSet<&str> =
        annotations.groups.iter().map(|group| group.id.as_str()).collect();

    snapshot.groups = annotations
        .groups
        .iter()
        .map(|group| TableGroup {
            id: group.id.clone(),
            name: group.name.clone(),
            hue: group.hue,
            note: group.note.clone(),
        })
        .collect();

    for table in &mut snapshot.tables {
        let key = table_key(db_type, table.schema.as_deref(), &table.name);
        let Some(annotation) = annotations.tables.get(&key) else { continue };

        if let Some(note) = annotation.note.as_deref().filter(|value| !value.trim().is_empty()) {
            // Preserve whatever the database said before overwriting it.
            if matches!(table.note_source, NoteSource::Database) {
                table.shadowed_note = table.note.clone();
            }
            table.note = Some(note.to_string());
            table.note_source = NoteSource::Local;
        }

        // A group reference that names no defined group is dropped rather
        // than assigned — a dangling id would render an empty group header.
        table.group_id = annotation
            .group
            .as_deref()
            .filter(|id| known_groups.contains(id))
            .map(ToOwned::to_owned);

        for column in &table.columns {
            let column_fold = crate::docs::keys::fold_identifier(db_type, &column.name);
            let annotated = annotation
                .columns
                .iter()
                .find(|(name, _)| crate::docs::keys::fold_identifier(db_type, name) == column_fold);

            let Some((_, column_annotation)) = annotated else { continue };
            if column_annotation.note.trim().is_empty() {
                continue;
            }

            table.column_notes.insert(
                column.name.clone(),
                ColumnNote {
                    note: column_annotation.note.clone(),
                    source: NoteSource::Local,
                    shadowed: column.comment.clone().filter(|value| !value.trim().is_empty()),
                },
            );
        }
    }

    // `column_key` is used by orphan detection in Task 5; referenced here so
    // the import is meaningful in both tasks.
    let _ = column_key;
}

Note for the implementer: the trailing let _ = column_key; is a placeholder to keep the import valid before Task 5 uses it. If the compiler is satisfied without importing column_key in this task, omit both the import and that line and add the import in Task 5 instead. Report which you did.

  • Step 4: Run the tests

Run: cargo test -p dbx-core docs::annotations Expected: PASS, 15 tests.

  • Step 5: Commit
cargo fmt
git add crates/dbx-core/src/docs/
git commit -m "feat(docs): merge annotations into the schema snapshot"

Task 5: Orphan detection

Files:

  • Modify: crates/dbx-core/src/docs/annotations.rs

Interfaces:

  • Produces: pub fn detect_orphans(snapshot: &SchemaSnapshot, annotations: &AnnotationFile, db_type: DatabaseType) -> Vec<String>; apply_annotations pushes SnapshotWarning::OrphanedNotes { count }

Why no re-map suggestion: schema_diff::detect_renames needs TableSchemaDetail for both the old and new schema. The notes file stores prose only — it never retained the old schema — so there is nothing to diff. Suggestions become possible once snapshot history exists (spec Section 12). Part 2 detects and reports; it never deletes.

  • Step 1: Write the failing test

Append to the tests module:

    use crate::docs::SnapshotWarning;

    #[test]
    fn a_note_for_a_missing_table_is_reported_as_orphaned() {
        // The notes file describes core.orders; the schema no longer has it.
        let mut snapshot = snapshot_with(vec![table_named("core", "customers", None)]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        let orphans = detect_orphans(&snapshot, &annotations, DatabaseType::Postgres);
        assert_eq!(orphans, vec!["core.orders".to_string()]);

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);
        let orphan_warnings: Vec<&SnapshotWarning> = snapshot
            .warnings
            .iter()
            .filter(|warning| matches!(warning, SnapshotWarning::OrphanedNotes { .. }))
            .collect();
        assert_eq!(orphan_warnings.len(), 1);
        match orphan_warnings[0] {
            SnapshotWarning::OrphanedNotes { count } => assert_eq!(*count, 1),
            other => panic!("unexpected: {other:?}"),
        }
    }

    #[test]
    fn a_note_for_a_missing_column_is_reported_as_orphaned() {
        // The table exists but no longer has the annotated column.
        let mut table = table_named("core", "orders", None);
        table.columns.push(crate::types::ColumnInfo {
            name: "id".to_string(),
            data_type: "integer".to_string(),
            ..Default::default()
        });
        let snapshot = snapshot_with(vec![table]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        let orphans = detect_orphans(&snapshot, &annotations, DatabaseType::Postgres);
        assert_eq!(orphans, vec!["core.orders.status".to_string()]);
    }

    #[test]
    fn nothing_is_orphaned_when_everything_matches() {
        let mut table = table_named("core", "orders", None);
        table.columns.push(crate::types::ColumnInfo {
            name: "status".to_string(),
            data_type: "text".to_string(),
            ..Default::default()
        });
        let mut snapshot = snapshot_with(vec![table]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");

        assert!(detect_orphans(&snapshot, &annotations, DatabaseType::Postgres).is_empty());

        apply_annotations(&mut snapshot, &annotations, DatabaseType::Postgres);
        assert!(
            !snapshot.warnings.iter().any(|w| matches!(w, SnapshotWarning::OrphanedNotes { .. })),
            "no orphan warning when everything matches"
        );
    }

    #[test]
    fn orphan_detection_never_removes_anything_from_the_file() {
        let snapshot = snapshot_with(vec![]);
        let annotations: AnnotationFile = serde_json::from_str(SAMPLE).expect("parse");
        let before = annotations.tables.len();

        let _ = detect_orphans(&snapshot, &annotations, DatabaseType::Postgres);

        assert_eq!(annotations.tables.len(), before, "detection must not mutate the file");
    }
  • Step 2: Run the test to verify it fails

Run: cargo test -p dbx-core docs::annotations Expected: FAIL — cannot find function detect_orphans.

  • Step 3: Write the implementation
/// Annotation keys whose target no longer exists in the collected schema.
///
/// Returns fully-qualified keys, sorted, so the caller can list them for a
/// human to re-map. This function NEVER mutates the notes file — user prose
/// is only ever removed by an explicit human action.
///
/// Suggestions for where a renamed target went are deliberately absent:
/// producing them requires the OLD schema to diff against, and the notes
/// file stores prose only. That becomes possible once snapshot history
/// exists (see the spec's deferred versioning seam).
pub fn detect_orphans(
    snapshot: &SchemaSnapshot,
    annotations: &AnnotationFile,
    db_type: DatabaseType,
) -> Vec<String> {
    use std::collections::HashSet;

    let live_tables: HashSet<String> = snapshot
        .tables
        .iter()
        .map(|table| table_key(db_type, table.schema.as_deref(), &table.name))
        .collect();

    let live_columns: HashSet<String> = snapshot
        .tables
        .iter()
        .flat_map(|table| {
            table.columns.iter().map(move |column| {
                column_key(db_type, table.schema.as_deref(), &table.name, &column.name)
            })
        })
        .collect();

    let mut orphans = Vec::new();

    for (key, annotation) in &annotations.tables {
        let folded_table = fold_key(db_type, key);
        if !live_tables.contains(&folded_table) {
            orphans.push(folded_table);
            continue;
        }
        for column in annotation.columns.keys() {
            let folded_column =
                format!("{folded_table}.{}", crate::docs::keys::fold_identifier(db_type, column));
            if !live_columns.contains(&folded_column) {
                orphans.push(folded_column);
            }
        }
    }

    orphans.sort();
    orphans
}

/// Fold an already-dotted key (e.g. `Core.Orders`) segment by segment.
fn fold_key(db_type: DatabaseType, key: &str) -> String {
    key.split('.')
        .map(|segment| crate::docs::keys::fold_identifier(db_type, segment))
        .collect::<Vec<_>>()
        .join(".")
}

Then, at the END of apply_annotations (replacing the let _ = column_key; placeholder from Task 4):

    let orphans = detect_orphans(snapshot, annotations, db_type);
    if !orphans.is_empty() {
        snapshot.warnings.push(SnapshotWarning::OrphanedNotes { count: orphans.len() });
    }

Add SnapshotWarning to the use crate::docs::{...} line.

  • Step 4: Run the tests

Run: cargo test -p dbx-core docs::annotations Expected: PASS, 19 tests.

  • Step 5: Commit
cargo fmt
git add crates/dbx-core/src/docs/
git commit -m "feat(docs): detect orphaned annotations without deleting them"

Task 6: --notes CLI flag

Files:

  • Modify: crates/dbx-cli/src/main.rs

Interfaces:

  • Consumes: load_annotations (Task 3), apply_annotations (Task 4)

Note: DocsSnapshotOptions in dbx_mcp::backend carries options across the backend boundary. It does NOT need an annotations field — the CLI loads the file locally and, for LocalBackend, the annotations reach the collector directly. For WebBackend the notes file lives on the CLI's machine, not the server's, so annotations must be applied CLI-side after the snapshot returns. Implement it that way: load the file in run_dbml, and apply it to the returned snapshot with apply_annotations before calling to_dbml. This keeps both transports identical, which a Part 1 review specifically verified.

  • Step 1: Write the failing test

Append to main.rs's test module:

    #[test]
    fn parses_the_notes_flag() {
        let flags = parse_flags(&args(&["dbml", "local", "--notes", "docs/dbx-docs.json"])).expect("parse");
        assert_eq!(flags.args, args(&["dbml", "local"]));
        assert_eq!(flags.notes.as_deref(), Some(std::path::Path::new("docs/dbx-docs.json")));
    }

    #[test]
    fn notes_requires_a_value() {
        let error = parse_flags(&args(&["dbml", "local", "--notes"])).expect_err("should fail");
        assert_eq!(error.code, "INVALID_OPTION");
    }

    #[test]
    fn notes_appears_in_the_usage_text() {
        assert!(usage().contains("--notes"), "got: {}", usage());
    }
  • Step 2: Run the test to verify it fails

Run: cargo test -p dbx-cli Expected: FAIL — no field named notes on Flags.

  • Step 3: Write the implementation

  • Add notes: Option<PathBuf> to Flags (NO pub — the struct and all its fields are private) and initialise it to None in parse_flags.

  • Add the parse arm beside --out:

    "--notes" => flags.notes = Some(PathBuf::from(option_value(argv, &mut index, "--notes")?)),
    
  • In run_dbml, after obtaining the snapshot and before to_dbml:

    let mut snapshot = snapshot;
    if let Some(path) = flags.notes.as_ref() {
        if let Some(annotations) = dbx_core::docs::annotations::load_annotations(path)
            .map_err(|error| CliError::new("NOTES_INVALID", error))?
        {
            dbx_core::docs::annotations::apply_annotations(
                &mut snapshot,
                &annotations,
                connection.db_type,
            );
        }
    }
    

    A malformed notes file must abort with a non-zero exit — it must NOT fall through to producing documentation without the prose.

  • Extend the dbx dbml line inside the single usage() &'static str literal so it reads:

      dbx dbml <connection> [--out path] [--notes path] [--schema name] [--database name] [--tables a,b]
    
  • Step 4: Run the tests

Run: cargo test -p dbx-cli Expected: PASS.

  • Step 5: Commit
cargo fmt
git add crates/dbx-cli/src/main.rs
git commit -m "feat(docs): add --notes to dbx dbml"

Task 7: docs_notes_path on ConnectionConfig

Files:

  • Modify: crates/dbx-core/src/models/connection.rs

Interfaces:

  • Produces: ConnectionConfig.docs_notes_path: Option<String>

Why here rather than a new table: ConnectionConfig already carries per-connection preferences (note, color, init_script, visible_schemas) and is serde-persisted into the connections table. DesktopSettings.saved_sql_sync_dir is a GLOBAL setting and is not the right precedent despite what the spec implies. #[serde(default)] makes this non-breaking for existing stored connections — no migration.

This field is not consumed by anything in Part 2 (the CLI takes an explicit --notes path). It exists so Part 3's desktop dialog has somewhere to remember the path per connection.

  • Step 1: Write the failing test

Append to connection.rs's test module (or create one following the file's conventions):

    #[test]
    fn docs_notes_path_defaults_to_none_for_existing_stored_connections() {
        // A connection persisted before this field existed must still load.
        let json = r#"{
            "id": "c1", "name": "local", "note": "", "db_type": "Postgres",
            "host": "127.0.0.1", "port": 5432, "username": "postgres",
            "password": "", "attached_databases": [], "agent_java_options": [],
            "connect_timeout_secs": 10, "transport_layers": []
        }"#;
        let parsed: ConnectionConfig = serde_json::from_str(json).expect("parse");
        assert_eq!(parsed.docs_notes_path, None);
    }

The exact JSON must satisfy every non-default field on ConnectionConfig. Run the test, read the serde error, and add whatever fields it names — do NOT add #[serde(default)] to other fields to make it pass.

  • Step 2: Run the test to verify it fails

Run: cargo test -p dbx-core docs_notes_path Expected: FAIL — no field docs_notes_path.

  • Step 3: Write the implementation

Add to ConnectionConfig, beside init_script / color:

    /// Path to this connection's documentation notes file. Set by the
    /// desktop app; the CLI takes an explicit `--notes` path instead.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub docs_notes_path: Option<String>,

Then fix every ConnectionConfig { .. } literal the compiler flags. There are many (including crates/dbx-core/tests/live_postgres_query_result_export.rs:12); add docs_notes_path: None to each. Let the compiler enumerate them; report the count.

  • Step 4: Run the tests
cargo test -p dbx-core
cargo check --workspace --all-targets

Expected: pass and clean.

  • Step 5: Commit
cargo fmt
git add -A
git commit -m "feat(docs): remember a notes file path per connection"

Task 8: Live verification

Files:

  • Create: crates/dbx-core/tests/live_postgres_docs_annotations.rs

Follow the gating convention of crates/dbx-core/tests/live_postgres_docs_snapshot.rs exactly: #[tokio::test] + #[ignore = "requires DBX_LIVE_POSTGRES_..."], env vars with defaults, and the live_postgres_config(...) builder. Do NOT invent a skip-helper — #[ignore] is what keeps the test from silently passing with no database.

  • Step 1: Write the live test

The test must: write a temporary notes file annotating a table that genuinely exists in keycloak plus one that does not; collect a snapshot with those annotations; then assert

  • the real table's note is present with note_source == NoteSource::Local,
  • an OrphanedNotes warning is present with count >= 1,
  • the generated DBML contains the note text,
  • and the temp file is removed on both success and failure paths.

Print the generated DBML with println! so --nocapture shows it.

  • Step 2: Run it against the live database
export PATH="$HOME/.rustup/toolchains/stable-aarch64-apple-darwin/bin:$PATH"
DBX_LIVE_POSTGRES_HOST=127.0.0.1 DBX_LIVE_POSTGRES_PORT=5432 \
DBX_LIVE_POSTGRES_USER=postgres DBX_LIVE_POSTGRES_PASSWORD=postgres \
DBX_LIVE_POSTGRES_DATABASE=keycloak \
cargo test -p dbx-core --test live_postgres_docs_annotations -- --ignored --nocapture

Paste the ACTUAL generated DBML into the report — specifically a Table block showing the merged note and the Note: line inside it.

  • Step 3: Full sweep
cargo test --workspace
cargo clippy -p dbx-core -- -D warnings

Report clippy honestly. crates/dbx-core/src/table_structure_sql/triggers.rs:58 fails on map_or — that is PRE-EXISTING on main, verified byte-identical, and is NOT yours to fix. Confirm it is the only remaining lint.

  • Step 4: Commit
cargo fmt
git add crates/dbx-core/tests/
git commit -m "test(docs): verify annotations against a live database"

Done criteria

  • dbx dbml <connection> --notes docs/dbx-docs.json merges notes and groups into the emitted DBML.
  • An absent notes file is fine; a malformed one aborts with a non-zero exit naming the file.
  • A note whose target no longer exists produces an OrphanedNotes warning on stderr and is never deleted.
  • A local note shadowing a database comment preserves the comment in shadowed_note.
  • Table groups become TableGroup blocks with [color: #rrggbb] derived from each group's hue.
  • cargo test --workspace passes; clippy clean except the pre-existing triggers.rs lint.

Deferred to Part 3

  • The docs viewer, in-app editing dialog, group colour picker, and HTML export.
  • Rename suggestions for orphaned notes (needs snapshot history — spec Section 12).
  • Writing the notes file from DBX (Part 2 only reads it; the file is hand-edited or repo-managed).

Appendix: Corrections found during execution

Defects in this plan's own text, found while executing it. Grouped by failure mode rather than by task, because the modes repeat and the tasks do not. Every one originated in the plan; none was an implementer error. The companion plan for the viewer carries its own appendix in the same form.

Mode A — compiles, passes its own test, and is silently wrong forever

The most dangerous defect found anywhere in this feature, and the only one that would have shipped undetected.

  • Task 7, docs_notes_path. The plan added the field to ConnectionConfig and to its own test, which passed. But ConnectionConfig is not what deserialization reads — a mirror struct, ConnectionConfigData, carries the serde attributes, and a From impl copies fields across. Adding the field in one place compiles cleanly, satisfies a round-trip test written against the same struct, and then reads None forever after every load, because nothing ever populates it. Caught by pre-resolution, not by review: the field had to be added in three places, and only reading the deserialization path revealed the third.

The lesson: when a struct has a serde mirror, "it compiles and the test passes" is evidence about the wrong object. Follow the value from the file on disk to the field being read.

Mode B — an assertion too weak to detect the failure it names

Same failure mode as Mode 1 in the viewer plan's appendix, and the most common across both.

  • Task 1, round_trips_through_json. Asserted only tables.len() and a single group name. A serializer that silently dropped every note, hue, and column annotation would pass it. Fixed by asserting the full round-tripped structure.
  • Task 5. Each headline test individually survived a mutant; only the pair caught it. Accepted rather than fixed, but recorded, because a later edit that deletes one test would silently remove the coverage the other depends on.

Mode C — plan text written from memory instead of from the code

Four instances, all caught before dispatch by reading the actual source. None would have survived compilation, so the cost was implementer time rather than correctness — but that cost is real.

  • DatabaseType::Mysql, which the plan spelled MySQL.
  • A borrow hazard in a code block the plan supplied verbatim (Task 4).
  • let mut snapshot = snapshot; in the run_dbml shape, which was a guess at the real signature.
  • capabilities_for is pub(super), not pub, and AppState's module path was wrong.

The lesson: every identifier a plan names is a claim about the codebase. Checking them costs minutes; a dispatch that fails on one costs an implementer round-trip.

Mode D — matching logic that attaches data to the wrong object

  • Task 2, identifier folding. The original key-matching rule could fold two genuinely distinct identifiers to the same key, so a note written for one table would attach to another. The doc comment's stated MySQL rationale also described behaviour the code did not implement. Fixed by making fold_identifier engine-specific (Oracle uppercases, ClickHouse and MongoDB preserve, everything else lowercases) and correcting the comment to match.

A related seam was investigated and cleared: Oracle folding is used only for matching, never for output — the serializer emits the collector's reported name verbatim, so folding cannot corrupt what is written.

Mode E — a test helper that is itself flaky

  • Task 3. The plan's own test helper had a race that would produce intermittent failures. Caught before dispatch. A flaky test in a new suite is worse than no test: it trains the reader to re-run rather than to read.

A controller hypothesis that was wrong

Recorded because being wrong in this direction is cheap and worth doing more often. I suspected a second apply_annotations call would overwrite shadowed_note with the first local note and lose the database comment. Tracing it showed the guard simply does not fire on the second call, so shadowed_note keeps its first-call value. No defect existed. The check cost one read.