feat(f38): data dictionary and schema documentation - #41
Conversation
Data dictionary documenting what each column MEANS, keyed by the F31 stable column ID. New dictionary.rs owns the model (DictionaryField: displayName/description/role/unit/source/sensitivity/allowedValues/ example/owner/notes), the per-document container, the editor view (technical names + inferred F31 types prefilled), versioned JSON / Markdown / CSV export, and the import merge engine (match by column ID or mapped name; field-level conflict report requiring explicit per-field or take-all resolution). Stored on Document beside the schema with its own dictionaryRevision: edits never rewrite a cell or dirty the source. Column IDs are stable, so renames preserve entries and deletes report orphans (kept until explicitly discarded; undo re-attaches). Dictionary carries across reparse/reindex like the schema. Hooks: F08 profiles gain required_documentation rules that surface as missingDocumentation validation issues; the F28 PII scan preflight folds in confidential/restricted columns even without pattern hits. Commands (revision-guarded, atomic export writes): get_dictionary, set/remove_dictionary_field, discard_dictionary_orphans, export_dictionary, preview/apply_dictionary_import. Tests: merge matrix (id/name/conflict/keep/take), orphan+undo flow, rename survival, no-dirty guarantee, MD/CSV/JSON completeness + round-trip, profile enforcement, PII sensitivity hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Searchable DictionaryDialog (all DictionaryField editors incl. allowedValues chips, schema-prefilled technical name + inferred type, per-column completeness indicator), Grid column-header tooltips (display name / description / unit / sensitivity badge), and the import flow with a field-level ConflictDialog (existing-vs-incoming per-field choices, keep-all / take-all shortcuts, apply gated until every conflict is resolved). Adds JSON/Markdown/CSV export menu entries, the "Document column..." header-menu item, command-palette entries, the store slice (revision-guarded metadata edits), and TS mirror types + typed invoke wrappers. Surfaces the dictionary sensitivity flags in the PII dialog. Vitest covers the pure logic (completeness calc, conflict reduction). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Finalize the F38 data dictionary stage. All gates green: npm (lint, format:check, typecheck, 200 tests, build) and rust (fmt --check, clippy -D warnings, 503 lib tests). Fix a stray NUL byte embedded in src/lib/dictionary.ts conflictKey(), which made git treat the file as binary. Restored as an explicit U+0000 escape sequence: byte-identical runtime map-key behaviour, but valid UTF-8 text source. cargo fmt made no Rust changes. The Unreleased CHANGELOG entry and the README "Navigate & analyze" bullet for the data dictionary were already consolidated and prettier-clean from the prior stages; no further doc edits needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Thread the merge plan's dictionary revision through the import apply flow so
the stale-plan guard actually fires. applyDictionaryImport was echoing the
live cached view revision instead of the revision the plan was computed
against, so a documentation edit saved after the preview (which bumps the
dictionary revision) passed the guard and was silently overwritten by the
stale resolution. The dialog now passes MergePlan.dictionaryRevision; the
store action takes the expected revision as a parameter.
Report ambiguous by-name import matches instead of silently attributing
documentation to the first same-named column. Headers are not unique (a
source CSV or an in-app rename can duplicate one), so resolve_target now
detects when a name matches more than one column and leaves the entry
unmatched with a reason ("import by column ID"), never collapsing it onto
the first. documentation_gaps likewise now flags every column sharing a
required-doc name, deduped.
Tests: +3 Rust merge cases (ambiguous duplicate/case-variant headers, Auto
uses ID even when name is ambiguous), +1 Rust documentation_gaps case, +1
frontend store test locking the plan-time revision through apply.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rebase feat/f38-dictionary onto feat/f37-projects and wire the data dictionary into the F37 ProjectStore as a real registered section. - Register the previously-reserved `dictionary` section: typed `Vec<SourceDictionary>` (per-source, versioned `DictionaryExport` envelope), a `set_section_typed` arm, and `reserved: false` in the registry, so a project saves/loads each document's dictionary and it round-trips through the atomic save pipeline like schemas and views. - Documentation is configuration only (column IDs + metadata, never a cell value), so it passes the no-cell-data scan; add a per-source round-trip test proving accept-not-reject, verbatim reload, and a clean data-key scan, and cover it in the "every section" round-trip. - Fix integration fallout from the sibling f38 engine change: the F37 `a_profile()` test helper now sets the new `FileProfile` `required_documentation` field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff1b6e0ed6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// only — column IDs and metadata, never cell values). | ||
| #[serde(default)] | ||
| pub dictionary: Vec<Value>, | ||
| pub dictionary: Vec<SourceDictionary>, |
There was a problem hiding this comment.
This adds a per-source dictionary section, but the source cleanup paths still only prune views/schemas/row keys: remove_source and strip_sources never touch sections.dictionary. When a source is removed while opening a project, or when saving/creating a template, the project file keeps SourceDictionary entries whose sourceId no longer exists, leaving stale column documentation in subsequent saves/templates instead of dropping everything that referenced that source.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in b9898be. remove_source now retains sections.dictionary by source_id alongside views/schemas/row-keys, and strip_sources clears it, so neither a source removal on open nor a saved template / new-from-template carries SourceDictionary entries for sources that no longer exist. Covered by a new removing_a_source_prunes_its_dictionary_section test (removed source's dictionary dropped, sibling's kept) plus new assertions in templates_strip_sources_and_initialize_a_repeatable_workflow (template file serializes "dictionary": []) and new_project_from_a_full_project_also_strips_sources.
| // Guard with the revision captured when this plan was previewed, so an edit | ||
| // saved after the preview (which moves the dictionary revision) rejects this | ||
| // now-stale apply rather than silently discarding that edit. | ||
| const outcome = await applyImport(importPath, matchBy, resolution, plan.dictionaryRevision); |
There was a problem hiding this comment.
plan already records the matchBy used for the preview, but apply re-reads the mutable dialog state. If the user changes the Match by selector while a preview is still in flight, an older preview can populate plan after matchBy has changed; pressing Apply then reruns the merge under a different mode than the conflicts/counts the user reviewed, potentially applying documentation to a different set of columns. Pass plan.matchBy here and ignore stale preview results so the apply matches the displayed plan.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in b9898be. finishImport now applies under plan.matchBy — the mode the displayed plan was actually computed with (the backend echoes the requested mode into the plan verbatim) — via a new pure applyMatchBy(plan) helper in src/lib/dictionary.ts, so the apply can no longer diverge from the reviewed conflicts/counts; it pairs with the existing plan.dictionaryRevision guard. Also added the stale-preview guard you suggested: runPreview is sequence-numbered and a result that resolves after a newer request (or after Cancel) is dropped instead of populating plan. The threading is vitest-covered in src/lib/dictionary.test.ts (applyMatchBy returns the plan's recorded mode for all three modes and structurally cannot read the live selection).
…val/templates; apply imports under the previewed match mode Two P2 review findings on PR #41: - project.rs: `remove_source` and `strip_sources` pruned views/schemas/ row-keys/joins/comparisons but never `sections.dictionary`, so removing a source (or saving a template / new-from-template) kept SourceDictionary entries whose sourceId no longer exists — templates carried source-specific metadata. Both paths now retain/clear the dictionary section like every other per-source section. Covered by a new `removing_a_source_prunes_its_dictionary_section` test plus assertions in the template-strip and new-from-project tests. - DictionaryDialog: apply re-read the live `Match by` state instead of the mode the displayed plan was previewed under, so changing the selector while a preview was in flight could apply the merge under a different mode than the reviewed conflicts/counts. The apply now threads `plan.matchBy` via a new pure `applyMatchBy` helper (vitest-covered), and previews are sequence-guarded so a stale in-flight preview result can never populate (or resurrect) the panel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # src/store/useStore.test.ts # src/types.ts
1 participant
STACKED PR — review the delta vs
feat/f37-projectsonlyThis branch is stacked on
feat/f37-projects(which is itself stacked onfeat/f33-json→feat/infra-tabular). Base isfeat/f37-projects, soGitHub shows only F38's five commits; review just that delta. Merge the parents
first.
feat/f37-projectsis at parity withorigin(4841085).Summary
F38 adds a data dictionary — per-column documentation of what each column
means, as a layer of pure metadata on top of the data:
role, unit, source, sensitivity, allowed values, example, owner, notes),
keyed by its stable column ID, so the documentation survives renames and
reorders and is restored by undo/redo. Deleting a column reports its entry as
orphaned and keeps it until explicitly discarded (undo re-attaches it).
schema, and never rewrites a cell or marks the document dirty. The editor
prefills each column's technical name and inferred F31 type.
tabular CSV docs. Import merges by column ID (or by mapped column name when
IDs differ) and surfaces every field-level conflict for explicit resolution
before replacing anything.
owner on every column) as ordinary validation issues, and columns classified
confidential/restricted are folded into the PII scan preflight even when
no detector matches.
Chain integration (this stage)
Now that F37's
ProjectStoresits beneath F38, the previously reserveddictionaryproject section is wired up as a real registered section:ProjectSections.dictionaryis now a typedVec<SourceDictionary>(persource, holding the versioned
DictionaryExportenvelope), with aset_section_typedarm andreserved: falseinSECTION_REGISTRY.through the atomic save pipeline exactly like schemas, views and row keys.
Live-session capture stays deferred for all config sections (per F37's
documented scope); this stage delivers the persistence + registration.
value — so it passes the no-cell-data scan unchanged.
Tests
Full gates green against the chain (
run-gates.ps1, both suites exit 0):cargo test --lib: 600 passed / 0 failed. Includes the F38dictionarymodule suite and a newproject.rstest,dictionary_section_registers_and_round_trips_per_source(accept-not-reject,verbatim save+reload keyed by column ID, clean data-key scan). The populated
dictionary is also covered by the existing "every section" round-trip.
vitest: 264 passed / 37 files (incl.dictionary.test.tsand the store's dictionary tests).
cargo fmt --check,clippy -D warnings,tsc,eslint,prettier --checkall clean.
Acceptance-criteria mapping
dictionary.rs,document.rsdictionary.rs,document.rsdictionary.rs,commands.rsdictionary.rs,DictionaryDialog.tsxdictionary.rs(export_json/markdown/csv)dictionary.rs(parse_import,MergePlan),DictionaryDialog.tsxuseStore.ts,commands.rs(self-review fix)settings.rs(required_documentation)pii.rsproject.rs(this stage)🤖 Generated with Claude Code