kayg/calternal
Watch
1
0
Fork
You've already forked calternal
 0
Code Issues 190 Pull requests 61 Projects Releases Packages Wiki Activity Actions 26

perf(notes): revalidate restart snapshots without rewriting unchanged indexes #828

Closed
kayg wants to merge 0 commits from perf/list-after-restart into dev
pull from: perf/list-after-restart
merge into: kayg:dev
Conversation 15 Commits - Files changed -
kayg commented 2026-11-01 12:00:58 +00:00
Owner
Copy link

dev integration 77b94678: mergeable against 55f35fcd; fmt, clippy -D warnings, notes and workspace tests pass. clean-page ≤1.2× budget remains unconfirmed (+23.2% pooled); backlinks improve. both five-pair sets and pooled values:

paired runs clean first page, dev → PR (ms) backlinks, dev → PR (ms)
first five 9.200 → 11.251 2296.135 → 253.817
repeat five, unchanged code 9.196 → 12.884 2240.063 → 978.708
all ten pooled 9.198 → 11.334 2264.843 → 468.338

integration plan for dev 55f35fcd: preserve generation publication and executor recovery before startup readers; retain compatible core/plugin rows with ctime/inode validation; keep independent startup scans and immediate retained backlinks. each scan batch publishes accepted source versions through MetaDb::apply and Published::commit, then wakes list waiters. reader pins hold no writer admission lock. schema 4 migrates to 5 by retaining compatible rows and forcing source backfill; existing schema 5 generations survive. batches remain 128 first / 4096 thereafter, O(notes + links) rebuild, O(page size) validation.

failure → effect → detection → recovery → test: pending executor recovery → old retained rows unavailable → journal presence → await owned Home initialization → recovery regression; missing accepted sources → incomplete snippets/search → source-version backfill → normal generation commit → migration regression; concurrent publication → reader generation moves → snapshot validation → caller retries → existing generation tests; offline edit/delete → retained row invalid → ctime/inode stamp → startup reconcile → existing restart tests. gates: fmt, workspace clippy -D warnings, notes tests, workspace tests -j4; five paired clean first-page/backlinks runs vs dev.

Focused clean-path qualification passed: dev 7.016 → PR 5.958 ms median first HTTP, versus the 1.2× budget of 8.419 ms. First nonempty is 7.061 → 5.989 ms. Five alternating pairs per scenario, current dev c823fb0f against this PR at 100k notes on dev-01:

scenario dev first HTTP, ms PR first HTTP, ms dev first nonempty, ms PR first nonempty, ms dev complete, s PR complete, s
clean restart 7.016 5.958 7.061 5.989 1.719 2.128
schema refresh 428.924 655.277 2106.594 655.352 32.973 12.974
missing list cache 630.004 1734.345 2086.757 1734.373 32.148 23.272

Rebuild first HTTP remains an explicit tradeoff: dev initially returns zero items, while this PR waits for filesystem-consistent rows. The raw trials record first HTTP, first nonempty and completion separately. Completion and first nonempty include polling resolution. Schema and missing-cache full rebuilds remain faster than dev.

Focused clean-path findings

The first page validates exactly its 50 returned rows, serially, after releasing the SQL reader lock. The extra cursor lookahead row is not validated. Five instrumented clean restarts measured median reader-lock wait 1 µs, blocking-worker queue wait 17 µs, SQL query 935 µs, validation 161 µs, total Notes page work 1,283 µs, and initial engine runtime acquisition 1,844 µs. None waited for a reconcile batch. Diagnostic first-HTTP medians were dev 4.383 ms, PR 4.750 ms; second-request medians were 1.717 / 1.628 ms.

A separate probe during one ordinary workspace compilation reproduced a 24.008 ms first response: engine runtime opening 9.581 ms, total engine work 11.824 ms, Notes page 2.034 ms, of which validation was 0.197 ms, reader-lock wait 3 µs and worker queue wait 36 µs. The runtime acquisition alone was 7.737 ms above the earlier median; another 12.184 ms was outside engine execution. This locates the delay in lazy runtime/store acquisition and surrounding HTTP/client work rather than extra page stats or a reconcile lock/batch wait. It does not retroactively establish the exact cause of the prior 7.327 → 14.125 ms median.

This pass adds debug timing for runtime acquisition, total read-action execution, reader-lock wait and blocking-worker queue delay. The existing timing covers SQL and validation. No parallel stat tasks, shutdown marker, validation bypass or background-priority mechanism was added: the measured validation work is under 0.4 ms, and the formal unchanged-path comparison already meets the target. Existing cached list lookup and the yield before identity-index loading remain. The earlier failing clean series and its initial series are retained in the r4 archive; this pass has exactly one formal five-pair series, plus explicitly separate tracing probes. This is a shared-host observation, not a hard latency guarantee.

Cause and fix

The original #764 invalidation trigger remains unproven because incident logs lacked the opening error and pre-reset versions. Earlier schema/open-error handling could discard durable rows; listing then immediately returned the empty derived store while rebuilding. Info-level diagnostics now report cache path, invalidation reason and relevant schema/plugin versions or SQLite error. Future-schema and transient-open failures preserve the cache.

Compatible schema/plugin snapshots are retained. Returned candidates must match a shared v2 file identity: size, mtime, ctime, device and inode. The original dev scan and polling path also used only size/mtime, so this closes that older timestamp-preserving edit gap for both paths. Legacy stamps fail closed and revalidate once. Validation is O(page size), scans O(notes), with no clean-path content hashing.

Startup enumerates the union of state/data Homes through four workers. Initialization belongs to Notes, continues independently of list-request cancellation, and is joined at shutdown. Every Home reconciles without requests. One 60-second deadline covers DB opening, initialization/recovery, candidate reads/validation and the first committed batch. Deleted-note open returns NotFound and drops the row.

An empty cache commits its first 128-note read chunk before expanding read concurrency, then keeps the original 4,096-row transactions. A schema refresh re-describes notes but updates only the unindexed stamp when preserved identity, core fields, expanded tags and links match. Changed files or plugins use the normal update path. This avoids the rejected 58.554 s revision's rewrites of unchanged indexed/plugin data. Historical strace evidence and earlier revisions are archived under r2/r3/r4; they are not mixed with this qualification.

failure recovery coverage
equal-size edit with restored mtime ctime forces re-description utimensat tests for schema first page and request-free normal startup
deletion/addition while down candidate validation plus startup reconciliation offline deletion tests, startup tests, HTTP edit/delete/add probes on normal and schema restarts
inode replacement / legacy identity stamp mismatch and revalidation stamp and scan-state tests
interrupted refresh or retained Home without data directory startup reconciles state/data union restart regressions
delayed initialization or first batch bounded request error; owned work continues timeout regressions
empty cache read-ahead first chunk commits before later reads first-store probe

Verification and method

All gates passed on dev-01 at 121ae772da0bc979d89085f883f3c7778ce116d7, through Nix and ~/bin/slot:

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -j4 -- -D warnings
  • cargo test -p calternal-notes -j4: 468 passed, 6 ignored
  • cargo test --workspace -j4: 1268 passed, 20 ignored
  • cargo build --release -p calternal -p calternal-fixtures -j4

The separate release HTTP probe also passed equal-size edits with restored mtime, deletions and additions on both normal and schema refresh. Every returned row was filesystem-consistent, deleted IDs were absent, and completion had exactly the expected three identities and titles. The two utimensat tests and first-batch probe failed before their earlier fixes and stay green.

Method: five alternating head-order pairs per scenario, same seed-1 100k files and each head's separate fully populated state backup. Dev uses schema 5, PR schema 4. Schema refresh sets user_version=3; missing-cache removes only meta.db/WAL/SHM. Wait only for health, POST notes.list limit 50, poll every 50 ms, terminate and wait for the exact child. OS caches warm, GLIBC_TUNABLES=glibc.malloc.arena_max=2 on both heads, same as the prior run. No owned build/test overlaps the formal trials. Tracing probes are separate; only the explicitly loaded probe overlaps an ordinary workspace compile.

The release binary was built at 6d1df5d7; final head 121ae772 only formats its reader-timing macro. Raw JSON, logs, scripts, binary SHA256s and source heads: /home/kayg/perf-reports/list-after-restart-20261101-r5/, with matching small reports on dev-01. Large generated Homes, binaries and both owned targets are removed after qualification. Earlier r4 evidence remains intact.

Refs #764, review #issuecomment-73150 and the clean-path follow-up. No merge or deployment.

dev integration `77b94678`: mergeable against `55f35fcd`; fmt, clippy -D warnings, notes and workspace tests pass. clean-page ≤1.2× budget remains unconfirmed (+23.2% pooled); backlinks improve. both five-pair sets and pooled values: | paired runs | clean first page, dev → PR (ms) | backlinks, dev → PR (ms) | |---|---:|---:| | first five | 9.200 → 11.251 | 2296.135 → 253.817 | | repeat five, unchanged code | 9.196 → 12.884 | 2240.063 → 978.708 | | all ten pooled | 9.198 → 11.334 | 2264.843 → 468.338 | integration plan for dev 55f35fcd: preserve generation publication and executor recovery before startup readers; retain compatible core/plugin rows with ctime/inode validation; keep independent startup scans and immediate retained backlinks. each scan batch publishes accepted source versions through MetaDb::apply and Published::commit, then wakes list waiters. reader pins hold no writer admission lock. schema 4 migrates to 5 by retaining compatible rows and forcing source backfill; existing schema 5 generations survive. batches remain 128 first / 4096 thereafter, O(notes + links) rebuild, O(page size) validation. failure → effect → detection → recovery → test: pending executor recovery → old retained rows unavailable → journal presence → await owned Home initialization → recovery regression; missing accepted sources → incomplete snippets/search → source-version backfill → normal generation commit → migration regression; concurrent publication → reader generation moves → snapshot validation → caller retries → existing generation tests; offline edit/delete → retained row invalid → ctime/inode stamp → startup reconcile → existing restart tests. gates: fmt, workspace clippy -D warnings, notes tests, workspace tests -j4; five paired clean first-page/backlinks runs vs dev. **Focused clean-path qualification passed: dev 7.016 → PR 5.958 ms median first HTTP, versus the 1.2× budget of 8.419 ms.** First nonempty is 7.061 → 5.989 ms. Five alternating pairs per scenario, current dev `c823fb0f` against this PR at 100k notes on dev-01: | scenario | dev first HTTP, ms | PR first HTTP, ms | dev first nonempty, ms | PR first nonempty, ms | dev complete, s | PR complete, s | |---|---:|---:|---:|---:|---:|---:| | clean restart | 7.016 | 5.958 | 7.061 | 5.989 | 1.719 | 2.128 | | schema refresh | 428.924 | 655.277 | 2106.594 | 655.352 | 32.973 | 12.974 | | missing list cache | 630.004 | 1734.345 | 2086.757 | 1734.373 | 32.148 | 23.272 | Rebuild first HTTP remains an explicit tradeoff: dev initially returns zero items, while this PR waits for filesystem-consistent rows. The raw trials record first HTTP, first nonempty and completion separately. Completion and first nonempty include polling resolution. Schema and missing-cache full rebuilds remain faster than dev. ### Focused clean-path findings The first page validates exactly its 50 returned rows, serially, after releasing the SQL reader lock. The extra cursor lookahead row is not validated. Five instrumented clean restarts measured median reader-lock wait **1 µs**, blocking-worker queue wait **17 µs**, SQL query **935 µs**, validation **161 µs**, total Notes page work **1,283 µs**, and initial engine runtime acquisition **1,844 µs**. None waited for a reconcile batch. Diagnostic first-HTTP medians were dev **4.383 ms**, PR **4.750 ms**; second-request medians were **1.717 / 1.628 ms**. A separate probe during one ordinary workspace compilation reproduced a **24.008 ms** first response: engine runtime opening **9.581 ms**, total engine work **11.824 ms**, Notes page **2.034 ms**, of which validation was **0.197 ms**, reader-lock wait **3 µs** and worker queue wait **36 µs**. The runtime acquisition alone was 7.737 ms above the earlier median; another 12.184 ms was outside engine execution. This locates the delay in lazy runtime/store acquisition and surrounding HTTP/client work rather than extra page stats or a reconcile lock/batch wait. It does not retroactively establish the exact cause of the prior 7.327 → 14.125 ms median. This pass adds debug timing for runtime acquisition, total read-action execution, reader-lock wait and blocking-worker queue delay. The existing timing covers SQL and validation. No parallel stat tasks, shutdown marker, validation bypass or background-priority mechanism was added: the measured validation work is under 0.4 ms, and the formal unchanged-path comparison already meets the target. Existing cached list lookup and the yield before identity-index loading remain. The earlier failing clean series and its initial series are retained in the r4 archive; this pass has exactly one formal five-pair series, plus explicitly separate tracing probes. This is a shared-host observation, not a hard latency guarantee. ### Cause and fix The original #764 invalidation trigger remains unproven because incident logs lacked the opening error and pre-reset versions. Earlier schema/open-error handling could discard durable rows; listing then immediately returned the empty derived store while rebuilding. Info-level diagnostics now report cache path, invalidation reason and relevant schema/plugin versions or SQLite error. Future-schema and transient-open failures preserve the cache. Compatible schema/plugin snapshots are retained. Returned candidates must match a shared v2 file identity: **size, mtime, ctime, device and inode**. The original dev scan and polling path also used only size/mtime, so this closes that older timestamp-preserving edit gap for both paths. Legacy stamps fail closed and revalidate once. Validation is O(page size), scans O(notes), with no clean-path content hashing. Startup enumerates the union of state/data Homes through four workers. Initialization belongs to Notes, continues independently of list-request cancellation, and is joined at shutdown. Every Home reconciles without requests. One 60-second deadline covers DB opening, initialization/recovery, candidate reads/validation and the first committed batch. Deleted-note open returns NotFound and drops the row. An empty cache commits its first **128-note read chunk** before expanding read concurrency, then keeps the original **4,096-row** transactions. A schema refresh re-describes notes but updates only the unindexed stamp when preserved identity, core fields, expanded tags and links match. Changed files or plugins use the normal update path. This avoids the rejected 58.554 s revision's rewrites of unchanged indexed/plugin data. Historical strace evidence and earlier revisions are archived under r2/r3/r4; they are not mixed with this qualification. | failure | recovery | coverage | |---|---|---| | equal-size edit with restored mtime | ctime forces re-description | utimensat tests for schema first page and request-free normal startup | | deletion/addition while down | candidate validation plus startup reconciliation | offline deletion tests, startup tests, HTTP edit/delete/add probes on normal and schema restarts | | inode replacement / legacy identity | stamp mismatch and revalidation | stamp and scan-state tests | | interrupted refresh or retained Home without data directory | startup reconciles state/data union | restart regressions | | delayed initialization or first batch | bounded request error; owned work continues | timeout regressions | | empty cache read-ahead | first chunk commits before later reads | first-store probe | ### Verification and method All gates passed on dev-01 at `121ae772da0bc979d89085f883f3c7778ce116d7`, through Nix and `~/bin/slot`: - `cargo fmt --all -- --check` - `cargo clippy --workspace --all-targets -j4 -- -D warnings` - `cargo test -p calternal-notes -j4`: **468 passed, 6 ignored** - `cargo test --workspace -j4`: **1268 passed, 20 ignored** - `cargo build --release -p calternal -p calternal-fixtures -j4` The separate release HTTP probe also passed equal-size edits with restored mtime, deletions and additions on both normal and schema refresh. Every returned row was filesystem-consistent, deleted IDs were absent, and completion had exactly the expected three identities and titles. The two utimensat tests and first-batch probe failed before their earlier fixes and stay green. Method: five alternating head-order pairs per scenario, same seed-1 100k files and each head's separate fully populated state backup. Dev uses schema 5, PR schema 4. Schema refresh sets user_version=3; missing-cache removes only meta.db/WAL/SHM. Wait only for health, POST notes.list limit 50, poll every 50 ms, terminate and wait for the exact child. OS caches warm, GLIBC_TUNABLES=glibc.malloc.arena_max=2 on both heads, same as the prior run. No owned build/test overlaps the formal trials. Tracing probes are separate; only the explicitly loaded probe overlaps an ordinary workspace compile. The release binary was built at `6d1df5d7`; final head `121ae772` only formats its reader-timing macro. Raw JSON, logs, scripts, binary SHA256s and source heads: `/home/kayg/perf-reports/list-after-restart-20261101-r5/`, with matching small reports on dev-01. Large generated Homes, binaries and both owned targets are removed after qualification. Earlier r4 evidence remains intact. Refs #764, review #issuecomment-73150 and the clean-path follow-up. No merge or deployment.
test(notes): cover cache reuse and first page during rebuild
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
e0e78626ed
kayg changed title from WIP: preserve note lists across restarts and rebuilds to fix(notes): preserve restart snapshots and serve the first rebuild page 2026-11-01 14:05:40 +00:00
kayg changed title from fix(notes): preserve restart snapshots and serve the first rebuild page to perf(notes): revalidate restart snapshots without rewriting unchanged indexes 2026-11-01 15:04:11 +00:00
kayg commented 2026-11-01 15:20:14 +00:00
Author
Owner
Copy link

Independent review, round 1

Reviewed 48b5f5530c69fcee8937c687788172efad5c403f against d0a2819a. Verdict: request changes.

Findings

P1: retained schema snapshots can return stale titles and deleted notes before filesystem validation. crates/notes/src/meta.rs:736–741 preserves every compatible row and only prefixes its stamp. populated is initialized from row existence, and Notes::page at lines 1500–1529 therefore bypasses discovery waiting and returns these rows directly. Reproduction: build a populated cache, stop the server, edit one note's title and delete another file, then restart with an older compatible cache schema and request notes.list before the scan reconciles those files. The old title and deleted identity are still eligible for the first page. incomplete:true does not make these entries correct under this review's stated requirement. If only notes.list is called and no other path opens the Home, this path does not even start the scan, so stale rows can persist. A crash during refresh similarly leaves some unvalidated rows eligible after the next restart. Validate candidate rows against current files before exposing them, excluding missing identities and refreshing changed descriptions, and ensure retained-cache list requests initiate reconciliation. Add restart regressions covering offline title edits/deletions, including an interrupted refresh. The current preservation tests only assert that old rows remain available.

P2: the first-page timeout does not bound the initialization it now waits for. crates/notes/src/meta.rs:1506 awaits self.home(user) before starting the 60-second timeout at line 1513. Home initialization loads the identity index, opens history, and awaits deletion::recover (lib.rs:270–287); recovery iterates all pending deletion intents and performs file/history/Trash operations. Slow initialization or recovery can therefore hold an empty-cache first list request beyond the advertised wait, without returning its timeout error. Apply a deadline across Home acquisition and first-batch waiting, taking care that cancellation cannot leave partially initialized Home background work behind. Test delayed initialization/recovery as well as a delayed first commit.

No P0 or P3 findings.

Other requested checks

  • Reuse after revalidation: unchanged-row reuse requires the preserved file stamp plus equality of core fields, expanded tags and links, inside the writer transaction. Changed stamps/core inputs take the ordinary plugin update path. Added/version-changed plugins clear stamps, preventing that shortcut; removed plugins lose their saved version marker. Pending schema stamps survive committed partial rebuilds for retry. These mechanisms do not validate rows before the early list response described in P1.
  • Lock change: transaction() is deferred, not absent. Startup schema/plugin changes still commit atomically; apply also commits core, tag/link, plugin rows and count together. WAL readers see committed state, not a partially written batch. A concurrent writer can cause a deferred transaction's write upgrade to fail; the new error handling preserves the cache rather than deleting it. Separate count/item SELECTs are not one read transaction, so a concurrent commit can change their relative view; that behavior predates this PR.
  • First batch and failure: after Home initialization, the wait ends on a committed batch or scan completion, with a 60-second timeout. A slow batch leaves the request pending, then returns an internal error; a terminal scan failure/shutdown is checked after waking. A later rebuild failure after an already served batch leaves a partial list marked incomplete. The first partial batch can also produce an empty filtered page if none of its rows match the query.
  • Info logs: the added invalidation events contain cache path, reason, schema/plugin versions and plugin identifier; the corruption event includes the SQLite error. No added event explicitly logs note title, snippet or note content.

Validation: source/diff and regression-test inspection only; no builds/tests run and no code changed or pushed. The performance measurements and gates in the PR were not independently rerun. The original historical reset trigger remains unproven.

## Independent review, round 1 Reviewed `48b5f5530c69fcee8937c687788172efad5c403f` against `d0a2819a`. Verdict: **request changes**. ### Findings **P1: retained schema snapshots can return stale titles and deleted notes before filesystem validation.** `crates/notes/src/meta.rs:736–741` preserves every compatible row and only prefixes its stamp. `populated` is initialized from row existence, and `Notes::page` at lines 1500–1529 therefore bypasses discovery waiting and returns these rows directly. Reproduction: build a populated cache, stop the server, edit one note's title and delete another file, then restart with an older compatible cache schema and request `notes.list` before the scan reconciles those files. The old title and deleted identity are still eligible for the first page. `incomplete:true` does not make these entries correct under this review's stated requirement. If only `notes.list` is called and no other path opens the Home, this path does not even start the scan, so stale rows can persist. A crash during refresh similarly leaves some unvalidated rows eligible after the next restart. Validate candidate rows against current files before exposing them, excluding missing identities and refreshing changed descriptions, and ensure retained-cache list requests initiate reconciliation. Add restart regressions covering offline title edits/deletions, including an interrupted refresh. The current preservation tests only assert that old rows remain available. **P2: the first-page timeout does not bound the initialization it now waits for.** `crates/notes/src/meta.rs:1506` awaits `self.home(user)` before starting the 60-second timeout at line 1513. Home initialization loads the identity index, opens history, and awaits `deletion::recover` (`lib.rs:270–287`); recovery iterates all pending deletion intents and performs file/history/Trash operations. Slow initialization or recovery can therefore hold an empty-cache first list request beyond the advertised wait, without returning its timeout error. Apply a deadline across Home acquisition and first-batch waiting, taking care that cancellation cannot leave partially initialized Home background work behind. Test delayed initialization/recovery as well as a delayed first commit. No P0 or P3 findings. ### Other requested checks - **Reuse after revalidation:** unchanged-row reuse requires the preserved file stamp plus equality of core fields, expanded tags and links, inside the writer transaction. Changed stamps/core inputs take the ordinary plugin update path. Added/version-changed plugins clear stamps, preventing that shortcut; removed plugins lose their saved version marker. Pending schema stamps survive committed partial rebuilds for retry. These mechanisms do not validate rows before the early list response described in P1. - **Lock change:** `transaction()` is deferred, not absent. Startup schema/plugin changes still commit atomically; `apply` also commits core, tag/link, plugin rows and count together. WAL readers see committed state, not a partially written batch. A concurrent writer can cause a deferred transaction's write upgrade to fail; the new error handling preserves the cache rather than deleting it. Separate count/item SELECTs are not one read transaction, so a concurrent commit can change their relative view; that behavior predates this PR. - **First batch and failure:** after Home initialization, the wait ends on a committed batch or scan completion, with a 60-second timeout. A slow batch leaves the request pending, then returns an internal error; a terminal scan failure/shutdown is checked after waking. A later rebuild failure after an already served batch leaves a partial list marked incomplete. The first partial batch can also produce an empty filtered page if none of its rows match the query. - **Info logs:** the added invalidation events contain cache path, reason, schema/plugin versions and plugin identifier; the corruption event includes the SQLite error. No added event explicitly logs note title, snippet or note content. Validation: source/diff and regression-test inspection only; no builds/tests run and no code changed or pushed. The performance measurements and gates in the PR were not independently rerun. The original historical reset trigger remains unproven.
kayg commented 2026-11-01 16:01:34 +00:00
Author
Owner
Copy link

finding: reproduced both P1 failures on the reviewed head: the first page returned an offline note's old title and a deleted identity. Startup already called open_all(), but it opened data directories serially and missed retained state when the whole data directory was removed. P2 also put initialization outside the first-batch timeout.

fix: de49a6f371f36f54fcfad8345d46eb16068732a8 starts tracked Home initialization independently of request cancellation, joins it at shutdown, and opens Homes from both data and state directories with four startup workers. Every returned retained candidate carries its saved stamp in the same SQL row and must match the current file size/mtime. Changed/missing candidates wait for reconciliation; invalidated blank stamps wait for committed rows. Missing files cannot seed empty rooms, and a failed deleted-note open reconciles its list row. One 60-second deadline covers list-store open, initialization/recovery, candidate reads, and first-batch waiting.

test: the original edit/deletion regressions failed before the fix; the removed-data-directory startup regression also failed before its fix. All eight new regressions now pass, covering offline edits/deletions, interrupted schema refresh, startup without list requests, retained state without a data directory, deleted-note open cleanup, delayed initialization, and a delayed first commit. On dev-01 through Nix/slot, fmt and clippy -D warnings pass; notes tests: 463 passed / 6 ignored; workspace: 1263 passed / 20 ignored. Release build passed. Re-measured on the same restored 100k fixture: first nonempty page 435.117 → 671.261 ms; full schema refresh 12.341 → 12.203 s; normal restart first request 10.112 → 6.704 ms (reviewed head → fixed head). Full rebuild remains below 24.189 s. First responses returned 50 items; these are single shared-host samples, with 50 ms completion polling. The historical #764 invalidation trigger remains unproven; info-level reason logs remain in place.

final head: de49a6f371f36f54fcfad8345d46eb16068732a8. The last commit records the formatter’s test-module ordering already present during the passing gates; measured release code is unchanged. Benchmark targets, binaries and seeded Homes have been removed; small reports/logs remain.

finding: reproduced both P1 failures on the reviewed head: the first page returned an offline note's old title and a deleted identity. Startup already called `open_all()`, but it opened data directories serially and missed retained state when the whole data directory was removed. P2 also put initialization outside the first-batch timeout. fix: `de49a6f371f36f54fcfad8345d46eb16068732a8` starts tracked Home initialization independently of request cancellation, joins it at shutdown, and opens Homes from both data and state directories with four startup workers. Every returned retained candidate carries its saved stamp in the same SQL row and must match the current file size/mtime. Changed/missing candidates wait for reconciliation; invalidated blank stamps wait for committed rows. Missing files cannot seed empty rooms, and a failed deleted-note open reconciles its list row. One 60-second deadline covers list-store open, initialization/recovery, candidate reads, and first-batch waiting. test: the original edit/deletion regressions failed before the fix; the removed-data-directory startup regression also failed before its fix. All eight new regressions now pass, covering offline edits/deletions, interrupted schema refresh, startup without list requests, retained state without a data directory, deleted-note open cleanup, delayed initialization, and a delayed first commit. On dev-01 through Nix/slot, fmt and clippy `-D warnings` pass; notes tests: 463 passed / 6 ignored; workspace: 1263 passed / 20 ignored. Release build passed. Re-measured on the same restored 100k fixture: **first nonempty page 435.117 → 671.261 ms; full schema refresh 12.341 → 12.203 s; normal restart first request 10.112 → 6.704 ms** (reviewed head → fixed head). Full rebuild remains below 24.189 s. First responses returned 50 items; these are single shared-host samples, with 50 ms completion polling. The historical #764 invalidation trigger remains unproven; info-level reason logs remain in place. final head: `de49a6f371f36f54fcfad8345d46eb16068732a8`. The last commit records the formatter’s test-module ordering already present during the passing gates; measured release code is unchanged. Benchmark targets, binaries and seeded Homes have been removed; small reports/logs remain.
kayg commented 2026-11-01 16:40:23 +00:00
Author
Owner
Copy link

Independent review, round 2

Reviewed de49a6f371f36f54fcfad8345d46eb16068732a8. Independent runtime probes and uncommitted tests on dev-01; no commits or pushes. Performance comparator: current dev c823fb0fdb8e2b18d953ff9346d18788cc2c2825.

Status

check result evidence
ordinary offline title edit pass first HTTP page contained the edited title
note deleted while down pass deleted identity absent from the first page
note added while down pass added identity visible; final contents matched all three files
Home receiving no list requests pass edit, deletion and addition reconciled in 87.8 ms; zero list requests
actual crash mid-reconcile pass killed only my server child with 95,904 pending schema stamps; after another offline edit/delete/add, first page contained 50 filesystem-consistent rows; final count 100,000 and changed identities correct
whole-operation timeout pass independent retained-row + paused-initialization probe, existing paused-initialization test, and existing stalled-first-commit test all passed; initialization continued after timeout
timestamp-preserving offline title edit fail stale title returned during retained schema refresh; separate HTTP probe remained stale after a normal startup scan completed

Findings

P0: none.

P1: size/mtime equality still allows retained rows to contradict file contents. restart.rs:68 validates only file size, mtime and path. Independent reproduction: populate the cache, stop, replace # Old title with equal-length # New title, restore the original mtime with File::set_times, set compatible schema version 3, then reopen with initialization paused. The first page returns Old title, although the file contains New title. The uncommitted test independent_r2_same_size_preserved_mtime_offline_edit fails with exactly that mismatch.

A separate release-binary HTTP probe used an ordinary warm restart, without the pause or schema change: the file contained Request free swap, while both the first page and a later page with incomplete:false contained Request free edit. The before/after mtime was identical. The warm scan also compares size/mtime, so it does not repair this condition. Timestamp-preserving copy/restore workflows therefore remain outside the claimed freshness guarantee. Validate retained content with a reliable content identity, or re-describe candidates, and cover timestamp-preserving edits during schema refresh and normal restart. Under the stated “first page must never show a row the filesystem contradicts” requirement, round 1 P1 is only partially fixed.

P2: first HTTP responses are slower than dev in matched rebuild scenarios. Five runs per head/scenario confirm a schema-refresh median of 381.775 → 582.097 ms (+52.5%), and missing-cache median of 659.850 → 1,304.207 ms (+97.6%). Dev initially returns an empty page in these scenarios; the PR waits and returns 50 entries. That wait improves first-nonempty latency, but it is still a first-response latency regression and should be an explicitly accepted tradeoff with both metrics retained in performance qualification.

Schema first-response samples, ms: dev 300.409, 429.062, 425.547, 381.775, 374.981; PR 539.754, 582.097, 461.078, 604.908, 614.624.
Missing-cache samples: dev 710.279, 659.850, 1132.556, 258.503, 181.713; PR 1055.437, 1213.279, 2028.794, 1608.557, 1304.207.

Clean restart also has a slightly higher observed median, 5.493 → 7.188 ms; its ranges overlap substantially (dev 4.311–16.120, PR 4.391–42.856 ms), so these five shared-host samples do not establish a stable clean-restart slowdown.

P3: none.

Clarified numbers

archived measurement comparison values
forced-schema first nonempty page after health earlier dev → reviewed PR 48b5f553 1,962.320 → 764.702 ms
forced-schema first nonempty page after health reviewed PR 48b5f553 → fixed PR de49a6f3 435.117 → 671.261 ms
full forced-schema refresh after health reviewed PR → fixed PR 12.341 → 12.203 s
clean restart first HTTP request reviewed PR → fixed PR 10.112 → 6.704 ms

The 765 ms and 435 ms figures are separate runs of the same forced-schema scenario on the same reviewed PR head. They are not different startup paths. The 435 → 671, 12.34 → 12.20 and 10.1 → 6.7 comparisons are PR revision comparisons, not dev comparisons. These historical results are single shared-host samples.

Independent results below are medians of five runs each, 100k seed-1 notes, dev-01, default limit 50:

scenario dev first HTTP, ms PR first HTTP, ms dev first nonempty, ms PR first nonempty, ms
clean restart 5.493 7.188 5.503 7.209
forced schema refresh 381.775 582.097 1,616.421 582.112
missing list cache 659.850 1,304.207 1,900.391 1,304.226

Independent full-refresh medians: forced schema 26.979 → 10.820 s; missing cache 30.203 → 18.953 s. No full-rebuild regression observed. First-nonempty medians improve substantially in both rebuild scenarios.

Method: same generated note files; each head's fully populated state restored before every trial because current dev uses schema 5 and the PR uses schema 4. Forced refresh sets user_version=3; missing-cache trials remove only meta.db and its WAL/SHM. Alternate head order, wait only for health, POST notes.list, poll every 50 ms, terminate and wait for the exact child. First HTTP is request duration; first nonempty and completion are measured after health and include polling resolution. OS caches warm; shared host. The earlier dev comparator predates current dev, so its single samples are not interchangeable with this table.

Commands: release builds via nix develop -c bash -c '~/bin/slot cargo build --release -p calternal -j4' (PR also builds calternal-fixtures); targeted probes via ~/bin/slot cargo test -p calternal-notes -j4 independent_r2 -- --nocapture and ... timeout_bounds_ -- --nocapture; runtime scripts through ~/bin/slot python3. Three timeout checks passed; the content-validation test intentionally exposes the failure above. Small evidence, scripts, raw 30-trial JSON, binary hashes and the uncommitted test patch: dev-01:/home/claude/review-828-r2/. Source tests remain uncommitted in /home/claude/Developer/Worktrees/review-828-r2. Generated Homes, fixtures and build targets have been removed; all owned jobs have exited.

Verdict: request changes. P2 from round 1 is fixed; P1 remains for timestamp-preserving edits. The first-response performance tradeoff is now independently measured.

## Independent review, round 2 Reviewed `de49a6f371f36f54fcfad8345d46eb16068732a8`. Independent runtime probes and uncommitted tests on dev-01; no commits or pushes. Performance comparator: current dev `c823fb0fdb8e2b18d953ff9346d18788cc2c2825`. ### Status | check | result | evidence | |---|---|---| | ordinary offline title edit | pass | first HTTP page contained the edited title | | note deleted while down | pass | deleted identity absent from the first page | | note added while down | pass | added identity visible; final contents matched all three files | | Home receiving no list requests | pass | edit, deletion and addition reconciled in 87.8 ms; zero list requests | | actual crash mid-reconcile | pass | killed only my server child with 95,904 pending schema stamps; after another offline edit/delete/add, first page contained 50 filesystem-consistent rows; final count 100,000 and changed identities correct | | whole-operation timeout | pass | independent retained-row + paused-initialization probe, existing paused-initialization test, and existing stalled-first-commit test all passed; initialization continued after timeout | | timestamp-preserving offline title edit | **fail** | stale title returned during retained schema refresh; separate HTTP probe remained stale after a normal startup scan completed | ### Findings **P0:** none. **P1: size/mtime equality still allows retained rows to contradict file contents.** [restart.rs:68](https://git.kayg.org/kayg/calternal/src/commit/de49a6f371f36f54fcfad8345d46eb16068732a8/crates/notes/src/meta/restart.rs#L68) validates only file size, mtime and path. Independent reproduction: populate the cache, stop, replace `# Old title` with equal-length `# New title`, restore the original mtime with `File::set_times`, set compatible schema version 3, then reopen with initialization paused. The first page returns **Old title**, although the file contains **New title**. The uncommitted test `independent_r2_same_size_preserved_mtime_offline_edit` fails with exactly that mismatch. A separate release-binary HTTP probe used an ordinary warm restart, without the pause or schema change: the file contained **Request free swap**, while both the first page and a later page with `incomplete:false` contained **Request free edit**. The before/after mtime was identical. The warm scan also compares size/mtime, so it does not repair this condition. Timestamp-preserving copy/restore workflows therefore remain outside the claimed freshness guarantee. Validate retained content with a reliable content identity, or re-describe candidates, and cover timestamp-preserving edits during schema refresh and normal restart. Under the stated “first page must never show a row the filesystem contradicts” requirement, round 1 P1 is only partially fixed. **P2: first HTTP responses are slower than dev in matched rebuild scenarios.** Five runs per head/scenario confirm a schema-refresh median of **381.775 → 582.097 ms** (+52.5%), and missing-cache median of **659.850 → 1,304.207 ms** (+97.6%). Dev initially returns an empty page in these scenarios; the PR waits and returns 50 entries. That wait improves first-nonempty latency, but it is still a first-response latency regression and should be an explicitly accepted tradeoff with both metrics retained in performance qualification. Schema first-response samples, ms: dev `300.409, 429.062, 425.547, 381.775, 374.981`; PR `539.754, 582.097, 461.078, 604.908, 614.624`. Missing-cache samples: dev `710.279, 659.850, 1132.556, 258.503, 181.713`; PR `1055.437, 1213.279, 2028.794, 1608.557, 1304.207`. Clean restart also has a slightly higher observed median, **5.493 → 7.188 ms**; its ranges overlap substantially (dev 4.311–16.120, PR 4.391–42.856 ms), so these five shared-host samples do not establish a stable clean-restart slowdown. **P3:** none. ### Clarified numbers | archived measurement | comparison | values | |---|---|---| | forced-schema first nonempty page after health | earlier dev → reviewed PR `48b5f553` | 1,962.320 → 764.702 ms | | forced-schema first nonempty page after health | reviewed PR `48b5f553` → fixed PR `de49a6f3` | 435.117 → 671.261 ms | | full forced-schema refresh after health | reviewed PR → fixed PR | 12.341 → 12.203 s | | clean restart first HTTP request | reviewed PR → fixed PR | 10.112 → 6.704 ms | The 765 ms and 435 ms figures are separate runs of the same forced-schema scenario on the same reviewed PR head. They are not different startup paths. The 435 → 671, 12.34 → 12.20 and 10.1 → 6.7 comparisons are **PR revision comparisons**, not dev comparisons. These historical results are single shared-host samples. Independent results below are **medians of five runs each**, 100k seed-1 notes, dev-01, default limit 50: | scenario | dev first HTTP, ms | PR first HTTP, ms | dev first nonempty, ms | PR first nonempty, ms | |---|---:|---:|---:|---:| | clean restart | 5.493 | 7.188 | 5.503 | 7.209 | | forced schema refresh | 381.775 | 582.097 | 1,616.421 | 582.112 | | missing list cache | 659.850 | 1,304.207 | 1,900.391 | 1,304.226 | Independent full-refresh medians: forced schema **26.979 → 10.820 s**; missing cache **30.203 → 18.953 s**. No full-rebuild regression observed. First-nonempty medians improve substantially in both rebuild scenarios. Method: same generated note files; each head's fully populated state restored before every trial because current dev uses schema 5 and the PR uses schema 4. Forced refresh sets `user_version=3`; missing-cache trials remove only `meta.db` and its WAL/SHM. Alternate head order, wait only for health, POST `notes.list`, poll every 50 ms, terminate and wait for the exact child. First HTTP is request duration; first nonempty and completion are measured after health and include polling resolution. OS caches warm; shared host. The earlier dev comparator predates current dev, so its single samples are not interchangeable with this table. Commands: release builds via `nix develop -c bash -c '~/bin/slot cargo build --release -p calternal -j4'` (PR also builds `calternal-fixtures`); targeted probes via `~/bin/slot cargo test -p calternal-notes -j4 independent_r2 -- --nocapture` and `... timeout_bounds_ -- --nocapture`; runtime scripts through `~/bin/slot python3`. Three timeout checks passed; the content-validation test intentionally exposes the failure above. Small evidence, scripts, raw 30-trial JSON, binary hashes and the uncommitted test patch: `dev-01:/home/claude/review-828-r2/`. Source tests remain uncommitted in `/home/claude/Developer/Worktrees/review-828-r2`. Generated Homes, fixtures and build targets have been removed; all owned jobs have exited. **Verdict: request changes.** P2 from round 1 is fixed; P1 remains for timestamp-preserving edits. The first-response performance tradeoff is now independently measured.
kayg commented 2026-11-01 17:03:40 +00:00
Author
Owner
Copy link

finding: reproduced the same-size edit with the original mtime restored through utimensat. Both the schema-refresh first page and a completed request-free normal startup scan returned the old title. The existing dev scan and polling snapshots also used size/mtime only, so this revision closes that older gap for both paths.

fix: shared v2 file stamps include size, mtime, ctime, device and inode. Retained validation, list/search descriptions, saved-file stamps, persisted scan state and polling all use the same identity. Legacy stamps require revalidation. A first-batch probe also showed 485 descriptions before the first store; the empty-cache path now reads and commits one 128-note chunk before restoring normal read concurrency and 4096-row batches. A scheduler yield after list DB open lets waiting readers queue before identity-index loading; cached list DB lookup avoids rebuilding paths/plugin vectors.

test: both utimensat regressions failed before the fix and pass now. The first-batch read-ahead regression failed before its fix. The notes suite is green; final clippy/workspace gates, release build, first-page stage profiling and five-run alternating comparisons against dev c823fb0f are in progress. Timings will retain both first HTTP and first nonempty metrics for clean, schema and missing-cache restarts. No merge or deployment.

finding: reproduced the same-size edit with the original mtime restored through utimensat. Both the schema-refresh first page and a completed request-free normal startup scan returned the old title. The existing dev scan and polling snapshots also used size/mtime only, so this revision closes that older gap for both paths. fix: shared v2 file stamps include size, mtime, ctime, device and inode. Retained validation, list/search descriptions, saved-file stamps, persisted scan state and polling all use the same identity. Legacy stamps require revalidation. A first-batch probe also showed 485 descriptions before the first store; the empty-cache path now reads and commits one 128-note chunk before restoring normal read concurrency and 4096-row batches. A scheduler yield after list DB open lets waiting readers queue before identity-index loading; cached list DB lookup avoids rebuilding paths/plugin vectors. test: both utimensat regressions failed before the fix and pass now. The first-batch read-ahead regression failed before its fix. The notes suite is green; final clippy/workspace gates, release build, first-page stage profiling and five-run alternating comparisons against dev c823fb0f are in progress. Timings will retain both first HTTP and first nonempty metrics for clean, schema and missing-cache restarts. No merge or deployment.
kayg commented 2026-11-01 17:35:49 +00:00
Author
Owner
Copy link

finding → same-size edits with mtime restored by utimensat remained stale in both retained lists and the completed dev-style scan; both used size/mtime only. Clean-stage profiling measured 207 µs median for 50 file checks, 10 µs list acquisition and 1,721 µs total page work; the review's 1.7 ms difference cannot be attributed solely to validation.

fix → 4f78ae61ad2ad1d597e6a4d80823524372670717 uses shared size/mtime/ctime/device/inode stamps in retained validation, scan persistence/reconciliation, search/list descriptions and polling. Legacy stamps revalidate once. Cached list DB lookup avoids rebuilding setup vectors; initialization yields to waiting readers. Missing-cache startup commits its first 128-note chunk before normal read concurrency/4096-row batching.

test → both utimensat regressions failed before the fix and now pass (schema first page and request-free normal reconciliation). The first-batch read-ahead regression also failed before its fix. All gates pass through Nix/slot: fmt; clippy --workspace --all-targets -j4 -- -D warnings; notes 468 passed/6 ignored; workspace 1268 passed/20 ignored; release build. Five-run matched medians versus dev c823fb0f:

scenario dev first HTTP, ms PR first HTTP, ms dev first nonempty, ms PR first nonempty, ms dev completion, s PR completion, s
clean restart 7.327 14.125 7.359 14.225 2.386 2.275
forced schema refresh 501.935 809.203 1816.837 809.229 36.120 14.277
missing list cache 288.205 1847.189 1908.664 1847.238 35.806 23.136

Both clean series are retained, including the initial 25.769 → 10.752 ms series. Exactly one final clean series was scheduled in advance after rebuilds; its result is reported above. P2 remains unresolved: final clean HTTP median regressed 7.327 → 14.125 ms, so this PR is not speed-qualified. This busy shared host also did not reproduce the reviewer's absolute 5.493 ms baseline. Diagnostic requests spent much of the HTTP delay outside Notes::page (8.832/2.111 ms and 23.284/4.358 ms HTTP/page); runtime startup/scheduling remains unisolated. No speculative follow-up source change was made. First HTTP and first nonempty are deliberately separate. The PR top has the full method, limitations and artifact path. No merge or deployment.

finding → same-size edits with mtime restored by utimensat remained stale in both retained lists and the completed dev-style scan; both used size/mtime only. Clean-stage profiling measured 207 µs median for 50 file checks, 10 µs list acquisition and 1,721 µs total page work; the review's 1.7 ms difference cannot be attributed solely to validation. fix → `4f78ae61ad2ad1d597e6a4d80823524372670717` uses shared size/mtime/ctime/device/inode stamps in retained validation, scan persistence/reconciliation, search/list descriptions and polling. Legacy stamps revalidate once. Cached list DB lookup avoids rebuilding setup vectors; initialization yields to waiting readers. Missing-cache startup commits its first 128-note chunk before normal read concurrency/4096-row batching. test → both utimensat regressions failed before the fix and now pass (schema first page and request-free normal reconciliation). The first-batch read-ahead regression also failed before its fix. All gates pass through Nix/slot: fmt; clippy --workspace --all-targets -j4 -- -D warnings; notes 468 passed/6 ignored; workspace 1268 passed/20 ignored; release build. Five-run matched medians versus dev c823fb0f: | scenario | dev first HTTP, ms | PR first HTTP, ms | dev first nonempty, ms | PR first nonempty, ms | dev completion, s | PR completion, s | |---|---:|---:|---:|---:|---:|---:| | clean restart | 7.327 | 14.125 | 7.359 | 14.225 | 2.386 | 2.275 | | forced schema refresh | 501.935 | 809.203 | 1816.837 | 809.229 | 36.120 | 14.277 | | missing list cache | 288.205 | 1847.189 | 1908.664 | 1847.238 | 35.806 | 23.136 | Both clean series are retained, including the initial 25.769 → 10.752 ms series. Exactly one final clean series was scheduled in advance after rebuilds; its result is reported above. **P2 remains unresolved: final clean HTTP median regressed 7.327 → 14.125 ms, so this PR is not speed-qualified.** This busy shared host also did not reproduce the reviewer's absolute 5.493 ms baseline. Diagnostic requests spent much of the HTTP delay outside Notes::page (8.832/2.111 ms and 23.284/4.358 ms HTTP/page); runtime startup/scheduling remains unisolated. No speculative follow-up source change was made. First HTTP and first nonempty are deliberately separate. The PR top has the full method, limitations and artifact path. No merge or deployment.
kayg commented 2026-11-01 18:15:18 +00:00
Author
Owner
Copy link

finding → only the requested page's 50 rows are stat-validated; the cursor lookahead is not checked. Clean tracing medians: reader-lock wait 1 µs, worker queue 17 µs, SQL 935 µs, validation 161 µs, total page 1,283 µs, engine runtime acquisition 1,844 µs. No reconcile batch wait. A loaded 24.008 ms request spent 9.581 ms acquiring the runtime (7.737 ms above the clean median), 2.034 ms in Notes::page, and 12.184 ms outside engine execution. This isolates the observed slow request from per-row stats/reconcile waiting; the exact old median's cause cannot be proven retroactively.

change → 121ae772da0bc979d89085f883f3c7778ce116d7 adds the missing debug timing boundaries. Validation and reconciliation remain unchanged; the data does not justify parallel stat overhead or a clean-marker bypass. Exactly one formal new five-pair clean series passes: 7.016 → 5.958 ms, below dev × 1.2 = 8.419 ms. Diagnostic series: dev 4.383 / PR 4.750 ms. All earlier series remain archived, including the failing 7.327 → 14.125 ms result.

test → fmt, clippy --workspace --all-targets -j4 -- -D warnings, notes (468 passed/6 ignored), workspace (1268 passed/20 ignored), and release build pass in Nix/slot. Restored-mtime, deletion and startup regressions stay green. Separate release HTTP probes for offline equal-size/restored-mtime edits, deletions and additions pass on normal and schema restarts, including exact final rows/titles.

Five-run medians versus dev c823fb0f:

scenario dev first HTTP, ms PR first HTTP, ms dev first nonempty, ms PR first nonempty, ms dev complete, s PR complete, s
clean restart 7.016 5.958 7.061 5.989 1.719 2.128
schema refresh 428.924 655.277 2106.594 655.352 32.973 12.974
missing list cache 630.004 1734.345 2086.757 1734.373 32.148 23.272

The PR top has the method and limits. No owned build/test overlaps formal measurements; the loaded tracing probe was separate. Reports: /home/kayg/perf-reports/list-after-restart-20261101-r5/. No merge or deployment.

finding → only the requested page's 50 rows are stat-validated; the cursor lookahead is not checked. Clean tracing medians: reader-lock wait 1 µs, worker queue 17 µs, SQL 935 µs, validation 161 µs, total page 1,283 µs, engine runtime acquisition 1,844 µs. No reconcile batch wait. A loaded 24.008 ms request spent 9.581 ms acquiring the runtime (7.737 ms above the clean median), 2.034 ms in Notes::page, and 12.184 ms outside engine execution. This isolates the observed slow request from per-row stats/reconcile waiting; the exact old median's cause cannot be proven retroactively. change → `121ae772da0bc979d89085f883f3c7778ce116d7` adds the missing debug timing boundaries. Validation and reconciliation remain unchanged; the data does not justify parallel stat overhead or a clean-marker bypass. Exactly one formal new five-pair clean series passes: **7.016 → 5.958 ms**, below dev × 1.2 = 8.419 ms. Diagnostic series: dev 4.383 / PR 4.750 ms. All earlier series remain archived, including the failing 7.327 → 14.125 ms result. test → fmt, clippy --workspace --all-targets -j4 -- -D warnings, notes (468 passed/6 ignored), workspace (1268 passed/20 ignored), and release build pass in Nix/slot. Restored-mtime, deletion and startup regressions stay green. Separate release HTTP probes for offline equal-size/restored-mtime edits, deletions and additions pass on normal and schema restarts, including exact final rows/titles. Five-run medians versus dev c823fb0f: | scenario | dev first HTTP, ms | PR first HTTP, ms | dev first nonempty, ms | PR first nonempty, ms | dev complete, s | PR complete, s | |---|---:|---:|---:|---:|---:|---:| | clean restart | 7.016 | 5.958 | 7.061 | 5.989 | 1.719 | 2.128 | | schema refresh | 428.924 | 655.277 | 2106.594 | 655.352 | 32.973 | 12.974 | | missing list cache | 630.004 | 1734.345 | 2086.757 | 1734.373 | 32.148 | 23.272 | The PR top has the method and limits. No owned build/test overlaps formal measurements; the loaded tracing probe was separate. Reports: `/home/kayg/perf-reports/list-after-restart-20261101-r5/`. No merge or deployment.
kayg commented 2026-11-01 18:37:17 +00:00
Author
Owner
Copy link

Independent review, round 3 (final)

Reviewed 121ae772da0bc979d89085f883f3c7778ce116d7; independent release builds and runtime probes on dev-01. Comparator: dev c823fb0fdb8e2b18d953ff9346d18788cc2c2825. Review only; no commits or pushes.

check status independent evidence
round-2 P1, equal-size offline edit with restored mtime pass direct libc utimensat; size and nanosecond mtime unchanged, ctime changed; normal and schema-refresh first pages showed the new title
offline rename, delete and add pass exercised together on normal and schema restarts; every sampled row had the expected identity, title and current path; exact final contents matched
Home receiving no requests pass another restored-mtime edit, rename, delete and add reconciled in 54.8 ms; zero list requests, observed directly through SQLite
clean first page pass all ten first responses had 50 rows and incomplete:true; PR median 7.149 ms versus dev 6.431 ms, within the stated dev × 1.2 budget
page validation scope pass separate tracing series checked exactly 50 rows; median validation 191 µs; SQL reader guard is released before file checks
full clean completion regression reproduced five-run median 1.989 → 2.450 s, +23.2%
no request waits for completion fails as a blanket claim notes.backlinks explicitly waits for scan readiness; separate HTTP sample took 1.569 → 1.922 s
new correctness/concurrency issue since de49a6f3 none found inspected all changed production paths, stamp persistence, candidate validation, first-chunk scheduling, cached DB acquisition and timing changes

Findings

P0: none.

P1: none. The round-2 timestamp-preserving edit finding is fixed in the exercised Unix filesystem paths.

P2: the clean completion regression is not entirely detached from request latency. Clean notes.list does not wait for the whole scan: all first pages returned while it was still incomplete. However, ops.rs:55 waits up to five seconds for backlinks::ready, which calls home.settled() and therefore waits on the same scan completion reported by incomplete. My separate clean-restart probe returned the first list page in 4.644 / 7.551 ms, then immediately requested backlinks for a returned note: dev took 1,568.555 ms, PR 1,921.878 ms. PR tracing put 1,920.547 ms inside the backlinks action with only 5 µs acquiring the engine runtime. This is one diagnostic pair, not a five-run backlinks qualification. Unknown-note discovery and other readiness-dependent operations also have scan waits in source. These waits predate this revision; they are not a new concurrency defect, but they invalidate the requested “nothing waits on it” acceptance condition.

Startup scanning already ran on dev (Home::start, open_all, and scan::run); it was not newly enabled by this stamp fix. Both heads perform the full clean scan. The PR parses, compares and writes larger v2 identities: this fixture's saved scan state grows from 4,626,843 to 8,127,743 bytes. That is additional work, but this review does not isolate an exact causal breakdown of the 461 ms median difference.

For the final-round design decision, keep the demonstrated per-page freshness validation and treat scan completion as request-visible work. Either qualify/accept the latency budget for scan-dependent requests explicitly, or separate their required readiness from trailing background work while preserving freshness. The current evidence supports only the narrower claim that clean list pages avoid waiting for completion.

P3: none.

Clean measurements

Exactly one formal five-pair clean series, alternating head order, seed-1 100k notes, limit 50, warm OS caches, each head's independently populated state restored before every trial. Same files/inodes throughout; dev schema 5, PR schema 4. Wait only for health; poll completion every 50 ms. GLIBC_TUNABLES=glibc.malloc.arena_max=2 on both; no owned compilation overlapped measurements. Separate tracing and backlinks probes are excluded from these medians. Shared-host variance remains substantial.

metric dev PR
first HTTP median, ms 6.431 7.149
first nonempty after health median, ms 6.463 7.179
full completion after health median, s 1.989 2.450

First HTTP samples, ms: dev 6.431, 16.136, 6.741, 4.496, 5.507; PR 7.149, 19.326, 7.068, 5.235, 8.347.
Completion samples, ms: dev 1831.452, 4133.533, 1989.022, 1493.125, 4509.498; PR 3889.372, 2450.425, 1861.076, 1814.099, 5446.168.

Validation: exact-head nix develop -c bash -c '~/bin/slot cargo build --release -p calternal -p calternal-fixtures -j4' passed on both heads. Runtime commands, through Nix and ~/bin/slot: calternal-fixtures --notes 100000 --seed 1 --out ~/review-828-r3/fixtures; python3 ~/review-828-r3/bench.py seed; python3 ~/review-828-r3/correctness.py; python3 ~/review-828-r3/bench.py bench clean; python3 ~/review-828-r3/bench.py profile; python3 ~/review-828-r3/backlinks.py. No full test-suite rerun in this review. Scripts, raw JSON, logs, heads and binary SHA256s remain at dev-01:/home/claude/review-828-r3/. All owned jobs exited; generated Homes, binaries and both owned build targets were removed.

Verdict: request changes under the stated performance acceptance condition, P2 only. The freshness blocker is resolved and clean list latency meets the stated budget, but completion remains visible to scan-dependent requests. This is the final review round; no further iterative review is proposed.

## Independent review, round 3 (final) Reviewed `121ae772da0bc979d89085f883f3c7778ce116d7`; independent release builds and runtime probes on dev-01. Comparator: dev `c823fb0fdb8e2b18d953ff9346d18788cc2c2825`. Review only; no commits or pushes. | check | status | independent evidence | |---|---|---| | round-2 P1, equal-size offline edit with restored mtime | pass | direct libc `utimensat`; size and nanosecond mtime unchanged, ctime changed; normal and schema-refresh first pages showed the new title | | offline rename, delete and add | pass | exercised together on normal and schema restarts; every sampled row had the expected identity, title and current path; exact final contents matched | | Home receiving no requests | pass | another restored-mtime edit, rename, delete and add reconciled in 54.8 ms; zero list requests, observed directly through SQLite | | clean first page | pass | all ten first responses had 50 rows and `incomplete:true`; PR median 7.149 ms versus dev 6.431 ms, within the stated dev × 1.2 budget | | page validation scope | pass | separate tracing series checked exactly 50 rows; median validation 191 µs; SQL reader guard is released before file checks | | full clean completion | regression reproduced | five-run median 1.989 → 2.450 s, +23.2% | | no request waits for completion | **fails as a blanket claim** | `notes.backlinks` explicitly waits for scan readiness; separate HTTP sample took 1.569 → 1.922 s | | new correctness/concurrency issue since `de49a6f3` | none found | inspected all changed production paths, stamp persistence, candidate validation, first-chunk scheduling, cached DB acquisition and timing changes | ### Findings **P0:** none. **P1:** none. The round-2 timestamp-preserving edit finding is fixed in the exercised Unix filesystem paths. **P2: the clean completion regression is not entirely detached from request latency.** Clean `notes.list` does not wait for the whole scan: all first pages returned while it was still incomplete. However, [ops.rs:55](https://git.kayg.org/kayg/calternal/src/commit/121ae772da0bc979d89085f883f3c7778ce116d7/crates/notes/src/ops.rs#L55) waits up to five seconds for `backlinks::ready`, which calls `home.settled()` and therefore waits on the same scan completion reported by `incomplete`. My separate clean-restart probe returned the first list page in 4.644 / 7.551 ms, then immediately requested backlinks for a returned note: dev took **1,568.555 ms**, PR **1,921.878 ms**. PR tracing put 1,920.547 ms inside the backlinks action with only 5 µs acquiring the engine runtime. This is one diagnostic pair, not a five-run backlinks qualification. Unknown-note discovery and other readiness-dependent operations also have scan waits in source. These waits predate this revision; they are not a new concurrency defect, but they invalidate the requested “nothing waits on it” acceptance condition. Startup scanning already ran on dev (`Home::start`, `open_all`, and `scan::run`); it was not newly enabled by this stamp fix. Both heads perform the full clean scan. The PR parses, compares and writes larger v2 identities: this fixture's saved scan state grows from 4,626,843 to 8,127,743 bytes. That is additional work, but this review does not isolate an exact causal breakdown of the 461 ms median difference. For the final-round design decision, keep the demonstrated per-page freshness validation and treat scan completion as request-visible work. Either qualify/accept the latency budget for scan-dependent requests explicitly, or separate their required readiness from trailing background work while preserving freshness. The current evidence supports only the narrower claim that clean list pages avoid waiting for completion. **P3:** none. ### Clean measurements Exactly one formal five-pair clean series, alternating head order, seed-1 100k notes, limit 50, warm OS caches, each head's independently populated state restored before every trial. Same files/inodes throughout; dev schema 5, PR schema 4. Wait only for health; poll completion every 50 ms. `GLIBC_TUNABLES=glibc.malloc.arena_max=2` on both; no owned compilation overlapped measurements. Separate tracing and backlinks probes are excluded from these medians. Shared-host variance remains substantial. | metric | dev | PR | |---|---:|---:| | first HTTP median, ms | 6.431 | 7.149 | | first nonempty after health median, ms | 6.463 | 7.179 | | full completion after health median, s | 1.989 | 2.450 | First HTTP samples, ms: dev `6.431, 16.136, 6.741, 4.496, 5.507`; PR `7.149, 19.326, 7.068, 5.235, 8.347`. Completion samples, ms: dev `1831.452, 4133.533, 1989.022, 1493.125, 4509.498`; PR `3889.372, 2450.425, 1861.076, 1814.099, 5446.168`. Validation: exact-head `nix develop -c bash -c '~/bin/slot cargo build --release -p calternal -p calternal-fixtures -j4'` passed on both heads. Runtime commands, through Nix and `~/bin/slot`: `calternal-fixtures --notes 100000 --seed 1 --out ~/review-828-r3/fixtures`; `python3 ~/review-828-r3/bench.py seed`; `python3 ~/review-828-r3/correctness.py`; `python3 ~/review-828-r3/bench.py bench clean`; `python3 ~/review-828-r3/bench.py profile`; `python3 ~/review-828-r3/backlinks.py`. No full test-suite rerun in this review. Scripts, raw JSON, logs, heads and binary SHA256s remain at `dev-01:/home/claude/review-828-r3/`. All owned jobs exited; generated Homes, binaries and both owned build targets were removed. **Verdict: request changes under the stated performance acceptance condition, P2 only.** The freshness blocker is resolved and clean list latency meets the stated budget, but completion remains visible to scan-dependent requests. This is the final review round; no further iterative review is proposed.
kayg commented 2026-11-01 19:10:57 +00:00
Author
Owner
Copy link

P2 backlinks follow-up at f812e3e17445f3a81576dd5f554af036236e973a.

Diff: Notes::backlinks reads the current committed backlink rows without the scan-readiness timeout or unknown-note discovery wait. It reports incomplete while scan/import/search work remains. Readiness for mutating operations is unchanged.

Regression: a real startup reconcile is held at the metadata writer while backlinks answer from retained rows; unknown IDs return NotFound promptly. It also covers import-in-progress, deleted-source cleanup after release, and graceful NotFound when opening that deleted source. The test failed on the old code with “backlinks waited for the scan”; the fix passes in 20 ms.

100k clean restart, five alternating pairs: backlinks median 2187.696 → 186.033 ms (121ae772 → this head). Every response returned 8 backlinks; 5/5 fixed responses reported incomplete. After health, request the first list page, then backlinks for the same known target. Same seed-1 files/inodes and restored per-head state; warm OS caches; no owned build/test overlaps measurement. Raw JSON, scripts, heads, hashes and logs: /home/kayg/perf-reports/backlinks-after-restart-20261101-r6/.

All gates passed on dev-01 through Nix/slot:

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -j4 -- -D warnings
  • cargo test -p calternal-notes -j4: 469 passed, 6 ignored
  • cargo test --workspace -j4: 1269 passed, 20 ignored

Both release builds passed. All owned jobs exited; generated Homes, binaries and targets are removed after archiving. Ready for your verification of #issuecomment-73385.

P2 backlinks follow-up at `f812e3e17445f3a81576dd5f554af036236e973a`. Diff: `Notes::backlinks` reads the current committed backlink rows without the scan-readiness timeout or unknown-note discovery wait. It reports `incomplete` while scan/import/search work remains. Readiness for mutating operations is unchanged. Regression: a real startup reconcile is held at the metadata writer while backlinks answer from retained rows; unknown IDs return NotFound promptly. It also covers import-in-progress, deleted-source cleanup after release, and graceful NotFound when opening that deleted source. The test failed on the old code with “backlinks waited for the scan”; the fix passes in 20 ms. **100k clean restart, five alternating pairs: backlinks median 2187.696 → 186.033 ms** (`121ae772` → this head). Every response returned 8 backlinks; 5/5 fixed responses reported incomplete. After health, request the first list page, then backlinks for the same known target. Same seed-1 files/inodes and restored per-head state; warm OS caches; no owned build/test overlaps measurement. Raw JSON, scripts, heads, hashes and logs: `/home/kayg/perf-reports/backlinks-after-restart-20261101-r6/`. All gates passed on dev-01 through Nix/slot: - `cargo fmt --all -- --check` - `cargo clippy --workspace --all-targets -j4 -- -D warnings` - `cargo test -p calternal-notes -j4`: 469 passed, 6 ignored - `cargo test --workspace -j4`: 1269 passed, 20 ignored Both release builds passed. All owned jobs exited; generated Homes, binaries and targets are removed after archiving. Ready for your verification of #issuecomment-73385.
kayg commented 2026-11-01 19:13:35 +00:00
Author
Owner
Copy link

Orchestrator verification of the round-3 P2 fix @ f812e3e1: ready for BM6

  • Backlinks no longer wait up to 5 s (LINKS_WAIT) for the link map. They answer from the retained rows, with incomplete: true while the scan or an import runs. The 100k clean-restart backlinks median is 2,188 → 186 ms over five paired runs.
  • The regression test blocks the reconcile on the DB lock and checks that backlinks answer within 1 s from retained rows. A source deleted while the server was down is listed only while incomplete, then dropped; notes.get on it fails gracefully.
  • The removed ensure_known(id) only identified an unindexed day file during a scan. Without it, a day note that isn't identified yet can have empty backlinks for a moment, with incomplete: true. That's acceptable, and it no longer blocks on file identification.
  • Gates (agent, dev-01): fmt, clippy -D warnings, notes 469 passed, workspace 1,269 passed.

All three review rounds found no P0/P1 at the end. Ready for train BM6.

## Orchestrator verification of the round-3 P2 fix @ `f812e3e1`: ready for BM6 - Backlinks no longer wait up to 5 s (`LINKS_WAIT`) for the link map. They answer from the retained rows, with `incomplete: true` while the scan or an import runs. The 100k clean-restart backlinks median is 2,188 → 186 ms over five paired runs. - The regression test blocks the reconcile on the DB lock and checks that backlinks answer within 1 s from retained rows. A source deleted while the server was down is listed only while incomplete, then dropped; `notes.get` on it fails gracefully. - The removed `ensure_known(id)` only identified an unindexed *day* file during a scan. Without it, a day note that isn't identified yet can have empty backlinks for a moment, with `incomplete: true`. That's acceptable, and it no longer blocks on file identification. - Gates (agent, dev-01): fmt, clippy `-D warnings`, notes 469 passed, workspace 1,269 passed. All three review rounds found no P0/P1 at the end. **Ready for train BM6.**
kayg commented 2026-11-01 20:21:44 +00:00
Author
Owner
Copy link

integration with dev 55f35fcde3bc91ca3781055577b93e93819f6cd4 is pushed at 77b94678c8f4493b1bb5b0ae31edb5f0ac6d678f; Forgejo reports mergeable. the four requested gates pass. the earlier clean-page ≤1.2× budget is not confirmed: pooled median is +23.2%; both five-pair sets are retained below. backlinks improve in both sets.

conflict resolutions:

  • lib.rs: owned startup initialization now recovers executor writes before deletion recovery and starting scan/watch; either recovery failure shuts the partial Home down. the whole list deadline also covers waiting for executor recovery, so retained file stamps cannot bypass generation recovery.
  • meta.rs: schema 5 and source_versions stay. compatible core/plugin snapshots survive migration and schema 4 sources backfill through five-field Source-bearing upserts. every startup reconcile batch uses Published::commit; the population signal follows that commit. keep the consistent read transaction, short pin lock, executor availability guards, ctime/device/inode stamps, and first 128 / later 4096 batches. pending executor files retain their last accepted rows; recovered images gain a reusable stamp only after matching bytes and unchanged before/after file stamps.
  • backlinks: keep the generation pin and accepted-source snippets, remove readiness/scan waits, and report scan/import incompleteness. readers take no executor admission lock.

tests: added executor-recovery deadline, schema-4 accepted-source backfill, pinned reader during startup publication, and offline title edit after an executor write. inherited action-phase, writer-admission, restored-mtime, deletion, independent reconcile, and retained-backlinks tests pass. the first gate run exposed an accepted-row/validator conflict; that was fixed before the green runs.

passed on dev-01 through Nix and slots for heavy commands:

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -j4 -- -D warnings
  • cargo test -p calternal-notes -j4: 536 passed, 6 ignored
  • cargo test --workspace -j4: 1,419 passed, 22 ignored

100k notes, five alternating paired clean restarts vs current dev, then one unchanged-code replication. each first page returned 50 real rows, total 100000; backlinks returned 8 rows without waiting for scan completion. no owned builds/tests overlapped sampling.

paired runs clean first page, dev → PR (ms) backlinks, dev → PR (ms)
first five 9.200 → 11.251 2296.135 → 253.817
repeat five, unchanged code 9.196 → 12.884 2240.063 → 978.708
all ten pooled 9.198 → 11.334 2264.843 → 468.338

three diagnostic pairs: validation checked exactly 50 rows in 141–229 µs; reader-lock waits 2–4 µs, blocking-worker queues 21–30 µs. the complete notes-page path took 1.260–2.663 ms; engine runtime opening took 2.429–5.570 ms. those measurements do not establish the cause of the clean HTTP delta, so no speculative latency change was added. clean-page performance still needs qualification; this is not a claim that the earlier budget passed.

raw samples, both sets, debug profiles, binary hashes, scripts, source heads and gate logs: control /home/kayg/perf-reports/list-restart-integration-r7/, dev-01 ~/list-restart-integration-r7/. owned binaries, fixture Homes and build targets are being removed after archiving.

integration with dev `55f35fcde3bc91ca3781055577b93e93819f6cd4` is pushed at `77b94678c8f4493b1bb5b0ae31edb5f0ac6d678f`; Forgejo reports mergeable. the four requested gates pass. the earlier clean-page ≤1.2× budget is **not confirmed**: pooled median is +23.2%; both five-pair sets are retained below. backlinks improve in both sets. conflict resolutions: - `lib.rs`: owned startup initialization now recovers executor writes before deletion recovery and starting scan/watch; either recovery failure shuts the partial Home down. the whole list deadline also covers waiting for executor recovery, so retained file stamps cannot bypass generation recovery. - `meta.rs`: schema 5 and `source_versions` stay. compatible core/plugin snapshots survive migration and schema 4 sources backfill through five-field Source-bearing upserts. every startup reconcile batch uses `Published::commit`; the population signal follows that commit. keep the consistent read transaction, short pin lock, executor availability guards, ctime/device/inode stamps, and first 128 / later 4096 batches. pending executor files retain their last accepted rows; recovered images gain a reusable stamp only after matching bytes and unchanged before/after file stamps. - backlinks: keep the generation pin and accepted-source snippets, remove readiness/scan waits, and report scan/import incompleteness. readers take no executor admission lock. tests: added executor-recovery deadline, schema-4 accepted-source backfill, pinned reader during startup publication, and offline title edit after an executor write. inherited action-phase, writer-admission, restored-mtime, deletion, independent reconcile, and retained-backlinks tests pass. the first gate run exposed an accepted-row/validator conflict; that was fixed before the green runs. passed on dev-01 through Nix and slots for heavy commands: - `cargo fmt --all -- --check` - `cargo clippy --workspace --all-targets -j4 -- -D warnings` - `cargo test -p calternal-notes -j4`: 536 passed, 6 ignored - `cargo test --workspace -j4`: 1,419 passed, 22 ignored 100k notes, five alternating paired clean restarts vs current dev, then one unchanged-code replication. each first page returned 50 real rows, total 100000; backlinks returned 8 rows without waiting for scan completion. no owned builds/tests overlapped sampling. | paired runs | clean first page, dev → PR (ms) | backlinks, dev → PR (ms) | |---|---:|---:| | first five | 9.200 → 11.251 | 2296.135 → 253.817 | | repeat five, unchanged code | 9.196 → 12.884 | 2240.063 → 978.708 | | all ten pooled | 9.198 → 11.334 | 2264.843 → 468.338 | three diagnostic pairs: validation checked exactly 50 rows in 141–229 µs; reader-lock waits 2–4 µs, blocking-worker queues 21–30 µs. the complete notes-page path took 1.260–2.663 ms; engine runtime opening took 2.429–5.570 ms. those measurements do not establish the cause of the clean HTTP delta, so no speculative latency change was added. **clean-page performance still needs qualification; this is not a claim that the earlier budget passed.** raw samples, both sets, debug profiles, binary hashes, scripts, source heads and gate logs: control `/home/kayg/perf-reports/list-restart-integration-r7/`, dev-01 `~/list-restart-integration-r7/`. owned binaries, fixture Homes and build targets are being removed after archiving.
kayg commented 2026-11-01 20:23:20 +00:00
Author
Owner
Copy link

Orchestrator decision after the rebase onto dev (BM5 executor) @ 77b94678: ready for the next train

  • The conflict resolutions publish the startup reconcile through the executor's generation path (#814's rule: readers never block writer admission).
  • Gates: notes 536 passed, workspace 1,419 passed, fmt and clippy clean.
  • 10-pair pooled medians against current dev:
    • clean first page: 9.20 → 11.33 ms (1.23×, just over the 1.2× target I set);
    • backlinks: 2,265 → 468 ms.

I'm accepting the extra ~2 ms on the clean first page. It's imperceptible, it buys correct lists after a restart (no stale rows, no empty pages) and backlinks that are 4.8× faster, and the earlier rounds measured noise of a similar size. Follow-up (separate): profile the clean path on top of the executor to win back those 2 ms.

## Orchestrator decision after the rebase onto dev (BM5 executor) @ `77b94678`: ready for the next train - The conflict resolutions publish the startup reconcile through the executor's generation path (#814's rule: readers never block writer admission). - Gates: notes 536 passed, workspace 1,419 passed, fmt and clippy clean. - 10-pair pooled medians against current dev: - clean first page: 9.20 → 11.33 ms (1.23×, just over the 1.2× target I set); - backlinks: 2,265 → 468 ms. I'm accepting the extra ~2 ms on the clean first page. It's imperceptible, it buys correct lists after a restart (no stale rows, no empty pages) and backlinks that are 4.8× faster, and the earlier rounds measured noise of a similar size. **Follow-up (separate):** profile the clean path on top of the executor to win back those 2 ms.
kayg commented 2026-11-02 08:51:17 +00:00
Author
Owner
Copy link

bm7 bisect and source fix: pushed fe46caf501.

first bad browser merge is this PR's 1b4db39e. base 55f35fcd and 0b6accf1 pass 11/11; 1b4db39e and both later sampled merges fail the same four tests. narrowing the composer selector preserves stale daily-note state and reproduces all cataloged empty/glass/link-suggest failures too.

root cause: file stamps gained ctime, but room/save.rs captured Prepared metadata before rename/exchange. placement changes ctime; meta/restart.rs then rejects accepted rows and fails entire notes.list pages. composer fails in snippetOf after task creation, not in the task action. saved and actual stamps were observed differing only in ctime.

the minimal fix uses existing byte-verified saved_meta handling to obtain the final file stamp. saved_file_stamps_validate_first_writes_and_replacements fails on the original source with that exact mismatch, and passes with this fix. it covers first writes, replacements, stamp equality and page contents. this may add a file read on save; no performance claim is made.

local merge of all eight fixed heads: b05d4ccd10, unpushed. requested four-spec chromium-1440 + webkit-1440 matrix: repeat passed 22/22, workers=1, CI=1, retries=0. first run had 21 passes and one distinct composer daily-note creation timeout, retained as an open intermittent risk.

cargo fmt --all -- --check passes. full cargo test --workspace -j4 with strace supplied failed in unchanged kill_nine_mid_append_on_a_long_note: 96 confirmed appends, final >100 assertion; durability assertions passed. it passed in the prior attempt. running the remainder with that test skipped failed later in #833 upcoming_uses_when_else_deadline_and_orders_days_before_months (Kept done vs Tomorrow); 1479 tests passed across executed result groups, but Cargo stopped there, so this is not a green workspace gate. both added regressions and the shipping capability inventory pass. logs retained under ~/bm7-bisect-report on the dev VMs and archived on control. no train branch changed.

bm7 bisect and source fix: pushed fe46caf501870c367e5d5859fb047389abfabf16. first bad browser merge is this PR's 1b4db39e. base 55f35fcd and 0b6accf1 pass 11/11; 1b4db39e and both later sampled merges fail the same four tests. narrowing the composer selector preserves stale daily-note state and reproduces all cataloged empty/glass/link-suggest failures too. root cause: file stamps gained ctime, but room/save.rs captured Prepared metadata before rename/exchange. placement changes ctime; meta/restart.rs then rejects accepted rows and fails entire notes.list pages. composer fails in snippetOf after task creation, not in the task action. saved and actual stamps were observed differing only in ctime. the minimal fix uses existing byte-verified saved_meta handling to obtain the final file stamp. saved_file_stamps_validate_first_writes_and_replacements fails on the original source with that exact mismatch, and passes with this fix. it covers first writes, replacements, stamp equality and page contents. this may add a file read on save; no performance claim is made. local merge of all eight fixed heads: b05d4ccd102ccbefeb2711ceffedff3bd19372ef, unpushed. requested four-spec chromium-1440 + webkit-1440 matrix: repeat passed 22/22, workers=1, CI=1, retries=0. first run had 21 passes and one distinct composer daily-note creation timeout, retained as an open intermittent risk. cargo fmt --all -- --check passes. full cargo test --workspace -j4 with strace supplied failed in unchanged kill_nine_mid_append_on_a_long_note: 96 confirmed appends, final >100 assertion; durability assertions passed. it passed in the prior attempt. running the remainder with that test skipped failed later in #833 upcoming_uses_when_else_deadline_and_orders_days_before_months (Kept done vs Tomorrow); 1479 tests passed across executed result groups, but Cargo stopped there, so this is not a green workspace gate. both added regressions and the shipping capability inventory pass. logs retained under ~/bm7-bisect-report on the dev VMs and archived on control. no train branch changed.
kayg commented 2026-11-02 11:25:30 +00:00
Author
Owner
Copy link

BM8 full-suite evidence found a second cancellation issue: the owned Home initializer failed with note list: database is locked. list_db currently awaits spawn_blocking inside a request-owned OnceCell initializer. Request cancellation drops the initializer permit while the blocking SQLite open continues; another request/Home can start a second schema open.

Plan: retain each list initialization waiter in Notes’ existing owned initialization task set, using the shared OnceCell and a oneshot observer. Cancelling a request drops only its observer. Shutdown closes admission under the same initialization mutex before draining those tasks. O(1) per pending waiter; one SQLite open per successful user cell. No retry or database-error masking.

failure effect detection recovery test
request cancelled during blocking schema open observer dropped, one owned open continues shared OnceCell next waiter gets the same database paused schema plus aborted caller, exactly one schema call
SQLite open fails error delivered, cell remains unset existing error later request may retry existing preserved-cache/open errors
shutdown races admission new open refused; admitted tasks drain initialization closed flag normal close/checkpoint existing shutdown suite plus regression shutdown

Gate: regression red before fix and green after, restart_listing tests, full cargo test --workspace -j4, final two-browser selectors. Product append path unchanged; final append diagnostic will be refreshed.

BM8 full-suite evidence found a second cancellation issue: the owned Home initializer failed with `note list: database is locked`. `list_db` currently awaits spawn_blocking inside a request-owned OnceCell initializer. Request cancellation drops the initializer permit while the blocking SQLite open continues; another request/Home can start a second schema open. Plan: retain each list initialization waiter in Notes’ existing owned initialization task set, using the shared OnceCell and a oneshot observer. Cancelling a request drops only its observer. Shutdown closes admission under the same initialization mutex before draining those tasks. O(1) per pending waiter; one SQLite open per successful user cell. No retry or database-error masking. | failure | effect | detection | recovery | test | |---|---|---|---|---| | request cancelled during blocking schema open | observer dropped, one owned open continues | shared OnceCell | next waiter gets the same database | paused schema plus aborted caller, exactly one schema call | | SQLite open fails | error delivered, cell remains unset | existing error | later request may retry | existing preserved-cache/open errors | | shutdown races admission | new open refused; admitted tasks drain | initialization closed flag | normal close/checkpoint | existing shutdown suite plus regression shutdown | Gate: regression red before fix and green after, restart_listing tests, full `cargo test --workspace -j4`, final two-browser selectors. Product append path unchanged; final append diagnostic will be refreshed.
kayg commented 2026-11-02 13:21:18 +00:00
Author
Owner
Copy link

BM8 validation at scratch b9dac07806a398f53f8a9acaf1506d4b6f718a30: cargo fmt --all -- --check and cargo test --workspace -j4 passed on dev-01 in the standard Nix/slot shell, 1,483 passed and 22 ignored, with no skips or test-thread override. Four report selectors passed 22/22 on Chromium/WebKit, zero retries. Final production code passed 30 fresh Chromium composer invocations (90 tests, zero retries) during one ordinary slot build. Exact commands, head provenance, append samples, retained failures and attached raw evidence: #844 (comment). No merge or train change; owned jobs finished and VM targets/temp Homes cleaned.

BM8 validation at scratch `b9dac07806a398f53f8a9acaf1506d4b6f718a30`: `cargo fmt --all -- --check` and `cargo test --workspace -j4` passed on dev-01 in the standard Nix/slot shell, 1,483 passed and 22 ignored, with no skips or test-thread override. Four report selectors passed 22/22 on Chromium/WebKit, zero retries. Final production code passed 30 fresh Chromium composer invocations (90 tests, zero retries) during one ordinary slot build. Exact commands, head provenance, append samples, retained failures and attached raw evidence: https://git.kayg.org/kayg/calternal/pulls/844#issuecomment-74911. No merge or train change; owned jobs finished and VM targets/temp Homes cleaned.
kayg commented 2026-11-04 13:04:54 +00:00
Author
Owner
Copy link

Closing: head 5d7b9b36 is already in dev (reached via a20cd5f1); nothing left to merge. Branch kept.

Closing: head 5d7b9b36 is already in dev (reached via a20cd5f1); nothing left to merge. Branch kept.
kayg closed this pull request 2026-11-04 13:04:54 +00:00
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backend
Rust server

  
design
Mockups, UI and TASTE

  
docs
ADRs, CONTEXT, TASTE

  
idea
Parked idea, never worked until promoted

  
infra
Deploy, CI, VMs

  
perf
Performance and budgets

  
reliability
Data integrity, corruption traps

  
web
Web client

No labels
backend
design
docs
idea
infra
perf
reliability
web
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal!828
No description provided.