feat(f40): row bookmarks, tags, and notes - #43
Conversation
Row bookmarks, tags and notes (F40) backend: mark and annotate records
without touching source data. New annotations.rs owns the model (row
marks, row/cell notes, per-document tag namespace with usage counts,
author label + created/updated timestamps), a doc_id-keyed store with its
own revision, the rematch engine (matched/ambiguous/orphaned) anchored by
row_identity (composite KeySpec key or source record + content
fingerprint), annotation-state filter predicates, tag-to-column
preview+apply as one undoable document op, JSON/CSV export, and the
versioned sidecar / project-section persistence envelope.
- project.rs: annotations section activated (was reserved) as a typed
per-source SourceAnnotations{AnnotationsExport}; no-cell-data scan still
passes; round-trip + registration tests.
- commands.rs: 19 commands (view/rematch/edit/notes/tags/filter/
tag-to-column/export/sidecar) with revision guards; annotations live in
the AnnotationRegistry outside the Document so they survive the
whole-document swap on reparse (front end calls annotations_rematch
after a reload).
- error.rs: StaleAnnotationsRevision guard, independent of data/schema/
dictionary revisions (annotating never dirties the document).
Rust gates: fmt + clippy clean; cargo test --lib 643 passed / 0 failed
(18 annotation tests).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Front end for row bookmarks, tags & notes: grid gutter glyphs (star / flag / note) plus a per-cell note corner and subtle row tint, all placed through a new display-row -> record bridge command so indicators track any sort/filter; an inline cell context menu (star, flag, tags, row note, cell note); the AnnotationsPanel (type filters, search, jump-to-row, tag namespace with create/remove/to-column, ambiguous/orphaned review with resolve actions, and author/key-column anchoring settings); note, tag picker, tag-to-column (preview + one undo) and export dialogs; annotation state filters + row ops in the command palette; a per-tab store slice that loads the doc_id-keyed registry, re-resolves on reparse and persists to the `.ceesvee-notes.json` sidecar. Types, tauri wrappers and vitest for the pure logic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Consolidated F40 annotations user-facing docs and confirmed all gates green on the integrated branch. - CHANGELOG: surface the annotations side panel (list + jump-to-row + review queue) in the existing Unreleased entry. - README: add a "Row bookmarks, tags & notes" bullet to Navigate & analyze. Gates: npm (lint/format/typecheck/312 vitest/build) + rust (fmt/clippy/ 643 lib tests) all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire the project-vs-sidecar annotation persistence that the spec, CHANGELOG and README promised but the front end never implemented, and cover two open paths that skipped sidecar hydration. - Persist annotations into the project's `annotations` section: capture each open source's export envelope in captureProjectSections (merging with referenced-but-closed sources) and load it back in applyProjectPlan on project open. New pure helper buildAnnotationsSection mirrors buildViewsSection (unit-tested). - Make sidecar persistence project-aware: persistAnnotationSidecar is a no-op when a project is open (the two stores are mutually exclusive per the migration rule); hydrateAnnotationsFromSidecar never reads the sidecar while a project is open or mid-open (openingProject flag bridges applyProjectPlan, where get().project is not yet set), so the project section always wins. - Hydrate the sidecar on the indexed (F10) and archive-extract editable open completion paths (job-finished handler + confirmOpenEditable archive branch) so an annotation edit on a large read-only or extracted document no longer overwrites an existing .ceesvee-notes.json with an empty store. - Add the missing AnnotationStore::discard_orphans test (orphan removed, matched entry with its tags/notes intact, no-op + no revision bump when none orphaned). - Store tests: sidecar written with no project / skipped with a project open; indexed open hydrates the sidecar. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rebase feat/f40-annotations onto feat/f48-sampling (554a7d0). The new f48 commit touched only src-tauri/src/{derived,sampling}.rs, disjoint from every file f40 changes, so the replay was conflict-free. Full gates green on the first compile against the chain (npm lint/format/typecheck/test 320, build; cargo fmt/clippy; cargo test --lib 648) — no source fixes required. 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: d2dacecbf4
ℹ️ 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".
| store.set_key_spec(key_spec); | ||
| store.reanchor(&source, None)?; |
There was a problem hiding this comment.
When the user changes the key columns (or clears them), this overwrites store.key_spec before reanchor() resolves the existing key-anchored entries. Existing anchors only contain the old composite value, so rematch() now builds the index from the new spec (or no index when clearing) and can either orphan every annotation or match the old value against an unrelated column, silently reattaching notes to the wrong row. Resolve with the previous key spec first, then capture anchors under the new spec.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0935201.
Confirmed the bug: set_key_spec overwrote store.key_spec before reanchor() resolved the existing key anchors, so the subsequent rematch built its index from the new spec (or none, when clearing) and resolved the leftover old-spec composite keys against it — orphaning every keyed note on a clear, or matching an old value against an unrelated column and silently reattaching notes to the wrong row.
Replaced the two-step set_key_spec + reanchor with a single atomic AnnotationStore::set_key_spec_and_reanchor: it rematches under the OLD spec first, captures each MATCHED entry's replacement anchor under the NEW spec up front (an invalid new spec errors before any mutation), then commits the spec + anchors together. Ambiguous / orphaned entries keep their old anchor and stay in the review list — an uncertain row is never silently re-anchored (the F40 invariant).
Covered by 4 new lib tests: clear→record, switch key columns (follows the new key), record→key upgrade, and set_key_spec_and_reanchor_never_reattaches_uncertain_rows (an entry ambiguous under the old spec is never auto-pinned to a duplicate).
| * the next explicit save/export instead of interrupting an annotation edit. | ||
| */ | ||
| const persistAnnotationSidecar = (docId: number, path: string | null) => { | ||
| if (get().project) return; |
There was a problem hiding this comment.
When a project is open, annotation edits take this early return and are only kept in the backend registry until projectSave() later captures the annotations section. However the project dirty snapshot tracks tabs/layout/views, not annotations, so adding a note/tag in a project can leave isProjectDirty() false; quit/discard flows then do not prompt or save the project, and the new annotations are lost. Mark the project dirty or update the project section when project-backed annotations change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0935201.
Confirmed: with a project open the edit takes the early return, and the project dirty derivation (ProjectSnapshot) only tracks tabs / active / layout / views — not annotations. So adding a note/tag left isProjectDirty() false, and quit/discard neither prompted nor saved, dropping the new annotations.
Added an explicit projectAnnotationsDirty flag: set in persistAnnotationSidecar whenever a project is open (instead of just no-oping), reset to false wherever the baseline is (re)captured — projectNew, applyProjectPlan (open), projectSave, closeProjectNow, and initial state. It is folded into isProjectDirty(), guardDiscardProject(), and the reactive useProjectDirty() hook, so the dirty dot and quit/close prompts now account for project-backed annotation edits. On the next projectSave the annotations section captures the edit and the flag clears.
Two new store tests: the project is marked dirty on a project-backed edit, and is not dirtied on a sidecar (no-project) edit.
| source_path: String, | ||
| annotations: State<'_, AnnotationRegistry>, | ||
| ) -> AppResult<()> { | ||
| let store = annotations.try_with(doc_id, |store| Ok(store.clone()))?; |
There was a problem hiding this comment.
The frontend saves sidecars fire-and-forget after an annotation edit, so a quick tab close can race with this command: close_document removes the registry entry, then this try_with recreates a default empty store for the missing doc id and save_sidecar deletes the existing notes file. The save path should clone an existing store only, or receive the export snapshot from the edit that triggered the save, instead of creating an empty store on miss.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0935201.
Confirmed the race: close_document calls annotations.remove(doc_id), then a fire-and-forget annotations_save_sidecar for that same doc id hit try_with, whose entry(doc_id).or_default() recreated a default empty store on the miss; save_sidecar of an empty store then deletes the notes file — data loss.
Added AnnotationRegistry::clone_existing, which clones only when the entry is present and returns None on a miss without creating anything. annotations_save_sidecar now returns Ok(()) on None, leaving the existing sidecar untouched (nothing to persist).
New test clone_existing_does_not_resurrect_a_closed_store asserts a miss returns None and that the registry map stays empty (no phantom entry), and that a present store is cloned.
| .into_iter() | ||
| .map(|(record, value)| (record as usize, column, value)) | ||
| .collect(); | ||
| doc.set_cells(changes)?; |
There was a problem hiding this comment.
For existingColumn, arbitrary tag names are written with doc.set_cells, bypassing the schema validation path used by normal cell edits (schema_ops::apply_validated_cells). If the target is a strict integer/date/boolean column, copying a tag like review will commit a value the backend would otherwise reject, leaving the document inconsistent with its declared schema. Route these writes through the validated cell-apply path before registering the undo op.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0935201.
Confirmed: the ExistingColumn branch wrote through doc.set_cells directly, bypassing the F31 validation path (schema_ops::apply_validated_cells) that ordinary cell edits use. A tag like review copied into a strict integer / date / boolean column would commit a value the backend would otherwise reject, leaving the document inconsistent with its declared schema.
Routed the writes through schema_ops::apply_validated_cells (same batched op, so the same single undo group): a strict target now rejects the whole batch before anything applies; an advisory column applies and records issues, exactly like a normal edit.
New test tag_to_existing_strict_column_is_rejected_by_validation tags a row, then asserts copying it into a strict integer column is rejected ("invalid value for column") with the model untouched.
…car/tag safety Addresses the four Codex review comments on PR #43. P1 (commands.rs) — changing the annotation key spec re-anchored existing notes AFTER the new spec was already installed, so rematch resolved the leftover old-spec key anchors against the new index: it orphaned every keyed note when clearing, or silently reattached them to an unrelated column. Replaced set_key_spec + reanchor with a single atomic AnnotationStore::set_key_spec_and_reanchor that resolves under the OLD spec first, captures each MATCHED entry's replacement anchor under the NEW spec up front (an invalid new spec fails before any mutation), then commits. Ambiguous / orphaned entries keep their old anchor and stay in the review list — an uncertain row is never silently re-anchored. P1 (useStore.ts) — project-backed annotation edits took the sidecar early return and never marked the project dirty (annotations are not part of the ProjectSnapshot), so quit/close could drop them. Added an explicit projectAnnotationsDirty flag set on every project-backed annotation edit, reset whenever the project baseline is (re)captured (new/open/save/close), and folded into isProjectDirty() and useProjectDirty(). P2 (commands.rs) — annotations_save_sidecar used try_with, which recreates a default empty store on a registry miss; a fire-and-forget save racing a tab close would then delete the notes file. Added AnnotationRegistry::clone_existing (no resurrection) and skip the save when the store is absent. P2 (commands.rs) — apply_tag_to_column wrote an existing column via raw doc.set_cells, bypassing F31 validation, so a tag could commit into a strict integer/date/boolean column. Routed the writes through schema_ops::apply_validated_cells (same undo shape). Tests: 6 new Rust lib tests (4 re-anchor semantics incl. the never-attach -to-uncertain-rows invariant, clone_existing no-resurrect, strict-column tag rejection) and 2 new store tests (project marked dirty on a project- backed edit; not dirtied on a sidecar edit). All gates green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # src-tauri/src/project.rs # src/store/useStore.test.ts # src/store/useStore.ts
1 participant
feat(f40): row bookmarks, tags, and notes
Mark and annotate records without ever touching the source data. F40 adds a
per-document annotation layer — bookmarks (star/flag), a named-tag namespace,
row notes and per-column cell notes — pinned to row identity and re-matched
on reparse, with a side panel, a state filter, tag→column materialization, and
JSON/CSV export.
What ships
(per-document namespace with usage counts); attach a row note or per-column
cell notes with an optional author label and created/updated timestamps.
(survives reordering; duplicate keys reported ambiguous) or, absent a key,
the source record number plus a content fingerprint. On reparse / external
change they re-match into a matched / ambiguous / orphaned review queue,
so a note is never silently reattached to an uncertain row; a deleted row
keeps its annotation as an orphan until the delete is committed or reverted.
dedicated review queue for the ambiguous/orphaned entries.
tagged, has-note) through the existing filter view.
operation (derived edit; source untouched until saved).
never inside the CSV and never in an ordinary data export.
project open, a versioned
<file>.ceesvee-notes.jsonsidecar writtenatomically. When a project is open the sidecar is not read or written.
Command surface (registered in
lib.rs)annotations_view,annotations_rematch,annotations_set_key_spec,annotations_set_author,annotations_edit_row,annotations_set_row_note,annotations_set_cell_note,annotations_remove_row,annotations_discard_orphans,annotations_define_tag,annotations_remove_tag,apply_annotation_filter,export_annotations,annotations_get_export,annotations_load_export,annotations_load_sidecar,annotations_save_sidecar(+
AnnotationRegistrymanaged state).Tests
Full gates green on the first compile against the chain — no source fixes
required:
annotations.test.ts(17) andthe project round-trip / sidecar precedence cases in
project.test.ts&useStore.test.ts.cargo test --lib(0 failed) — incl. 18annotationsunit tests (identity keys, re-match classification, tag namespace, filter,
export envelope, atomic sidecar).
--check,tsc --noEmit,npm run build,cargo fmt --check,cargo clippy --all-targets --all-features -D warnings.Acceptance mapping
annotations.rsengine;annotations_edit_row/_set_row_note/_set_cell_note/_define_tagannotations_set_key_spec; identity/fingerprint logic inannotations.rsannotations_rematch+annotations_view; classification testscomponents/AnnotationsPanel.tsxapply_annotation_filter+ filter view wiringcomponents/TagToColumnDialog.tsx; derived undoable opexport_annotations;AnnotationExportDialog.tsxproject.ts(buildAnnotationsSection) + sidecar guards inuseStore.tsSTACKED PR
Base is
feat/f48-sampling, notmain. Review the delta vsfeat/f48-samplingonly — the earlier chain (main ← f31 ← infra ← f33 ← f37← f38 ← f48) is reviewed in its own PRs. This branch rebased cleanly onto the
latest f48 tip (
554a7d0); that commit touches onlyderived.rs/sampling.rs,disjoint from every file F40 changes, so integration was conflict-free.
🤖 Generated with Claude Code