Repository navigation
v0.2.54.0 feat: a run is refused up front when saved studies and test replays fill the limit, and a server replay has storage to land in (Cycle 0059 pt1) - #148
Merged
Conversation
…ist and open unchanged
…le with its writer rules and bounds, and a writer that drains accepted entries before its seal
…server histories together, in both directions
…le separation of the server history store
…on baselines (moved writer identity, shared pair masker, pt3-only members suppressed with a reason)
…plits the 20-second bounds file instead of weighing it as one second
…udy's save counts the test histories too
…and never publishes a seal without end An output offered after the seal began is no longer queued behind end (it is dropped, not a failure), a retried seal keeps its single end, the inherited finish goes through the end-writing seal, an abandon during the final publish removes the published sealed.json, and the rules count an entry only once its line is durable.
…stop whose executed source was not stored seals the capture incomplete The seal keeps every reason uncut, so the stop's unavailable list is bounded by the declaration's inspection limit (the largest legal capture seals inside the 256 KiB seal limit). A stop whose source entry says too large or not captured adds SERVER_HISTORY_SOURCE_UNAVAILABLE right after the snapshot reason.
…browser save, as an unreadable Node history store never blocked one
… that no longer match a finding seal is now used by the server writer's finish, and the three NodeTestHistoryWriter members that moved out of nodeTestHistoryStore.ts match no finding; the two writer entries that still match stay.
…o the partial server history seal is always written
…erver histories that share the saved-study limit
…ft, a request after the terminal entry and output after its truncation, in writer, scan and read
Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
Contributor
Author
|
Local CodeRabbit CLI on the final range (repository delivery rule; hosted CodeRabbit stays off)
|
12 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A learner who explains their own Node server with
/codebookloses everything the run captured when the server stops: Save answers that server runs are not saved yet. Cycle 0059 (Issue 147) makes that explanation a learning replay in three pull requests. This first one lays the storage a server replay is written to and read from, so the next two can read it back (pt2) and record into it during the run (pt3). Nothing writes a server history yet, so a server run is still refused at Save learning replay.What a learner notices today is that the saved study limit now counts the same way everywhere. Before, a test's start counted saved studies, but a new study's save, the check at a run's start and a recording's start did not count test replays. A run could start, finish its explanation and then be refused at its save. Now that run is refused before the application receives any input, with the study library's full-library sentence.
Under the hood:
endonce, and never publishes a seal after it was abandoned.Blast radius
The refactor runs under every saved test replay (
NodeTestHistoryStorenow extends a generic store) and the admission change runs under every browser save, run start and recording start. That is safe because the 7,970-odd existing tests, node-test store, summary, citation and other-window suites included, pass with no edit to their files, and new tests drive both admission directions from two windows (script ran:pnpm --dir extension test, 8016 tests, 0 failures).Test Coverage
Tests: 533 → 533 test files at audit time (the branch's ten new test files were already counted)
Coverage: 100% value-weighted (100% including 0 weakly covered paths)
Test value: 0 tests written, 1 rejected by the authoring gate, 1 existing test extended, 0 paths weakly covered (weak = ★, gate-failing or unrated).
Tests ran: tests count unavailable (receipt); the runner's own summary on the final tree reads 8016 tests, 8006 pass, 0 fail, 10 skipped.
Pre-Landing Review
Pass 1 (whole diff, 6,526 lines): core checklist (a fresh subagent on the same checklist), testing, maintainability, security, performance, simplification and state-machines specialists, the red team, and Codex's structured review (
codex review --base origin/main). Two critical findings, both fixed with a test that failed first:nodeServerHistoryWriter.ts: a long append-failure cause made the partial seal unwritable forever (the seal schema caps a reason's param at 1,024 characters). Fixed: the cause is cut to the seal's bound.recordingStore.ts(caused by the shared-limit change inlearningLibrary.ts): the run-start check and a recording's start did not count histories, so a run was admitted and then refused at its save. Fixed: both count test and server histories.Ten informational findings were answered Skip under the repository rule for feature cycles (non-critical findings are not fixed by the ship review). They are listed below and tracked on Issue 147 to be fixed in pt2 and pt3 of this cycle, each with a test that fails without its fix. Pass 2 (fix range, 170 lines): testing, maintainability, performance, red team NO FINDINGS; state-machines one informational note. Converged.
REVIEW_LOOP: passes=2 blocking_found=2 deferred=0 noted=1 fixed=2 scope=frozen
Gate edits: none detected (13 listed, 0 read; patterns: RH-1,RH-3,RH-4,RH-12,RH-13,RH-14,RH-15,RH-16; unlisted idioms: none)
Outside review: Codex structured review of the whole diff in pass 1 completed with three P2 findings and no P0 or P1 (all three are informational notes below, two reproduced by Codex). On the fix range, the Codex adversarial challenge and the Codex structured review both reported no findings in Japanese prose without severity tags, so the result parser reads them as unverified: that is missing outside coverage for the fix range, not a pass. The native adversarial pass on the fix range completed with three low notes and recommends shipping as is.
Local reviews before
/ship(repository rule): CodeRabbit CLI 0.9.1 on the range before the fixes (--committed --base-commit 250ade84 -c .coderabbit.yaml, included reviews, no credits) completed with 3 findings;open-code-review-delegatereviewed 36 of 36 reviewable files (13 default exclusions: README, fixture files, 9 test files) with 1 high, 5 medium, 5 low findings. All of them were fixed before/shipwith red-first tests. Hosted CodeRabbit is off for this repository (.coderabbit.yamlreviews.auto_review.enabled: false); nobody asked it to review this PR.Review notes
Below the blocking floor (advisory, informational, test-gap or wording items); recorded here and not fixed in this PR.
extension/src/nodeServerHistoryWriter.ts:416finish() takes end's elapsedMs from durable entries only; a queued later entry makes end move time backward and a clean run seals partial (reproduced by Codex) — MULTI-SOURCE CONFIRMED (state-machines + codex)extension/src/utils/evaluateNodeServerCapture.ts:62an interrupted history without end and without a terminal entry reads captureComplete true on re-read (no appendFailure passed), reproduced by Codexextension/src/nodeServerStorageDescriptor.ts:85the incremental scan drops source entries, so the pre-seal summary misses SERVER_HISTORY_SOURCE_UNAVAILABLE that the seal then reports (reproduced by Codex)extension/src/utils/maskSecretFormPairs.ts:18undecodable-name branch untestedextension/src/utils/maskNodeServerConfiguration.ts:22no direct test (body-absent branch and composition)extension/src/nodeServerHistoryWriter.ts:115seal version written as literal 1 instead of NODE_SERVER_HISTORY_FORMAT_VERSIONextension/src/utils/cutHistoryReason.ts:7JSDoc cites internal id F265extension/src/historyStorageDescriptor.ts:143JSDoc says pt1's storeextension/src/nodeHistoryJournalWriter.ts:160untested writer cells: sealed x abandon, abandoned x append/finish/abandon, seal-failed x abandon (behavior correct by reading; downgraded from CRITICAL by lead judgment: no defect named)extension/src/utils/nodeServerJournalRules.ts:138untested cell: a second request-sent or one with no open attempt is refused (downgraded: no defect named)extension/src/nodeTestHistoryStore.ts:319history start counts archives + histories, recording start counts recordings + histories, save/pre-check count archives + histories; a history can take the last place after a recording was admitted and before its archive is saved (pre-existing overtaking, not made worse)extension/src/nodeHistoryRepository.ts:82countHistories counts an unreadable history directory as 0 for every admission caller (deliberate: an unreadable store never blocks browser saves), so the shared limit is not enforced while a history directory is unreadableextension/src/studyWorkspace.ts:10727lazy non-null assertion over nodeHistoryRepository; not reachable todayextension/src/nodeTestHistoryStore.ts:319recordings vs archives counted differently by entry point (pre-existing; same as pass-2 note)Exploratory QA
Functional surface only (pt1 has no producer, UI or MCP surface), probed through the repository's own test files with
node --teston the compiled extension. Pass 1 smoke (before the fixes): the server history store's write, seal and read path passed 7 of 7, and the cross-window admission edge passed 9 of 9. Those captures were superseded by the pass-1 fixes. On the current inputs the same two charters reran and passed: 7 of 7, and 13 of 13 for the admission edge. That run also covers the run-start check and a recording's start against test and server histories. Findings: none. Limits: no native editor check runs in pt1; the cycle's native trial N59 (VS Code and Cursor) runs on pt3.Design Review
No frontend files changed — design review skipped.
Eval Results
No prompt-related files changed — evals skipped.
Scope Drift
Scope Check: CLEAN. Intent: Cycle 0059 pt1 storage foundation with no producer. Delivered: that, plus the shared saved study limit the plan's admission contract requires, its packaged README sentence, and shard timings for the new test files.
Plan Completion
Plan: the plan
/autoplanapproved for Cycle 0059 (Issue 147 body and its pinned comment), audited for the part this PR owns (tasks 1, 2 and the writer and seal parts of task 4, plus engineering tests E1 to E4). Result: 21 items, 17 done, 3 partial, 1 not done, 0 unverifiable.closed, dispose past the close bound and the status naming the cause need the service wiring, pt3)Acceptance edits: baseline unavailable (plan is not tracked in this repository).
Verification Results
Plan checks executed in this ship:
pnpm --dir extension health:staticPASS,pnpm --dir extension testPASS (8016 tests, 0 failures),pnpm --dir extension test:packagePASS (1093 tests, 0 failures), pre-gate preflight PASS (secrets scan: 6124 added lines clean). Still to run before merge (repository delivery chain): required CI, the exact-HEAD gatepnpm --dir extension gate(its fifteen checks includehealth), and the local CodeRabbit CLI on the final range. 3 PASS, 0 FAIL, 0 BLOCKED, 3 NOT RUN yet (they run after this PR opens).Documentation
Status: current - no documentation edits needed, no blockers.
Scope: base 250ade8 to head 1c3c3d9 plus unstaged working-tree changes (CHANGELOG.md, README.md, VERSION, extension/package.json, extension/scripts/tests/package.test.mjs, extension/src/constants.ts, one test). Candidate docs: AGENTS.md, CHANGELOG.md, PRODUCT_GOAL.md, README.md, extension/README.md, extension/skills/codebook/SKILL.md, the v9 PNG (binary, not edited). Discovery found the remaining .md/.txt files (docs/agents/*, test fixtures) unrelated to the diff.
Documentation health
Coverage debt: no new public surface (no commands, settings, MCP tools or user-visible flows); the storage foundation is internal and covered by tests only. The one user-visible behavior, the shared study limit, has reference coverage in extension/README.md and CHANGELOG.md. No diagram drift (no architecture diagrams affected).
Test plan
pnpm --dir extension test(Node 24.22.1,LANG=en_US.UTF-8, macOS): 8016 tests, 8006 pass, 0 fail, 10 skippedpnpm --dir extension test:package: 1093 tests, 1083 pass, 0 fail, 10 skippedpnpm --dir extension typecheck,lint,health:static,dupes,dead-code,format:check: exit 0 (run by the implementers after each fix and re-run by the driving agent on the pre-fix tree)node --test extension/scripts/tests/testShards.test.mjs: 101 pass, 0 fail (new test files carry measured timings, so the shard plan splits the 20 s bounds file)gstack-pregate --stage preflight --allow-repo-commands: pass, 6 checks, secrets scan cleanpnpm --dir extension gateon this headRefs Issue 147 (Cycle 0059; this PR is part 1 of 3 and closes nothing).
🤖 Generated with Claude Code