feat(occ): inform flow — every note-scoped response carries the revision (SVA-21)#125
Open
vasylenko wants to merge 32 commits into
Open
feat(occ): inform flow — every note-scoped response carries the revision (SVA-21)#125vasylenko wants to merge 32 commits into
vasylenko wants to merge 32 commits into
Conversation
…n formatter
Lays the operations and infra groundwork for the OCC inform contract (SVA-21).
No tool wiring yet — that's the next commit.
- BearNote gains a required `revision: NoteRevision` field; `NoteRevision`
is a named alias for `number` so call sites read as OCC tokens rather than
arbitrary integers.
- Three read queries project Z_OPT: getNoteContent, findNotesByTitle (via
ZSFNOTE row), and findUntaggedNotes. Required-field TS check confirms every
BearNote construction site populates it.
- awaitRevisionIncrement(id, baseline) — single-DB-connection poll, 15ms
interval / 500ms cap. Compares for inequality (not baseline+1) to handle
Bear's +2 first-edit-after-creation jump (subtitle/index recompute).
Returns the new revision or null on timeout; caller emits the timeout
sentence rather than a stale value.
- awaitNoteCreation now returns { id, revision } — same SELECT, atomic.
- Search-result revision hydration goes against the live Bear DB
(fetchRevisionsForResults batch SELECT in fts-index.ts), NOT a cached
Z_OPT in the FTS shadow. Rationale: the index drift sentinel
MAX(ZMODIFICATIONDATE) + COUNT(*) misses pin-only and tag-only writes
that bump Z_OPT without bumping ZMODIFICATIONDATE — a cached revision
would silently lag.
- REVISION_TIMEOUT_SENTENCE and formatRevisionLine centralize the response
format and sentinel wording in src/tools/responses.ts. The sentence is
composed from REVISION_POLL_CAP_MS (runtime constant) per MCP_STANDARDS
anti-drift rule.
Unit tests cover: awaitRevisionIncrement timeout + change-detected paths,
awaitNoteCreation tuple shape, fetchRevisionsForResults under mid-search
Z_OPT mutation, REVISION_TIMEOUT_SENTENCE contract (prefix + cap value).
Refs SVA-21
Threads the foundation into all 9 in-scope tools (SVA-21). Out of scope: bear-list-tags, bear-rename-tag, bear-delete-tag, bear-capabilities — no specific note reference, so no Revision line. Read tools surface existingNote.revision (or each search hit's hydrated revision) directly in the response: - bear-open-note: Revision line in the metadata block (single-note path); out-of-scope for the disambiguation listing (it doesn't carry per-note metadata beyond ID + modified date). - bear-search-notes: per-result Revision line, indented to match Created/ Modified/Tags/ID. - bear-find-untagged-notes: per-result Revision line below ID. Write tools follow one of two patterns: A) Content writes (add-text/replace-text/create/add-file/add-tag) capture the pre-write revision from the existing getNoteContent (or awaitNoteCreation) call — no new DB hit — fire the URL, then call awaitRevisionIncrement to confirm the change landed and capture the new value. The response emits `Revision: <newRev>` on success or the timeout sentence on failure. B) bear-archive-note uses the pre-flight revision labeled `Revision at time of archive: <preArchiveRev>`. Post-archive the note is filtered from default queries (`ZARCHIVED = 0`), so a post-write read would return null. The explicit temporal label tells the LLM the value is a pre-write snapshot rather than the live current revision. Tool descriptions for the six write tools gain a sentence previewing the follow-up enforce flow (SVA-22): "a future enhancement will let you supply this value on writes to detect stale-body overwrites." Refs SVA-21
End-to-end verification against live Bear: each test reads Z_OPT directly from SQLite, dispatches the tool via MCP Inspector CLI, parses the Revision line from the response, and asserts it matches the post-write DB value (and for non-create writes, differs from the pre-write value). - tests/system/inspector.ts: shared helpers for reading Z_OPT and parsing the Revision line from response text. - open-note-by-title, tag-search: read-tool revision-presence assertions. - create-note, add-text, replace-text, attached-files, add-tag: write-tool pre/post Revision-delta assertions. - archive-revision.test.ts (new): verifies the pre-archive snapshot surfaces with the temporal label, and that the snapshot value equals Z_OPT read without the ZARCHIVED filter (proof the snapshot semantics are honest). - find-untagged-revision.test.ts (new): per-result Revision assertion for the list-of-notes response. `task test:system` — 13 files, 56 tests, all green. Refs SVA-21
- docs/dev/SPECIFICATION.md: new "Optimistic Concurrency Control (Inform Half)" section under Safety Gates. Covers the two write patterns (poll for change vs pre-archive snapshot), pin-only-write FTS drift caveat, and the constraints partially lifted by inform (write verification + test pause-after-write). - docs/dev/MCP_STANDARDS.md: codifies the mutation-response-metadata rule (ID + title + revision + what-changed for every note-level mutation) with the polling-for-change exception (post-write reads are forbidden UNLESS the read waits for a change) and the archive-flow exception (pre-write snapshot with explicit temporal labeling). - docs/dev/BEAR_DATABASE_SCHEMA.md (new): empirical Z_OPT findings — per-URL-action bump table, the +2 first-edit-after-creation jump, the pin-only-write divergence from ZMODIFICATIONDATE, and concurrency semantics (journal_mode=DELETE, busy_timeout=3000). Reference for future contributors observing or polling the Bear DB. - CHANGELOG.md: new "## [Unreleased]" section documenting the user-visible Revision line addition. Refs SVA-21
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the “inform” half of optimistic concurrency control by threading Bear’s ZSFNOTE.Z_OPT into note-scoped tool responses as a Revision: <n> line, enabling clients to detect note changes across calls.
Changes:
- Add
revision(Z_OPT) to theBearNotemodel and project/hydrate it across note reads, searches, and “find untagged”. - Add write-confirmation polling (
awaitRevisionIncrement) plus centralized response formatting (formatRevisionLine/ timeout sentence) and emit revision lines from note tools. - Add unit + system tests validating response revisions against live DB, and update specs/standards + changelog.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/system/tag-search.test.ts | Adds system assertion that per-result Revision matches live Z_OPT. |
| tests/system/replace-text.test.ts | Adds system coverage ensuring bear-replace-text emits a numeric revision or the timeout sentence. |
| tests/system/open-note-by-title.test.ts | Adds system assertion that bear-open-note revision matches live Z_OPT. |
| tests/system/inspector.ts | Adds helpers for parsing Revision and reading Z_OPT directly from the Bear DB for system tests. |
| tests/system/find-untagged-revision.test.ts | New system test validating bear-find-untagged-notes per-result revision wiring. |
| tests/system/create-note.test.ts | Adds system assertion that bear-create-note emits revision matching live Z_OPT (when creation is confirmed). |
| tests/system/attached-files.test.ts | Adds system coverage ensuring bear-add-file emits a numeric revision or the timeout sentence. |
| tests/system/archive-revision.test.ts | New system test validating archive responses include the explicitly-labeled pre-archive revision. |
| tests/system/add-text.test.ts | Adds system coverage ensuring bear-add-text emits a numeric revision or the timeout sentence. |
| tests/system/add-tag.test.ts | Adds system coverage ensuring bear-add-tag emits a numeric revision or the timeout sentence. |
| src/types.ts | Adds revision: NoteRevision to BearNote and introduces NoteRevision alias. |
| src/tools/responses.ts | Centralizes revision line formatting and the timeout sentence sourced from runtime constants. |
| src/tools/responses.test.ts | Unit tests locking in timeout sentence prefix and cap inclusion. |
| src/tools/note-tools.ts | Emits revision lines for note-scoped responses; adds post-write polling for content mutations; adds archive snapshot label. |
| src/operations/tags.ts | Projects Z_OPT as revision for untagged note queries. |
| src/operations/notes.ts | Projects Z_OPT in note reads; extends awaitNoteCreation to return {id, revision}; adds awaitRevisionIncrement. |
| src/operations/notes.test.ts | Adds unit tests for awaitRevisionIncrement and updated awaitNoteCreation return shape. |
| src/infra/fts-index.ts | Hydrates search-result revisions from the live Bear DB via an IN (...) lookup. |
| src/infra/fts-index.test.ts | Extends synthetic DB to include Z_OPT and tests live hydration behavior. |
| docs/dev/SPECIFICATION.md | Documents OCC inform behavior, polling semantics, and archive labeling. |
| docs/dev/MCP_STANDARDS.md | Updates tool-surface standards to require revision metadata and defines the polling exception. |
| docs/dev/BEAR_DATABASE_SCHEMA.md | New empirical notes on Z_OPT behavior and URL-action bump semantics. |
| CHANGELOG.md | Adds Unreleased entry describing revision lines, polling confirmation, and timeout behavior. |
The OCC section's 'Constraint partially lifted: write verification' cited 'Bear's URL Scheme Quirks' as the original constraint location, but the constraint actually lives under 'Intentional Exclusions' (line 179). Readers following the cross-reference landed on the wrong subsection. Refs PR #125 review.
…(F23, review) The OCC section claimed no test-side sleep is needed because polling waits for Bear. That's only true for the under-test write itself — tests that chain a prior create-note still need sleep(PAUSE_AFTER_WRITE_OP) between the create and the next baseline read, because Bear's +2 first-edit subtitle/index recompute save lands after awaitNoteCreation returns (see BEAR_DATABASE_SCHEMA.md). Four new tests (add-tag, add-text, attached-files, replace-text) demonstrate this in the same PR. Narrows the claim to describe what the polling actually covers; points to the schema doc for the recompute behavior. Refs PR #125 review.
…ns (F11+F12+F13, review) Six mutation tools each ended their description with: "The response includes the note's revision (a version token); a future enhancement will let you supply this value on writes to detect stale-body overwrites." Three issues with that sentence: 1. YAGNI in user-facing copy — describes an unshipped feature (the SVA-22 enforce sibling). Promising a future schema change in description-for- tool-selection trains the LLM on a contract that doesn't exist today. 2. DRY violation — same business knowledge duplicated across six tool descriptions; six sites to update in lockstep when SVA-22 lands. 3. MCP_STANDARDS rule violation — descriptions are for tool *selection*; the future-enhancement preview doesn't help selection, and the response body itself surfaces the Revision line. Additionally, the boilerplate misrepresented two specific tools: - bear-archive-note's response uses the label 'Revision at time of archive' (not 'Revision:'), and post-archive the note is filtered from default queries, so the snapshot can never serve as an OCC token for a next write. - bear-add-tag mutates tag relations, not body content, so 'stale-body overwrites' doesn't describe what add-tag does. Drop the sentence from all six descriptions. The response itself carries the Revision line; tool selection doesn't benefit from previewing it. This resolves review findings F11, F12, and F13 in one edit. Refs PR #125 review.
…stly (F9, review)
Two coupled fixes for the FTS revision-hydration path:
1. `executeQueryWithCount` no longer accepts `bearDb?` as optional. The
parameter existed only so legacy FTS tests didn't have to pass it; the
single production caller always passed it. `withFixture` in the test
helper now exposes bearDb to test bodies, and all 12 callsites pass it.
Argument order also reordered to (memDb, bearDb, spec) so the two DB
handles sit together; the spec moves to the tail (typical TS convention).
2. The `?? 0` fallback for missing-identifier hydration is replaced with
`?? null`. Bear's Z_OPT starts at 1 per BEAR_DATABASE_SCHEMA.md, so 0
was a structurally-impossible value masquerading as a real revision —
indistinguishable from a real value to LLM consumers and inconsistent
with the rest of the OCC inform doctrine (REVISION_TIMEOUT_SENTENCE for
write-side timeouts; explicit 'Note ID: unknown' for create-time
confirmation misses). `SearchResult.revision` widens to
`NoteRevision | null` via `Omit<BearNote, 'revision'>`; BearNote stays
non-null because all other read paths (getNoteContent, findUntaggedNotes,
awaitNoteCreation) project Z_OPT in the same SELECT that finds the row.
A new `REVISION_UNAVAILABLE_SENTENCE` constant distinguishes the
read-side miss ('note not found in live database — likely deleted,
archived, or encrypted since the search index was built') from the
write-side `REVISION_TIMEOUT_SENTENCE` ('write confirmation timed out
after 500ms'). `formatRevisionLine` gains an optional second arg for
the sentinel sentence, defaulting to the write-timeout sentence; the
bear-search-notes formatter passes the unavailable sentence.
Coverage: existing FTS unit test `hydrates revision from live Bear DB`
(`fts-index.test.ts`) already exercises the populated path with a
mid-search Z_OPT mutation. The null-path is rare in production (race
between FTS rebuild and live-DB hydration) and is covered structurally
by the type system rather than a dedicated test.
`task build` (106 unit tests) and `task test:system` (56 system tests
across 13 files) both green.
Refs PR #125 review.
…ion (F27, review)
The previous pattern in add-tag, add-text, replace-text, and attached-files:
expect(containsTimeoutSentence || responseRevision !== null).toBe(true);
if (responseRevision !== null) {
expect(responseRevision).toBeGreaterThanOrEqual(preWriteRevision!);
}
had two weakness layers:
1. Primary disjunction was vacuous — true whenever the response had either
line, which is essentially always.
2. `>=` accepted stale-baseline regressions. Per BEAR_DATABASE_SCHEMA.md,
Z_OPT increases monotonically; awaitRevisionIncrement returns only when
Z_OPT !== baseline, so on the bump path the returned value is strictly
greater than baseline, never equal. `>=` would silently pass if a
regression in the `!== baseline` guard returned the baseline itself —
exactly the bug class OCC exists to detect.
New pattern across all four files:
const responseRevision = tryExtractRevision(result);
expect(responseRevision).not.toBeNull();
expect(responseRevision!).toBeGreaterThan(preWriteRevision!);
const liveDbRevision = readNoteRevision(noteId);
expect(responseRevision).toBe(liveDbRevision);
This catches three failure classes:
- Stale-baseline regression: responseRevision === preWriteRevision fails toBeGreaterThan.
- Wrong-column projection: responseRevision !== liveDbRevision fails toBe.
- Bear-stopped-bumping regression: tryExtractRevision returns null because
the response carries the timeout sentence instead of a digit; toBe(number)
fails loudly. (Previously the test silently passed via the disjunction.)
Drops the forward-compat skip for the timeout sentence — if the bump path
breaks in a future Bear release, the test must fail visibly so the
implementation gets adjusted, not silently pass through the sentinel.
`task test:system` — 13 files, 56 tests, all green.
Refs PR #125 review.
Both tests asserted tautological properties of a templated string constant:
- startsWith('Revision: unknown') — the source literal starts with that prefix.
- contains `${REVISION_POLL_CAP_MS}ms` — the source uses that interpolation
one line away from the constant definition.
These can only fail if someone deliberately changes the constant, in which
case the test isn't catching a bug — it's restating the source. The
anti-drift coupling between REVISION_POLL_CAP_MS and the sentence is
already enforced at compile time by the template literal interpolation.
System tests across 5 files exercise the wording end-to-end via
`result.includes('Revision: unknown')` and the new `toBe(liveDbRevision)`
pattern, which would catch a real format regression at the boundary the
LLM actually consumes.
CLAUDE.md rule: 'response format with no Bear interaction is almost always
a misplaced unit test.' The first SVA-21 attempt's self-review trimmed
vanity tests; this rewrite carried them back. This commit lands the same
lesson.
`task test` now reports 8 test files, 104 unit tests (down from 9 files,
106 tests).
Refs PR #125 review.
…iew)
The diagram listed three write tools under one 'Content writes' branch
that described a pre-flight getNoteContent baseline — but bear-create-note
has no pre-flight read (the note doesn't exist yet) and uses
awaitNoteCreation's bundled SELECT instead. Conflating these in one box
misguides anyone debugging the create path or extending the OCC machinery
to a new write tool.
Splits into three boxes:
1. Content writes (add-text, replace-text, add-file, add-tag) — baseline
from pre-flight getNoteContent, post-write awaitRevisionIncrement poll.
2. bear-create-note — no baseline; single bundled SELECT for {id, revision}
in awaitNoteCreation after the URL fires.
3. bear-archive-note — pre-write snapshot with explicit temporal label;
no post-write read because the note is filtered from default queries.
Each box now describes its own input/output contract truthfully.
Refs PR #125 review.
…0 (F7, review)
src/operations/notes.test.ts inlined the Core Data epoch offset as the
literal 978307200 while a sibling test (src/infra/fts-index.test.ts:5,150)
already imports and uses the exported CORE_DATA_EPOCH_OFFSET constant
from src/config.ts. The inline comment above the literal even named the
constant without importing it — exactly the 'string-literal drift'
anti-pattern MCP_STANDARDS.md warns against ('Source numeric defaults
from runtime constants, not string literals').
Refs PR #125 review.
…n SPECIFICATION (F16+F17, review) MCP_STANDARDS.md was rewritten in the SVA-21 feature commit to inline specific tool names, DB columns, helper function names, source file paths, and runtime constants — exactly the inventory the prior intro explicitly said the doc 'deliberately doesn't name'. The standards doc inverted its own layering rule, conflating project-wide MCP-server conventions with this-server's instantiation details. Project rule (`feedback_mcp_standards_generic_only` memory): MCP_STANDARDS.md is reserved for generic, project-wide MCP design guidelines. All specifics — concrete tools, helpers, DB columns, file paths, constants — go in SPECIFICATION.md (architecture/contracts) or BEAR_DATABASE_SCHEMA.md (empirical DB findings). Changes: - MCP_STANDARDS.md: rewrote the 'Mutation Response Metadata' section as a generic rule with four abstract bullets (stable identifier, label, version token, what-changed) and one generic exception (polling for change). No tool names, no DB columns, no helper names, no file paths, no constants. Closes with a pointer to SPECIFICATION.md (concrete instantiation) and BEAR_DATABASE_SCHEMA.md (empirical DB findings). - SPECIFICATION.md OCC section: added two paragraphs — 'In-scope tools' (names the 9 in-scope and 4 out-of-scope tools explicitly) and 'Field-sourcing discipline' (concrete sourcing for ID, title, revision including the title-less create case — which closes the F17 'when available' qualifier finding naturally; the new wording reads 'from the input parameter when one was provided (the title-less creation path omits the line)'). The generic rule in MCP_STANDARDS now reads as transferable advice for any future MCP server in this codebase or beyond; the concrete contract lives where the contract is implemented. Refs PR #125 review.
…it jump (F30, review) `expect(responseRevision).toBe(dbRevision)` races against Bear's +2 first-edit-after-creation subtitle/index recompute save documented in BEAR_DATABASE_SCHEMA.md. responseRevision captures the value awaitNoteCreation first saw (typically Z_OPT=1); dbRevision is read immediately afterward and may already reflect Z_OPT=3 if the recompute landed in between. The schema doc explicitly predicts this race. Z_OPT increases monotonically, so the live value can only be greater than or equal to the captured value — strict equality is wrong. `toBeGreaterThanOrEqual` still catches the meaningful regression (responseRevision wrong = live DB doesn't move backward to match). `task test:system` create-note suite: 2 tests, all green. Refs PR #125 review.
…(F22, review) The FTS5 architecture diagram ended at 'Run: FTS5 MATCH + ORDER BY rank', silent on the post-query batch SELECT against the live Bear DB (`fetchRevisionsForResults`) added for OCC inform. Subsequent code work in fts-index.ts could inadvertently restructure the search path without realizing the hydration step exists. Add the hydration step as the fourth branch in the diagram, with the rationale (drift sentinel misses pin-only and tag-only writes) and the miss-case behavior (REVISION_UNAVAILABLE_SENTENCE for vanished notes). Refs PR #125 review.
… review) Inline `sleep(100)` replaced with the named constant pattern that four sibling tests (add-text, replace-text, attached-files, tag-management) already use. Pure consistency drift fix. Refs PR #125 review.
…review) The test inlined `/Revision:\\s+(\\d+)/` + parseInt to extract the first numeric Revision from a search response. The tryExtractRevision helper from inspector.ts:194 exists for exactly this case and is used by 5 sibling system tests. Pure consistency drift fix. Refs PR #125 review.
… review)
When awaitNoteCreation times out, the response previously emitted only
'Note ID: unknown — ...' with no Revision line at all. The mutation-
response contract (note ID + title + revision + what changed) needs to
stay symmetric across both branches — a consumer parsing every response
for a Revision line had to special-case the create-timeout path.
Now emits formatRevisionLine(null) alongside, which renders as
REVISION_TIMEOUT_SENTENCE ('Revision: unknown (write confirmation timed
out after 500ms)'). Consistent with how the other write paths handle
the timeout branch.
Refs PR #125 review.
… jump (F29, review)
The test read readNoteRevision(noteId) immediately after bear-create-note
returned, then invoked bear-archive-note which internally re-read via
getNoteContent. If Bear's documented +2 first-edit-after-creation
subtitle/index recompute save (BEAR_DATABASE_SCHEMA.md) landed between
those two unsynchronized DB reads, the test's preArchiveRevision and the
handler's existingNote.revision diverged by +2 and the strict
`toContain(\`Revision at time of archive: ${preArchiveRevision}\`)`
assertion failed.
Adds `await sleep(PAUSE_AFTER_WRITE_OP)` after create and before the
baseline read, matching the pattern the four other write-tool revision
tests in this PR (add-text, add-tag, attached-files, replace-text) all
use for the same race.
Refs PR #125 review.
…ion (F28, review) The test's second assertion expect(searchResult).not.toContain(noteId) passed vacuously when search returned 'No notes found' (no IDs at all in the response). Without a positive baseline — e.g., an unarchived note with the same search term that DOES appear — the assertion couldn't distinguish 'archive filter works' from 'search returned nothing.' Also mixed two concerns: OCC inform (the focus of this file) and archive- filter regression (a pre-existing behavior). Drops the second assertion. The primary 'Revision at time of archive' assertion above is the in-scope contract for this PR. If archive-filter behavior deserves its own coverage, it belongs in a separate tests/system/archive-filtering.test.ts with a proper positive baseline — out of scope for SVA-21 review followup. Refs PR #125 review.
…nIncrement (F2, review) The poll SELECT lacked the standard ZTRASHED/ZARCHIVED/ZENCRYPTED filters that every other ZSFNOTE query in this file applies. If a concurrent process (Bear UI, CloudKit sync) trashed, archived, or encrypted the note during the 500ms poll window and Bear bumped Z_OPT for that row update, the poll would resolve on that bump and report it as confirmation of OUR write — falsely returning a revision that came from a different mutation. With the filters applied, such a note becomes invisible to the poll mid-flight; the loop falls through to timeout and the response emits REVISION_TIMEOUT_SENTENCE honestly. bear-archive-note uses a pre-write snapshot (not this helper), so adding ZARCHIVED = 0 here doesn't affect the archive flow. Rare race in practice (a single-user MCP server is unlikely to see concurrent trashes during a 500ms window), but the fix aligns the helper with the file's other query patterns and removes a class of misleading output. Test fixture for awaitRevisionIncrement updated to include the filter columns (defaulting to 0) so the existing unit tests still see their rows. `task build` 104 unit tests + `task test:system` 56 system tests all green. Refs PR #125 review.
The JSDoc described the return as 'Tuple of created note's identifier and
revision' but the actual return type is `Promise<{id, revision} | null>` —
an object, not a tuple. A reader expecting tuple destructuring would get
a type error; the type system catches it but the doc led them astray.
Refs PR #125 review.
…ns (F7, review) Six in-scope write tools (bear-create-note, bear-add-text, bear-replace-text, bear-add-file, bear-add-tag, bear-archive-note) now mention that the response includes the note's revision and that a future capability will use it to reject stale writes. Matches the PR description's claim about descriptions gaining a one-sentence forward reference to the planned enforce-half (SVA-22).
…on hydration (F5, review) fetchRevisionsForResults' IN-clause was missing the active-note filters used by the FTS index build, so a note that flipped to archived/trashed/encrypted between index build and this hydration read still returned a numeric Z_OPT — contradicting the surrounding comment that says such cases should surface as null → REVISION_UNAVAILABLE_SENTENCE. Adds AND ZARCHIVED = 0 AND ZTRASHED = 0 AND ZENCRYPTED = 0 to the query (mirrors awaitRevisionIncrement's filter pattern). Unit test parameterized over all three state flips proves the hydration miss now surfaces as null.
…F3, review)
bear-create-note's timeout branch was emitting REVISION_TIMEOUT_SENTENCE
('write confirmation timed out after 500ms') but the path actually times out
via awaitNoteCreation, capped at POLL_TIMEOUT_MS=2000ms — the 500ms cap belongs
to awaitRevisionIncrement (post-write inequality poll for content writes).
The user-visible sentence cited the wrong cap and the wrong operation.
Adds REVISION_CREATION_TIMEOUT_SENTENCE (composed from POLL_TIMEOUT_MS, in
lockstep per the MCP_STANDARDS 'source numeric defaults from runtime constants'
rule) and passes it as the unknownSentence to formatRevisionLine in the
create-timeout branch. Updates SPECIFICATION's write-path diagram and helper-
constant inventory; CHANGELOG line generalized to cite both caps.
fetchRevisionsForResults builds an IN-clause with one placeholder per result. Without a schema-level cap on the search limit, a caller passing limit=50000 would build a 50000-placeholder query and (eventually, as SQLite versions shift) hit the bound-parameter ceiling with an opaque 'too many SQL variables' error. Adds MAX_SEARCH_LIMIT=1000 in config.ts, applies .max(MAX_SEARCH_LIMIT) on the bear-search-notes schema, and updates the totalCount paginate hint to cap at the same value (Math.min so small over-flows still suggest the actual totalCount). 1000 sits well under SQLite's current 32766 cap and is generous relative to realistic LLM-driven use (default 30). bear-find-untagged-notes uses a single non-IN-clause query against the live DB, so the parameter-cap concern doesn't apply there.
…h revision hydration (F5, review)" This reverts commit 6147a2c.
…plification) The 'Revision at time of archive: <n>' label distinguished a pre-write snapshot from a live current revision. In practice the distinction is functionally inert — post-archive the note is filtered from default queries, so no later read can reach it to compare against. The agent comparing revisions across calls can't hit this case. Generic 'Revision: <n>' via formatRevisionLine, comment in the handler explains the snapshot nature. tests/system/inspector.ts NOTE_REVISION_REGEX collapsed to the single shape; archive-revision.test.ts asserts on the new line.
Pass over OCC inform code and docs to remove WHAT-narration and condense WHY-explanations per CODE_STYLE.md. Net -54 lines across 15 files; no behavior change. Notable trims: - awaitRevisionIncrement JSDoc (17 lines → 6) — name + signature carry most meaning; comment now focuses on the +2 jump rationale and the one-DB optimization - fetchRevisionsForResults block comment (8 lines → 4) — drift sentinel gap and null-surfacing - responses.ts sentinels — consolidated into one comment block explaining the three failure modes, with the per-constant lookstep rationale - system-test 'settle briefly' comments — standardized to one line each pointing at BEAR_DATABASE_SCHEMA.md - create-note.test.ts +2 jump comment (6 lines → 3) — monotonicity is enough WHY for the >= assertion - CHANGELOG entry — collapsed to user-facing essentials (what shipped, why it helps, scope, edge case, additivity, follow-up)
Sonar quality gate flagged 16.5% duplication on new code. The bulk (86/131
lines) came from awaitRevisionIncrement's and awaitNoteCreation's describe
blocks, which each had their own near-identical beforeEach/afterEach for
file-backed temp DB lifecycle and BEAR_DB_PATH env-var swap.
Extracts setupTempBearDb(schema) returning a { writeDb } ref whose .writeDb
is rebound per test iteration. Both describe blocks now call it with their
respective ZSFNOTE schemas pulled into module-level constants.
Net -56 lines in notes.test.ts; all 104 unit tests still passing.
System tests share the standard vitest file header (imports, TEST_PREFIX, RUN_ID, PAUSE_AFTER_WRITE_OP, afterAll cleanup, opening describe). Sonar's CPD flagged it as duplication; extracting a shared header helper would force indirection on the canonical vitest spec shape. Production sources keep the standard 3% gate; only tests/system/*.test.ts is excluded. Refactor in fc32483 already removed the 86-line bulk from notes.test.ts — this addresses the remaining 45 lines of legitimate header boilerplate.
Extract pollUntilFound primitive shared by awaitRevisionIncrement and awaitNoteCreation. The previous unit tests scheduled a +50ms writer against the helper's wallclock deadline; under event-loop pressure on CI, await setTimeout slipped past the deadline before the writer fired, returning null instead of the new revision (PR #125 CI failure mode). The new tests exercise pollUntilFound directly with synchronous read callbacks — no DB, no scheduled writes, no race. End-to-end coverage of the SQLite integration stays in the system tests (tryExtractRevision assertions in tests/system/*.ts). Also: - Bump REVISION_POLL_CAP_MS from 500ms to 1s. Happy-path latency is unchanged (<30ms typical per BEAR_DATABASE_SCHEMA.md); the extra headroom absorbs event-loop slop on slow machines. - Soften REVISION_TIMEOUT_SENTENCE to drop the specific-ms claim — the post-write safety window is an internal implementation choice the caller doesn't act on. REVISION_CREATION_TIMEOUT_SENTENCE keeps its duration because the create-poll cap IS the user-facing budget.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Revision: <n>line tied to each referenced note ID, sourced from Bear'sZSFNOTE.Z_OPToptimistic-locking counter. An LLM client can compare revisions across calls to detect whether a note changed since it last looked.bear-open-note,bear-search-notes,bear-find-untagged-notes,bear-create-note,bear-add-text,bear-replace-text,bear-add-file,bear-add-tag,bear-archive-note. Out of scope:bear-list-tags,bear-rename-tag,bear-delete-tag(no specific note referenced).add-text,replace-text,create,add-file,add-tag): the response carries the post-write revision, captured by pollingZ_OPTuntil it differs from the pre-flight baseline (capped at REVISION_POLL_CAP_MS, currently 1s). A non-null return is simultaneously write confirmation and the new revision. On poll timeout the line becomes a duration-freeRevision: unknown (the write was issued but observable confirmation did not arrive within the safety window)rather than silently surfacing a stale value.bear-archive-note: the response carries the pre-write revision (captured before the URL fires). Post-archive, the note hasZARCHIVED=1and is filtered from default queries, so a post-write read would return null — preserving the project's no-post-write-reads discipline.open,search,find-untagged): revision is projected directly from the same query that returns the note. Search results in particular hydrate revisions against the live Bear DB (post-FTS batchSELECT) rather than cachingZ_OPTin the FTS shadow — the existing drift sentinelMAX(ZMODIFICATIONDATE) + COUNT(*)misses pin-only and tag-only writes that bumpZ_OPTwithout bumpingZMODIFICATIONDATE, and a cached revision would silently lag.current !== baseline), notbaseline + 1: the first edit after note creation bumpsZ_OPTby +2 because Bear runs a subtitle/index recompute save right after create. A+1comparison would miss the first edit and time out.docs/dev/SPECIFICATION.md). Per-tool system tests assert the response's parsed Revision matchesSELECT Z_OPT FROM ZSFNOTE WHERE ZUNIQUEIDENTIFIER=<id>.Why
This ships the inform half of Optimistic Concurrency Control. Once an LLM client has the revision for a note in hand, it can detect whether the note changed between its read and a follow-up write — the foundation any "did this change under me?" check needs. Without it, multi-turn workflows have no honest signal beyond re-reading and string-diffing the full body.
The companion enforce half (SVA-22) — accepting a revision as an input parameter and rejecting stale writes with a
412-equivalent error — builds on top of the metadata this PR makes available. Shipping inform first lets clients start exchanging revisions immediately, with zero behavior change for clients that ignore the new field.--
Linear: https://linear.app/svasylenko/issue/SVA-21/l1a-occ-inform-flow-every-note-scoped-tool-response-carries-the