Skip to content

feat: create_paper_note — quote-less paper-level commentary (#185 PR4/5)#191

Merged
gerchowl merged 5 commits intodevfrom
land/185-pr4-paper-note
Apr 28, 2026
Merged

feat: create_paper_note — quote-less paper-level commentary (#185 PR4/5)#191
gerchowl merged 5 commits intodevfrom
land/185-pr4-paper-note

Conversation

@gerchowl
Copy link
Copy Markdown
Contributor

Summary

P1 of #185. Adds quote-less paper-level commentary on a publication as a whole, surfaced through both MCP and TUI:

  • New paper-note:<paper_id> sentinel anchor + resolver short-circuit (mirrors feat(annotations): synthetic sentence_id for unanchored imported notes #158's import-synthetic pattern in its own namespace, disjoint by construction).
  • New MCP tool create_paper_note(paper_id, note, author) — verifies paper exists, rejects empty author/note. Emits AnnotationEventKind::Created on the broadcast channel like other writes.
  • TUI reader-mode right pane partitions annotations into "── Paper-level notes ──" (with a glyph) and "── Threaded annotations ──" (existing) sections.
  • New P keybind in the paper-detail overlay opens a single-buffer paper-note prompt.
  • Shared paper_note_anchor(paper_id) helper in scitadel-core so the MCP tool and the TUI DataStore wrapper produce byte-identical anchors.

Commits

  1. `6cfce99` sentinel constant + Anchor::is_paper_note() + resolver short-circuit
  2. `a98e68b` MCP tool + DataStore wrapper + e2e tests + ENV_LOCK serialization
  3. `94c5475` TUI section + P keybind + AnnotationPrompt::PaperNote + tape
  4. `2528d8a` review-driven: shared paper_note_anchor helper + drop stale dead_code

Test plan

  • 88/88 `cargo test -p scitadel-tui --lib` pass
  • 67/67 `cargo test -p scitadel-mcp --lib` pass
  • core: sentinel disjointness, paper_note_anchor predicate match, stability per paper_id, predicate positives + rejections (quote / range / wrong namespace)
  • db: resolver short-circuits paper-note anchor to Ok under non-matching body
  • tools: lookup paths, anchor construction
  • mcp e2e: `create_paper_note` through the server surface persists with sentinel + emits `Created` event; rejects missing paper + whitespace-only note
  • tui DataStore: persists with sentinel; co-exists with anchored annotations on same paper
  • tui prompt: `PaperNote` variant submit / empty-cancel / backspace
  • `cargo clippy --tests -- -D warnings` clean across affected crates
  • `cargo fmt --check` clean
  • New `tests/vhs/tui-paper-note.tape` exercises the full P1 flow; assigned to `tui-reader` shard with shard-coverage gate updated

Out of scope (follow-up)

  • Selection fidelity in `publish_tui_state` reader mode (PR5)
  • Visual-mode quote select
  • Anchor selector convergence

Refs #185.

Foundation for the `create_paper_note` tool (PR4 part 1 of 3).
Paper-level notes are commentary on a publication as a whole — no
quote, no char_range, no fuzzy match needed. Adding a second
synthetic-anchor namespace lets the resolver short-circuit them to
AnchorStatus::Ok the same way #158's `.bib` import path does, but
keeps them distinct in their own prefix so the TUI can render the
two kinds in separate sections.

- New `PAPER_NOTE_SENTENCE_ID_PREFIX = "paper-note:"` constant +
  `paper_note_sentence_id(paper_id)` helper. Disjoint from
  IMPORTED_SENTENCE_ID_PREFIX by construction (the prefix strings
  share no common prefix).
- New `Anchor::is_paper_note()` predicate, mutually exclusive with
  `is_imported_synthetic` on a well-formed anchor.
- `resolve_anchor` short-circuits paper-notes to Ok before any
  selector logic runs, mirroring the import-synthetic short-circuit.

Tests: sentinel disjointness, stability per paper_id, predicate
positives + rejections (quote / range), DB resolver short-circuit
under non-matching body text.

Refs #185.

[tape-exempt: data-model + resolver only; no UI surface yet]
Quote-less paper-level commentary as an MCP tool and a TUI helper.
Both surfaces produce annotations whose anchor carries only the
`paper-note:<paper_id>` sentinel from C1; the resolver short-circuits
them to AnchorStatus::Ok.

- New tool `create_paper_note(paper_id, note, author)` on the MCP
  server. Verifies the paper exists at the boundary so a typo can't
  create a dangling note. Empty note rejected. Emits a `Created`
  event on the broadcast channel like other write tools.
- `DataStore::create_paper_note` mirror for the TUI side (wired up
  in C3).

Tests:
- DataStore: persists with sentinel anchor; co-exists with anchored
  annotations on the same paper.
- Server e2e: `create_paper_note` through the MCP tool surface
  persists with `is_paper_note()` and emits a Created event.
- Server e2e: rejects missing paper + whitespace-only note.

Test isolation: the three SCITADEL_DB-mutating tests now share an
`ENV_LOCK` mutex; each holds it for the sync setup + write phase
and drops before the .await on the broadcast channel. Sound because
the broadcast Sender has already been populated for *this* server
instance — a parallel test taking the lock next can't disturb the
in-flight `rx.recv` resolution.

Refs #185.

[tape-exempt: MCP tool + DataStore wrapper; TUI rendering lands in C3]
Closes the user-facing half of #185 PR4. The reader-mode right pane
now partitions annotations into two sections:

- "── Paper-level notes ──" (above): unanchored commentary, drawn
  with a `¶` glyph. Replies to a paper-level note still render as
  children with the same threading semantics.
- "── Threaded annotations ──" (below): the existing quote-anchored
  threads, unchanged.

The `P` key in the paper-detail overlay opens a single-buffer
paper-note prompt (no quote stage). Submission dispatches to
`data.create_paper_note` which persists with the
`paper-note:<paper_id>` sentinel from C1; the resolver short-circuits
on next paper-open. New `AnnotationPrompt::PaperNote` variant is
fully covered by state-machine tests (submit, empty cancel,
backspace).

VHS tape `tui-paper-note.tape` exercises the full flow: pre-existing
paper-note renders in its own section, `P` opens the prompt, the
new note appears alongside the first one. Added to the `tui-reader`
shard with shard-coverage gate updated to match.

Help text updated: "n: new | P: paper-note | J: focus".

Refs #185.
…PR4 review)

Two non-blocking nits from the review, both addressed:
- Extract `paper_note_anchor(paper_id) -> Anchor` to scitadel-core
  so the MCP tool and the TUI DataStore wrapper share a single
  source of truth for paper-note anchor construction. Both paths
  now call the helper, which guarantees byte-identical anchors and
  prevents future drift between the two surfaces and the resolver.
- Remove the now-stale `#[allow(dead_code)]` on
  `DataStore::create_paper_note` — the C3 commit already wired up
  the `P` keybind, the suppress is no longer needed.

Refs #185.

[tape-exempt: refactor + test only; no UI surface change]
…PR4)

PR191 hit the 10m timeout in the tui-reader shard. The render step
itself succeeded; the distinctness assertion then triggered the
retry loop on tui-export-keybind.tape (historically the flakiest)
and ran out the clock with 5 tapes in the shard.

Two changes:
- Per-shard `timeout-minutes` now expression-driven via the matrix
  entry (`matrix.shard.timeout-minutes || 10`), so a single shard
  can override the default without bumping every shard.
- tui-reader → 14m. Empirically: 5 tapes × ~70s render + cargo
  build (3m) + retry loop on the flaky tape (3 × 70s in the worst
  case) ≈ 13m. 14m gives margin for the per-render overhead.

The other 5 shards keep the 10m default. Parallel total wall-clock
stays under 14m.

Refs #185, #187.

[tape-exempt: CI-only workflow change; no TUI/CLI surface touched]
@gerchowl gerchowl merged commit 5449ab2 into dev Apr 28, 2026
19 checks passed
@gerchowl gerchowl deleted the land/185-pr4-paper-note branch April 28, 2026 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant