Repository navigation
Complete live graph integration and slim MCP context - #300
Merged
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 273 files, which is 173 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (273)
You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThe changes add semantic harvesting to client hooks, configurable MCP tool profiles, live-graph verification, Node.js 22 support, updated Codex plugin wiring, and related documentation and tests. ChangesSemantic harvesting hooks
MCP tool profiles
Live graph and release verification
Node.js 22 platform update
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Agent
participant PostToolHook
participant HookAdapter
participant StopHook
participant MCPServer
Agent->>PostToolHook: Complete an editing tool call
PostToolHook->>HookAdapter: Record edited files
HookAdapter-->>Agent: Return recording guidance
Agent->>StopHook: End the session
StopHook->>HookAdapter: Request semantic harvest
HookAdapter->>MCPServer: Prompt active model to call wiki_write
MCPServer-->>Agent: Return harvest result or completion guidance
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Performance Benchmark Results |
ooples
marked this pull request as ready for review
August 9, 2026 04:51
Contributor
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Contributor
Performance Benchmark Results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/hooks/recording.test.mjs (1)
108-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the default branches of
semanticHarvestPrompt.The tests cover the model-supplied path and both null paths. Two branches stay uncovered: the fallback actor text when
modelis empty, and the emptysubjectwhenfilesis empty.🧪 Proposed additional test
test('does not invent work or loop after Codex already continued Stop', () => { expect(semanticHarvestPrompt({ edits: 0 })).toBeNull(); expect(semanticHarvestPrompt({ edits: 5, stopHookActive: true })).toBeNull(); }); + + test('names no model and no files when neither is reported', () => { + const out = semanticHarvestPrompt({ edits: 1 }); + + expect(out).toMatch(/the active model that did the reasoning/i); + expect(out).not.toMatch(/Work touched/); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/hooks/recording.test.mjs` around lines 108 - 127, Extend the `semanticHarvestPrompt` tests with a case using an empty `model` to assert the fallback actor text, and an empty `files` array to assert the prompt’s empty `subject` behavior. Keep the existing model-supplied and null-path coverage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/RELEASE_FLOW.md:
- Line 46: Synchronize the CI documentation with the current workflow matrix and
check names: in .github/RELEASE_FLOW.md lines 46-46, list Node.js 22, 24, 25,
and 26; in .github/BRANCH_PROTECTION.md lines 36-36, replace stale required
checks with exact “Test (Node <version>)” names or document “All Checks Passed”
if that is the intended gate; apply the same required-check update in
.github/CI_CD.md lines 255-255 and .github/RELEASE_FLOW.md lines 236-236,
removing Node.js 20 and the duplicate Node.js 22 entry at both latter sites.
In `@integrations/codex/plugin/hooks/lib/adapter.mjs`:
- Around line 232-257: Gate the persistent edit tracking in the post-tool
mutation handling so state.edits and state.editedFiles are updated only after
the tool result confirms a successful file mutation; preserve the existing
recordingContext behavior for confirmed edits. Apply this change in
integrations/codex/plugin/hooks/lib/adapter.mjs lines 232-257 and
integrations/gemini/hooks/lib/adapter.mjs lines 232-257, keeping their hook
behavior equivalent. Add a failed-mutation regression test that verifies the
subsequent Stop event emits {}.
In `@integrations/qwen/hooks/lib/adapter.mjs`:
- Around line 200-212: Update the shared hook core’s loadState/saveState flow to
persist a harvested marker, then in the Stop handler set and save
state.harvested when a prompt is emitted and pass raw.stop_hook_active === true
|| state.harvested === true as stopHookActive. Apply this in
integrations/qwen/hooks/lib/adapter.mjs lines 200-212 and
integrations/opencode/hooks/lib/adapter.mjs lines 200-212; regenerate
plugin/hooks/lib/adapter.mjs lines 200-212 with scripts/sync-hook-core.mjs
rather than editing it manually.
In `@scripts/verify-client-configs.mjs`:
- Around line 240-255: Update the validation around parsed.hooks and command
extraction to require each required lifecycle event—SessionStart, PreToolUse,
PostToolUse, and Stop—to contain a non-empty group array, with every group
containing a non-empty hooks array. Perform these structural checks before
extracting commands, and ensure invalid or empty groups cannot pass the
hook-command and referenced-script validations.
In `@tests/unit/manifest-versions-track-package.test.ts`:
- Around line 70-74: Add the Claude plugin manifest version entry to the targets
returned by targets(), using plugin/.claude-plugin/plugin.json and $.version
consistently with the existing manifest targets. Update the minimum target-count
assertion to require six targets.
---
Nitpick comments:
In `@tests/hooks/recording.test.mjs`:
- Around line 108-127: Extend the `semanticHarvestPrompt` tests with a case
using an empty `model` to assert the fallback actor text, and an empty `files`
array to assert the prompt’s empty `subject` behavior. Keep the existing
model-supplied and null-path coverage unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6e21c8e-d732-41df-ae15-f1f99ce77269
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (56)
.github/BRANCH_PROTECTION.md.github/CI_CD.md.github/QUICKSTART.md.github/RELEASE_FLOW.md.github/workflows/ci.yml.github/workflows/codex-autofix.yml.github/workflows/commitlint.yml.github/workflows/quality-gates.ymlREADME.mddocs/CLIENT_SUPPORT.mddocs/NPM_TRUSTED_PUBLISHER_SETUP.mddocs/QUICK_START_GUIDE.mdhooks-core/adapter.mjshooks-core/doctor.mjshooks-core/recording.mjsintegrations/README.mdintegrations/codex/hooks/hooks.jsonintegrations/codex/hooks/lib/adapter.mjsintegrations/codex/hooks/lib/doctor.mjsintegrations/codex/hooks/lib/recording.mjsintegrations/codex/hooks/post-tool.mjsintegrations/codex/hooks/stop.mjsintegrations/codex/plugin/.codex-plugin/plugin.jsonintegrations/codex/plugin/.mcp.jsonintegrations/codex/plugin/hooks/hooks.jsonintegrations/codex/plugin/hooks/lib/adapter.mjsintegrations/codex/plugin/hooks/lib/doctor.mjsintegrations/codex/plugin/hooks/lib/recording.mjsintegrations/codex/plugin/hooks/post-tool.mjsintegrations/codex/plugin/hooks/stop.mjsintegrations/gemini/hooks/lib/adapter.mjsintegrations/gemini/hooks/lib/doctor.mjsintegrations/gemini/hooks/lib/recording.mjsintegrations/opencode/hooks/lib/adapter.mjsintegrations/opencode/hooks/lib/doctor.mjsintegrations/opencode/hooks/lib/recording.mjsintegrations/qwen/hooks/lib/adapter.mjsintegrations/qwen/hooks/lib/doctor.mjsintegrations/qwen/hooks/lib/recording.mjspackage.jsonplugin/hooks/lib/adapter.mjsplugin/hooks/lib/doctor.mjsplugin/hooks/lib/recording.mjsrelease-please-config.jsonscripts/generate-client-entries.mjsscripts/verify-client-configs.mjsscripts/verify-live-graph-flow.mjssrc/server/index.tssrc/server/tool-profile.tstests/hooks/codex-semantic-harvest.test.mjstests/hooks/recording.test.mjstests/integration/client-hooks.test.tstests/integration/mcp-tool-profiles.test.tstests/unit/doctor-reports-harvest-state.test.tstests/unit/manifest-versions-track-package.test.tstests/unit/required-fields-enforced.test.ts
Contributor
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Contributor
Performance Benchmark Results |
Contributor
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Contributor
Performance Benchmark Results |
ooples
force-pushed
the
fix/live-graph-full-system
branch
from
August 9, 2026 11:00
aa61de1 to
5cb4844
Compare
Contributor
Performance Benchmark Results |
8 tasks done
This was referenced Aug 9, 2026
ooples
added a commit
that referenced
this pull request
Aug 26, 2026
Closes #322. #300 unregistered `plugin/hooks/stop-harvest.mjs` and nothing replaced it, so four capabilities went silently: the transcript archive, the harvest worker spawn, refusal detection, and the notice that says why no findings exist. On a full session with a valid credential, `harvestMode()` reports `remote`, the user has consented by not opting out, and nothing happens. THE ISSUE UNDERSTATES IT, and the correction matters for the fix. It says the other ten adapter clients are unaffected because they reach the harvest through the shared adapter path. They do not: `grep` finds exactly one spawner of `harvest-worker.mjs` in the repository, and it is the unregistered file. Every client routes Stop to its own generated entry, which routes to the adapter, and the adapter's stop branch only ever built the IN-BAND prompt asking the model to call `wiki_write` -- the very thing the design says the out-of-band harvest exists to avoid, since it makes the session pay for summarising itself. So the out-of-band harvest was dead everywhere, on all eleven clients. Qwen's Stop entry is even named `token-optimizer-semantic-harvest`. The archive is dead on master too, not restored: `archive()` has one caller and it is the same unregistered file. MOVED TO THE SHARED CORE rather than re-registered on one client. `stop-harvest.mjs` becomes `hooks-core/stop-harvest.mjs`, exporting `runStopHarvest(payload)`, and the adapter's stop branch awaits it. Fixing only Claude Code would have left ten clients with the same dead feature. AND THE WORKER MOVED WITH IT, which is the part that made a shared fix possible at all: `harvest-worker.mjs` lived in `plugin/hooks/` and is vendored to no other client, so even a correctly registered Stop on the other ten would have found no worker to spawn. It now sits in the core every client receives an identical copy of, and the adapter resolves it as a sibling. THE NOTICE IS RETURNED, NOT WRITTEN. The old hook wrote its own JSON to stdout beside whatever the hook emitted; two writers on one hook's stdout is how a Stop payload gets corrupted. The adapter folds it into the single object it emits, so the one line explaining why no findings exist can no longer be dropped by a full pipe or truncated by an exit. WHY IT SURVIVED, and the guard that now stops it. Nothing observed that the harvest had not run: there is no `kind:'harvest'` event to be absent from a report, no error, and a harvest that ran and found nothing looks exactly like one that never started. The reachability guard could not see it either -- the unregistered file's own imports gave `archive`, `detectRefusals` and `recordRefusal` their call sites, and a name-based scan proves a REFERENCE exists, never that it RUNS, which is the limitation #323 documented. The existing marker suite even went on passing, because it spawned the removed hook directly; it now drives the registered entry. Nine assertions in tests/hooks/stop-reaches-the-harvest.test.mjs, all through the file the host is actually configured to run, and both defects are mutation-tested: unwiring the adapter call fails four of them, and deleting Stop from hooks.json -- the #300 defect itself -- fails one. 221 suites, 2706 tests, 0 failed. tsc, eslint, prettier, sync:hooks:check and validate clean; verify:clients, :certification, :harvest, :live-graph, :ui and :interactions all pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HSFxBpREeR7mMkJXYb6wuX
ooples
pushed a commit
that referenced
this pull request
Aug 26, 2026
…lt honestly `derive` was already wired into the shared Stop branch by Task 4, and `plugin/hooks/stop.mjs` is a generated entry into it -- so Claude Code was reachable. Running the real hook binary showed reachable was not enough: a real session derived 0 candidates and stored 0 findings, for three measured reasons. RESTORE THE TRANSCRIPT ARCHIVE. #300 replaced `stop-harvest.mjs` with the generated `stop.mjs` in `hooks.json`, and everything the old entry did was lost silently -- the archive among it. `readArchive` returning [] is indistinguishable from a session with nothing in it, so no Claude Code session has been archived since. `derive`'s correction detector reads the archive first, and `lessons.mjs` can verify a verbatim quote against nothing else. Restored before `derive`, in its own fail-open try. Verified: 723 turns archived from a real transcript, and a real transcript containing corrections now stores 6 findings through the Claude Code Stop hook on a cold graph. STANDING RULE. One line in `standingRules`' heading, charged inside the same 400-token budget: a model reading a list of standing rules has no way to add to it unless the block says how one is made. Fixed overhead with one rule: 60 estimated tokens. SURFACE THE LOCAL ENDPOINT. `doctor` now says "local model found -- semantic harvest is on, free and private" and, on `off:no-key`, names the trade rather than the absence: `derive` needs no credential, a local endpoint runs the harvest free and private, a credential runs it from a bounded digest and never file contents. A mode-derived sentence joins the SessionStart policy, addressed to the model -- "a conclusion you do not record is not written down in words anywhere" changes what it does with one. Cache-safe in all four modes. STATE THE DEFAULT HONESTLY. README and WIKI_GRAPH: a default install produces the structural graph plus locally derived findings. The model-based harvest is NOT opt-in -- `TOKEN_OPTIMIZER_HARVEST=0` turns it off -- its real gate is a credential, and a local endpoint runs it free and private. Two of the new tests passed for the wrong reason and were rewritten: a budget test a five-line paragraph survived, and a "no digits" test that `volatileLines` never checked for. 17 mutations, each killing only its intended tests; the pre-existing kill sets still discriminate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ooples
pushed a commit
that referenced
this pull request
Aug 26, 2026
`derive` was already wired into the shared Stop branch by Task 4, and `plugin/hooks/stop.mjs` is a generated entry into it -- so Claude Code was reachable. Running the real hook binary showed reachable was not enough: a real session derived 0 candidates and stored 0 findings, for three measured reasons. RESTORE THE TRANSCRIPT ARCHIVE. #300 replaced `stop-harvest.mjs` with the generated `stop.mjs` in `hooks.json`, and everything the old entry did was lost silently -- the archive among it. `readArchive` returning [] is indistinguishable from a session with nothing in it, so no Claude Code session has been archived since. `derive`'s correction detector reads the archive first, and `lessons.mjs` can verify a verbatim quote against nothing else. Restored before `derive`, in its own fail-open try. Verified: 723 turns archived from a real transcript, and a real transcript containing corrections now stores 6 findings through the Claude Code Stop hook on a cold graph. STANDING RULE. One line in `standingRules`' heading, charged inside the same 400-token budget: a model reading a list of standing rules has no way to add to it unless the block says how one is made. Fixed overhead with one rule: 60 estimated tokens. SURFACE THE LOCAL ENDPOINT. `doctor` now says "local model found -- semantic harvest is on, free and private" and, on `off:no-key`, names the trade rather than the absence: `derive` needs no credential, a local endpoint runs the harvest free and private, a credential runs it from a bounded digest and never file contents. A mode-derived sentence joins the SessionStart policy, addressed to the model -- "a conclusion you do not record is not written down in words anywhere" changes what it does with one. Cache-safe in all four modes. STATE THE DEFAULT HONESTLY. README and WIKI_GRAPH: a default install produces the structural graph plus locally derived findings. The model-based harvest is NOT opt-in -- `TOKEN_OPTIMIZER_HARVEST=0` turns it off -- its real gate is a credential, and a local endpoint runs it free and private. Two of the new tests passed for the wrong reason and were rewritten: a budget test a five-line paragraph survived, and a "no digits" test that `volatileLines` never checked for. 17 mutations, each killing only its intended tests; the pre-existing kill sets still discriminate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ooples
added a commit
that referenced
this pull request
Aug 26, 2026
* fix(harvest): run the semantic harvest on the shared stop path Closes #322. #300 unregistered `plugin/hooks/stop-harvest.mjs` and nothing replaced it, so four capabilities went silently: the transcript archive, the harvest worker spawn, refusal detection, and the notice that says why no findings exist. On a full session with a valid credential, `harvestMode()` reports `remote`, the user has consented by not opting out, and nothing happens. THE ISSUE UNDERSTATES IT, and the correction matters for the fix. It says the other ten adapter clients are unaffected because they reach the harvest through the shared adapter path. They do not: `grep` finds exactly one spawner of `harvest-worker.mjs` in the repository, and it is the unregistered file. Every client routes Stop to its own generated entry, which routes to the adapter, and the adapter's stop branch only ever built the IN-BAND prompt asking the model to call `wiki_write` -- the very thing the design says the out-of-band harvest exists to avoid, since it makes the session pay for summarising itself. So the out-of-band harvest was dead everywhere, on all eleven clients. Qwen's Stop entry is even named `token-optimizer-semantic-harvest`. The archive is dead on master too, not restored: `archive()` has one caller and it is the same unregistered file. MOVED TO THE SHARED CORE rather than re-registered on one client. `stop-harvest.mjs` becomes `hooks-core/stop-harvest.mjs`, exporting `runStopHarvest(payload)`, and the adapter's stop branch awaits it. Fixing only Claude Code would have left ten clients with the same dead feature. AND THE WORKER MOVED WITH IT, which is the part that made a shared fix possible at all: `harvest-worker.mjs` lived in `plugin/hooks/` and is vendored to no other client, so even a correctly registered Stop on the other ten would have found no worker to spawn. It now sits in the core every client receives an identical copy of, and the adapter resolves it as a sibling. THE NOTICE IS RETURNED, NOT WRITTEN. The old hook wrote its own JSON to stdout beside whatever the hook emitted; two writers on one hook's stdout is how a Stop payload gets corrupted. The adapter folds it into the single object it emits, so the one line explaining why no findings exist can no longer be dropped by a full pipe or truncated by an exit. WHY IT SURVIVED, and the guard that now stops it. Nothing observed that the harvest had not run: there is no `kind:'harvest'` event to be absent from a report, no error, and a harvest that ran and found nothing looks exactly like one that never started. The reachability guard could not see it either -- the unregistered file's own imports gave `archive`, `detectRefusals` and `recordRefusal` their call sites, and a name-based scan proves a REFERENCE exists, never that it RUNS, which is the limitation #323 documented. The existing marker suite even went on passing, because it spawned the removed hook directly; it now drives the registered entry. Nine assertions in tests/hooks/stop-reaches-the-harvest.test.mjs, all through the file the host is actually configured to run, and both defects are mutation-tested: unwiring the adapter call fails four of them, and deleting Stop from hooks.json -- the #300 defect itself -- fails one. 221 suites, 2706 tests, 0 failed. tsc, eslint, prettier, sync:hooks:check and validate clean; verify:clients, :certification, :harvest, :live-graph, :ui and :interactions all pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HSFxBpREeR7mMkJXYb6wuX * fix(harvest): keep the vendored worker parseable, and normalise the stop payload Four review findings, all valid. One was critical and would have shipped this pull request as a no-op. THE HASHBANG. `sync-hook-core` prepends its GENERATED banner, which pushed harvest-worker's `#!/usr/bin/env node` to line 3 -- and Node accepts a hashbang only at byte zero. Every vendored copy was a SYNTAX ERROR. The worker is spawned detached with stdio ignored, so it would have failed instantly and silently: the harvest still not running, for exactly the reason it was not running before, with a green suite on either side of it. Verified with `node --check` on the generated file before the fix. Fixed in the generator rather than by deleting the hashbang, because the next executable added to the core would hit it again and the failure mode is invisible by construction. `withBanner` keeps a leading hashbang on line one and puts the banner after it. AND THE ASSERTION WHOSE ABSENCE ALLOWED IT. This branch's own test checked that the vendored worker EXISTS -- and it said so while every copy was unparseable. Existence is not executability, which is this pull request's whole point applied to its own output. Every vendored core file is now `node --check`ed in three client directories, and the hashbang property is pinned separately. Mutation-tested: restoring banner-before-hashbang fails both. THE STOP PAYLOAD. The adapter resolves `sessionId` and `cwd` through every alias a host might use -- `sessionId`, `conversation_id`, `working_directory` -- and then handed `raw` to `runStopHarvest`, which reads `session_id` and `cwd` only. On a client sending an alias the session id came back undefined and the archive root fell back to `process.cwd()`: the once-per-session notice and the debounce marker shared across every session, and transcripts filed under whatever directory the hook happened to start in. A normalised payload is passed now, and `cwd` is hoisted out of the try block below so both halves of the hook reason about one resolved directory. THE SPAWN. `spawn` reports a failed start through an asynchronous `error` event, and an unlistened one is raised rather than returned -- which would abort the Stop response itself. A harvest that cannot start must degrade to no harvest, never to a broken Stop. `child.on('error', () => {})` after unref. 221 suites, 2708 tests, 0 failed. tsc, eslint, prettier, sync:hooks:check and validate clean; all six verify gates pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HSFxBpREeR7mMkJXYb6wuX * test(harvest): check every vendored copy, and pin the banner to line two Two review findings, both about the guard this branch added rather than the code it guards. PARTIAL COVERAGE READ AS COVERAGE. The parse check looked at three of the eleven vendored directories and the hashbang check at two, so a broken generated file in any other client stayed invisible -- which is the same silence the guard was written to break. The directories are now DISCOVERED rather than listed: any directory named `lib` holding an `adapter.mjs` is a vendored core by construction, so a twelfth client is covered the day it is added rather than the day somebody remembers. A floor of eleven guards a walk that finds nothing, which would pass every assertion below forever. Widening it needed it to get faster, not slower. Spawning `node --check` per file cost 7.5s for three directories and would have cost half a minute for eleven; `vm.SourceTextModule` performs the same parse in process, without evaluating, and covers all eleven in 300ms. THE BANNER ASSERTION WAS `toContain`, which passes wherever the banner happens to land in the file -- including below the hashbang, which is the exact layout the test exists to reject. It now asserts the second line. Trimmed per line, because the vendored copies carry CRLF on Windows and the assertion is about which line the banner is on rather than about endings. Mutation-tested against a client the previous version never looked at: moving the hashbang to line three in integrations/kilo fails both checks, and displacing the banner in integrations/windsurf fails the layout one. 223 suites, 2737 tests, 0 failed. tsc, eslint, prettier and sync:hooks:check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HSFxBpREeR7mMkJXYb6wuX --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ooples
added a commit
that referenced
this pull request
Aug 26, 2026
… 2 of 3) (#328) * feat(derive): mandatory redaction for claims derived from captured output * fix(derive): isolate secret redaction tests and assert the null guard Fix round 1 on the redact.mjs task per coordinator review: - Split the bearer-token and sk-/pk-/ghp- prefix tests so each is triggered by a fixture that only the pattern under test can match (the original shared token satisfied both patterns at once, so disabling either alone left the suite green). - Added explicit catch/preserve tests for the PEM private-key pattern, previously verified only by hand. - Asserted redact(null) and redact(undefined) return '', not just that they don't throw -- without the `?? ''` guard they'd silently become the literal strings "null"/"undefined", a garbage claim worse than a thrown error because nothing would surface it. - Documented the redact-then-truncate ordering invariant and the null-guard rationale directly in hooks-core/redact.mjs so a later regex/formatting tweak can't quietly "simplify" either one away. Re-ran the full mutation matrix: each of the 7 mutations (5 patterns, the truncation off-by-one, the null guard) now kills exactly one test. * feat(capture): carry output and exit code on tool-outcome Only a success boolean was recorded, which cannot distinguish a compile error from a test failure from a denied permission. Extends the existing tool-outcome event rather than adding a parallel kind: that pipeline already runs (234 live events in this repo) and already joins each outcome to its injection, so a second event describing the same thing would have duplicated the join and given the overloaded kind field a fifth meaning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(capture): read mcp error status and capture only failed output Two corrections to the tool-outcome capture, both raised as concerns against the brief and both confirmed. toolSucceeded read no isError while src/server/index.ts sets it in seven places, so every failed smart_edit was recorded as a success. That boolean feeds episode outcomes and effectiveness accounting, so the bias ran in the optimizer's own favour -- a measurement that flatters the thing being measured. A differential over 16 clients x 5 envelopes x 13 response bodies (1088 verdicts) changes 96 of them, and every one is an isError:true payload going true -> false. Nothing else moved on any client. Output is now captured only when the call failed. The brief predates MCP tool names reaching the post-tool path, so it never considered that a successful smart_read would deposit 4 KB of file content into the evidence log on every call. A failure claim quotes the error text; a "this works" claim needs only success and exit, which are already recorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(capture): read mcp error status in mutation accounting mutationSucceeded read isError zero times while src/server/index.ts sets it in seven places, so a failed MCP smart_write was still counted as a successful mutation. Its allowlist returns true for claude-code, codex, qwen, opencode, kilo and windsurf on any post-tool event with no error and no status, which is exactly the shape of a failed smart_write. That inflates edit counts and harvest pressure -- different consumers from the effectiveness figures, same direction of bias, toward flattering the optimizer. Extracted rather than copied: reportedMcpError is now the single definition of what a failed MCP call looks like, shared with toolSucceeded, and responseEnvelopes sits above both readers. The duplication was not theoretical -- toolSucceeded listed four envelopes, mutationSucceeded listed three, and neither mentioned isError. The gate precedes the status read, the explicit-success read and the client allowlist, all three of which would otherwise pass a failed write; the function's own header already required that conservatism. A differential over 16 clients x 5 envelopes x 18 response bodies plus 6 top-level shapes (1536 verdicts) changes 123, every one an isError:true payload going true -> false, with zero changes not involving isError. The change is monotone: it can only remove a mutation count, never add one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plans): make plans 2 and 3 self-sufficient for another machine The plans were committed; the ledgers holding the corrections were not, and they are gitignored. So a fresh session elsewhere would have followed a plan text that is wrong in five specific places while a ledger it could not see knew better -- which is the same failure this whole effort exists to close, one level up. Both plans now carry an "Execution state and corrections" section that is authoritative over the task text above it: where execution stopped, what the plan says that is no longer true, which Plan 1 surfaces each task will collide with, the rulings that bind future tasks with their cost if wrong, and the verification bar (mutate every test, confirm only it fails, confirm no prior kill set shrank). Plan 3 additionally gains the three guard holes discovered after it was written -- an import specifier counting as a use, unread record fields being invisible, and holdout-consulting tests needing to pin the arm -- plus the allowlist ownership table showing which plan removes which of the remaining seven entries. Both also carry the measurement-bias class: four defects found so far that would each have inflated this project's own numbers in its own favour, none of which a test suite would notice, because the code works and the number lies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(metrics): share re-read grouping between reread waste and derive The churn detector needs per-anchor rows; rereadWaste returned only an aggregate. One shared helper rather than two definitions of what counts as a wasteful re-read. Existing aggregate fields are unchanged -- they are now a fold over the same rows -- and `worst` is additive. Rows are merged per anchor while GROUPING stays per session, so a file re-read in two sessions is one row with two repeats rather than two rows, and a cross-session pair is still not a repeat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(derive): three zero-cost extractors, so a default install produces findings This repository's graph held 2,965 symbol nodes, 904 file nodes, 128 task nodes and ONE finding, because every path to a verdict needed an API key this machine does not have. derive.mjs needs none: it reads outcomes, transitions and corrections already on disk and sends nothing anywhere, which makes it the only finding producer on CI, a corporate laptop, or a subscription-only login. Three detectors produce findings, not four. Churn describes our own reading behaviour, not the code; filed as a content-dependent `map` it would take the hottest files in the project as anchors, flip stale on edits to files it never described, and compete for the injection budget with real findings while saying nothing a future agent could act on. It returns as an observation instead -- the measurement it already is on the balance sheet. Precision is refusal, not a confidence number. Identical command text either side of a failure emits nothing: "X succeeded where X failed" is incoherent and "X fails" has just been disproved by the same pair, and no ceiling rescues a claim whose content is wrong. Claims say "succeeded where", never "fixes" -- two ordered outcomes do not establish cause. Wired at Stop in adapter.mjs, recording the candidate count. Storage is Task 4; `written` is empty by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(derive): budget selection before storage and delete content anchor selectForConsolidation had no caller, so nothing bounded what entered the graph -- risk 2 (bloat) unaddressed. derive now routes every candidate through it (1000 tokens, two per-command injection budgets' worth) and into writeHarvested as ORIGIN_HARVESTED, with the hook payload's own session id so the `answers` edge finally fires on a default install. Three defects found on the way, each of which would have made this ship as a no-op that reported success: - selectForConsolidation scored `entry.summary` while every layer that STORES a finding calls the same text `claim` -- including consolidationRatio in the same file. So the carrying cost was 0 for every candidate, `spent + 0 > budget` was never true, and the budget admitted everything while reporting `tokens: 0`. - Candidates anchored to the project ROOT, a directory. writeHarvested resolves anchors through indexFile, which cannot read a directory, so every candidate would have been refused and nothing stored. Now anchored to the file that declares the project's commands, which staleness.mjs already names as the right anchor for a command claim and which these types are never content-checked against. - derive's own evidence boilerplate said "a flaky run explains the same pair", and irrecoverability word-matches /flak/ into its top tier -- so every command candidate scored as an irrecoverable observation on the strength of its own epistemic caveat. Same shape as the race/"stack trace" defect that function was already fixed for once. derive stays synchronous: nothing under consolidate.mjs or harvest-write.mjs imports it, so there is no cycle for a dynamic import to break, and making it async would have changed every hook caller for nothing. contentAnchor is deleted rather than left dormant; the idea remains in issue #319. The reachability allowlist goes 7 -> 5: one entry left by being wired, one by being deleted. Also fixes a pre-existing fixture that proved nothing: `decisions are on the floor too` passed with ALWAYS_KEEP emptied, because its finding never fit the budget and the decision was kept by plain ranking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(derive): archive at stop and surface local harvest honestly `derive` was already wired into the shared Stop branch by Task 4, and `plugin/hooks/stop.mjs` is a generated entry into it -- so Claude Code was reachable. Running the real hook binary showed reachable was not enough: a real session derived 0 candidates and stored 0 findings, for three measured reasons. RESTORE THE TRANSCRIPT ARCHIVE. #300 replaced `stop-harvest.mjs` with the generated `stop.mjs` in `hooks.json`, and everything the old entry did was lost silently -- the archive among it. `readArchive` returning [] is indistinguishable from a session with nothing in it, so no Claude Code session has been archived since. `derive`'s correction detector reads the archive first, and `lessons.mjs` can verify a verbatim quote against nothing else. Restored before `derive`, in its own fail-open try. Verified: 723 turns archived from a real transcript, and a real transcript containing corrections now stores 6 findings through the Claude Code Stop hook on a cold graph. STANDING RULE. One line in `standingRules`' heading, charged inside the same 400-token budget: a model reading a list of standing rules has no way to add to it unless the block says how one is made. Fixed overhead with one rule: 60 estimated tokens. SURFACE THE LOCAL ENDPOINT. `doctor` now says "local model found -- semantic harvest is on, free and private" and, on `off:no-key`, names the trade rather than the absence: `derive` needs no credential, a local endpoint runs the harvest free and private, a credential runs it from a bounded digest and never file contents. A mode-derived sentence joins the SessionStart policy, addressed to the model -- "a conclusion you do not record is not written down in words anywhere" changes what it does with one. Cache-safe in all four modes. STATE THE DEFAULT HONESTLY. README and WIKI_GRAPH: a default install produces the structural graph plus locally derived findings. The model-based harvest is NOT opt-in -- `TOKEN_OPTIMIZER_HARVEST=0` turns it off -- its real gate is a credential, and a local endpoint runs it free and private. Two of the new tests passed for the wrong reason and were rewritten: a budget test a five-line paragraph survived, and a "no digits" test that `volatileLines` never checked for. 17 mutations, each killing only its intended tests; the pre-existing kill sets still discriminate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plan): add task 11 transcript reader for failed tool results Task 5 measured why the two highest-confidence detectors produced nothing: Claude Code never fires PostToolUse for a failed tool call. Proved with a deliberately failing command that emitted no event at all, and 2,238 of 2,238 live outcomes on the measuring machine were success: true. So command failed-then-succeeded (0.90) and test/build red-to-green (0.85) have no input on the primary client while working normally on the ten adapter clients. This task feeds them from the one place those failures do exist -- the local transcript archive the correction detector already reads. The task's deliverable is a measurement rather than a claim, and it carries an explicit stop condition: if failed results are not recoverable from the archive, report that and halt, because saying so is worth more than an extractor that finds nothing. * feat(derive): read failed tool results from the transcript, and refuse the junk they turn out to be Claude Code never fires PostToolUse for a failed tool call -- 2,238 of 2,238 live tool-outcome events carry success:true -- so derive's two strongest detectors, command failed-then-succeeded (0.90) and test/build red-to-green (0.85), had NO INPUT AT ALL on the primary client while working normally on the ten adapter clients. This gives them the only source those failures exist in. `failedResultsFromTranscript` (transcript.mjs) reads the raw transcript and stores nothing: no capture path, no hook, no event kind, and no widening of what leaves the machine. The plan's `failedResultsFromArchive(turns)` cannot work -- `readTurns` drops tool results by design, so `readArchive` returns no failure ever, and making the archive carry them would push command stderr into `buildFeedbackDigest`, the one path that transmits anything. The shape, measured rather than assumed: a `tool_result` block in a user message with `is_error: true` and a bare string `content`, joined to the command by `tool_use_id` -> the assistant's `tool_use.input.command`. Only `Exit code N` is admitted, on a positive allowlist: of 214 real `is_error` results just 83 were a command exiting non-zero, and the largest other group (109) is THIS TOOL'S OWN PreToolUse denial text. Ingesting those would derive 0.90-confidence findings from its own advice and serve them back as observations about the project. Keys match across sources because the reader applies the same 120-character truncation `recordToolOutcome` applies to `anchor`: on one real session that made 266 of 266 attempt keys agree, where the untruncated text agreed on 240. Deduplication is exact on `toolCallId`, which is the same string in both sources, with identical-text-within-2s as the fallback for a client that reports no call id. Two refusals were then forced by measurement, and are why this ships at zero rather than at four. The first wiring produced four candidates at the module's highest ceiling, every one quoting a truncated fragment of a throwaway shell script. So: - `quotable` refuses a command containing a newline or at/over the 120-char cap. 0 of 83 captured command failures across three transcripts are quotable; 8 of 571 across all 163 transcripts on this machine (1.4%). A fragment of a program is not a weaker claim, it is not a claim. - `hasAttemptIdentity` refuses a pair whose attempt key holds a bare shell separator. The single key `cd <repo> &&` covers 539 distinct command lines in this repository, so any pair drawn from it is false about both halves. This class was unreachable while no failures arrived; the reader makes it reachable, so it is closed here. Both apply to BOTH sources: where evidence arrived from has no bearing on whether the resulting sentence says anything. Ceilings unchanged, and both Task 3 refusals still hold. Bounded: last 8 MB of transcript, 50 failures, 4 KB of output each, redacted at this boundary because transcript text has never passed one. Fail-open throughout -- this runs at session end. Measured end to end: 0 candidates before, 0 after, on this machine's real evidence and through the real Stop hook. That is the finding. The fix that would recover most of the 571 is to skip a leading `cd <path> &&` when building attemptKey, which changes the key for every client and wants its own change. 18 mutations, each killing at least one test and the two pre-existing kill sets intact. Four new tests passed for the wrong reason and were fixed -- all four the one-fixture-two-mechanisms shape, including a dedup test whose two copies shared a timestamp so the fallback masked the call-id check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plan): add task 12 for unrelated command grouping Task 11 measured 539 distinct commands sharing one key in this repository, because a leading 'cd <repo> &&' consumes the whole three-token key. Framed as a correctness bug rather than an unlock: on the ten clients where the detectors do receive failures, the command detector can pair a failure with a success from an unrelated command and emit a 0.90-confidence claim that is false about both halves. 0.90 is the highest confidence any detector may claim. The task requires measured before/after evidence on real transcripts, and states explicitly that a larger candidate count is not success on its own -- the question is whether the pairs are true. * fix(derive): key an attempt on the command, not on the cd that preceded it `attemptKey` grouped by up to three non-flag tokens, so a leading `cd <path> &&` consumed the whole key. Measured on this repository's 2,243 command outcomes plus 573 exit-code failures from 164 transcripts: the single key `cd c:/users/.../token-optimizer-mcp &&` covered 547 distinct command lines, and one real quotable failure sat in a key holding 222 quotable successes -- one guard away from a 0.90 claim, the highest any detector may make, that would have been false about both halves. `commandBody` skips a leading `cd` up to `&&`, `;` or a newline: the three separators actually observed after a leading cd (1147 / 204 / 38 of 1400). Applied inside `attemptKey`, which is the one function both the event path and the transcript reader pass through, so both sources get it identically; the 120-char anchor cap that makes them agree is untouched and now MORE load-bearing (4342/4342 keys agree with it, on both arms). Nothing wider, each refusal measured. Environment assignments carry information about what ran -- the one true instance here is `TOKEN_OPTIMIZER_HOLDOUT=1 node ... jest.js`, and the holdout arm is a different attempt. `time`, `env`, `bash -c`, `sudo` and `pushd` occur zero times in 2,243 commands. `||` is an error branch, not the next step. The identical-text refusal is WIDENED rather than left behind: it now compares `commandBody` on both sides, because `cd repo && npm test` and `npm test` are one attempt spelled two ways and the better key would otherwise have emitted "`npm test` succeeded where `cd repo && npm test` failed" at 0.85. The claim text still quotes the raw command, since that is what ran. Most-populated key 547 -> 76 distinct commands; keys 904 -> 1726; keys holding >100 distinct commands 3 -> 0; false-pair exposure 222 -> 0. Candidates 0 -> 0, reported rather than dressed up: the binding constraint is `quotable` (8 of 573 command failures are actionable), not the key. With the quality filters disabled the key admits 40 pairs against 49 before, and all 40 are false -- unrelated heredoc scripts sharing an opener, every one refused downstream. Two existing tests were made to pass for the WRONG REASON by this change and are rewired: the hasAttemptIdentity test (its `cd repo &&` fixture now yields different keys, so it would have passed on a key mismatch) and the multi-line test (`cd sub\nnpm run ship` vs `...build` now split). Both found by mutating rather than reading. 21 mutations, baseline and restored 103/103, verdicts read from `jest --json`. Every pre-existing kill set held, including both Task 11 refusals. The circularity refusal rechecked on the full corpus: 1,984 is_error results, 1,149 of them denials, 0 ingested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plan): record tasks 11 and 12 in the execution state The state table stopped at Task 10, so both mid-plan tasks were invisible to the next reader. Task 12's row carries the numbers a future change to `attemptKey` needs: 547 -> 76 on the most-populated key, 222 -> 0 on false-pair exposure, and the prefixes deliberately NOT stripped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(metrics): add layer 1 explicit-reference classification Deliberately independent of read-suppression: that is Layer 2's estimand, and using it in both would make Task 8's calibration compare two spellings of one quantity. unknown is excluded from the denominator, never counted as a miss. Built on the injection-to-outcome join recordToolOutcome already performs -- which prefers an exact tool-call-id match and already reports when it cannot attribute -- rather than a weaker (sessionId, findingKey) join of its own. No change to inject.mjs was needed: it has recorded findingIds all along. TWO DETERMINATIONS, ANSWERED AGAINST REAL DATA rather than assumed. `expand` DOES NOT NAME A FINDING, so the brief's "a later query or expand event" is wrong and expand is excluded. recordExpansion writes a truncation capture `ref` (a 16-hex digest of tool output), a tool, a shape and free-text `asked`; none is a graph key, and all 64 live expand events on this machine carry sessionId: null, so they could not be scoped to an injection even if they did. Including it for symmetry would have added a branch that can never fire -- a metric with no producer, the exact defect this effort exists to close. It still counts as OPPORTUNITY, the only role it can honestly play. THE JOIN IS 99.97% UNATTRIBUTED AND THE JOIN IS NOT WHAT FAILED. 2,994 of 2,995 live tool-outcomes report joinMethod 'none' -- every one because no injection existed for that tool call. There are 2 inject events in 16,387 rows; the graph holds one finding. On the single tool call that did receive one, the join worked on its strongest key. So the denominator is 1 and the rate is null, not 0: one observation is not a rate, and the query channel has produced zero events on this machine, so a 0% would measure the absence of a producer. The audit prints no line at all rather than a figure. A MEASUREMENT-BIAS DEFECT IN THE FIRST DRAFT, recorded because it is the class this plan hunts and because it was biased pessimistic, which is no better. Keying "the join failed" on the episode scored 2,559 tool calls of a session that contained ONE injection as failed joins, where the true number is 0 -- episodeId IS the session id. Re-keyed onto the tool-call id, recordToolOutcome's own primary key, and pinned by a test that reproduces the 2,559-row shape. Also: the read window, not the join, is the binding constraint here. Both inject events sit at lines 502 and 883 of 6,871 and have aged out of the 2 MB tail, so the windowed denominator is 0 while the full-file one is 1. Wired to a real reader rather than parked for Task 8: renderAudit calls referenceNote -> referenceRate -> classify, so the allowlist stays at 5. 24 tests; 21 mutations, 21 killed, 0 survived, no vacuous test. Five mutations survived the first run and each exposed a real gap -- the two field guards were untested alone, nothing asserted the metric reached a human, inTimeOrder had no behavioural effect until row order was made contract, and the positive path was killed by nothing until M21 was added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plan): record task 6 in the execution state Layer 1 shipped. The two determinations it was told to make rather than assume both came back negative on live data -- expand names no finding, and the join's 99.97% unattributed rate is an absence of injections rather than a failure to attribute them -- so the honest output is a null rate over a denominator of 1, and the audit prints nothing at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(metrics): attribute a reference by finding key and time order, not by session An unverifiable model-supplied id is not evidence. wiki_query writes `options.sessionId ?? null`, so scoping the numerator on the session made it depend on the model choosing to identify itself -- the concern raised in round 0, and the fix is to stop depending on the field rather than hope for cooperation. Plan 1 settled the same question for the `answers` edge. So a query naming key K now counts as a reference to any injection of K that precedes it in time, in the same graph, whatever session either came from. Time order is the ONLY remaining guard, so it carries the whole weight of the attribution -- mutation M3 removes it and the "does not count a query that preceded the injection" test fails, which is the point. THE COST IS DISCLOSED WHERE THE NUMBER IS READ. Two concurrent sessions on one repository can cross-attribute: A's injection of K plus B's later query for K reads as a reference, and nothing in the log can separate them once the session id is gone. referenceNote -- the one place a human meets the figure, via renderAudit -- now says so beside every rate it prints, not only in the task report where nobody quoting the number will look. unscopedReferences is REMOVED rather than left at a permanent zero, which a later maintainer would read as evidence the loss had been fixed. sessionOf survives for the `unknown` determination alone, where a session is genuinely what the question is about. THE READ WINDOW IS REPORTED, NOT FIXED, and the reason is a bias fixing it would have introduced. referenceRate now returns `windowed`, from readTruncation read immediately after the only read that sets it, and the note appends "the denominator can understate" when it is true -- live, the repo graph reports denominator 0 / windowed true where the whole-file truth is 1. Giving only the INJECTION arm unwindowed access (it already has it, via BALANCE_KINDS and EVIDENCE_KINDS; `query` has neither) would make the two arms cover different time spans: every old injection would enter the denominator with no chance of a matching query in the window and score not-referenced, manufacturing a pessimistic rate out of the asymmetry -- the same shape as the episodeId defect from round 0. Unwindowing the query arm means widening EVIDENCE_KINDS, which two other consumers read, and this project's standard says that needs a per-consumer verdict comparison. A caller wanting the unwindowed truth passes its own events. 28 tests; 25 mutations, 25 killed, 0 survived, no vacuous test. Live figures unchanged where it matters: still zero query events on this machine, so the ceiling rose and the value did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plan): record task 6 fix round 1 in the execution state Session-scoped reference attribution is gone: an unverifiable model-supplied id is not evidence, so time order is the only guard and the cross-attribution cost is disclosed beside the figure. The read window is reported via a windowed flag rather than fixed, because unwindowing one arm alone manufactures a pessimistic rate from the asymmetry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(metrics): add layer 2 causal utility by leave-one-out Nobody in this space measures per-item value causally in production -- the state of the art is offline eval sets. Withhold ONE finding while serving the rest of that touch, then compare what the session spent reading the anchor afterwards. Guarded, because an experiment that degrades a turn owes an accounting: never a pinned or human-origin finding (a person asserted it), never a key whose node cannot be read, never more than one per touch, never more than one per SESSION, never on a command surface (no read event can join to command text), never fewer than two kept findings and never inside inHoldout -- either would make it withhold everything and call it a leave-one-out. Four prior injections before entry. Worst case a user can feel: one extra file read per session. Withholding runs AFTER fit, so the withheld arm differs by one finding rather than by one plus whatever took the freed budget, and the delivered set is re-priced so tokens bills only what was sent. Exploration at eps=0.10 runs BEFORE fit, so a low score cannot keep a finding from ever being served again. Statistics that can refuse: exact permutation p-values (84 relabellings at the 6-served/3-withheld floor, so the floor and q=0.10 are compatible by arithmetic), Benjamini-Hochberg across every candidate row, empirical-Bayes shrinkage with both the prior mean and the weight taken from the data, and the serving-policy version on every observation so effects are never pooled across a config change. Below the floor there is no verdict -- and that is not a zero. Disjoint from Layer 1 by construction: loo.mjs never reads a query event, usage.mjs never reads a token count, and tests move each layer while the other stays deeply equal. Task 8 would otherwise correlate a quantity with itself. On this machine it reports nothing at all, which is the correct output: 0 file-surface injections in 16,875 rows, one finding in the graph, no anchor with two. effects() is [], looNote() is null, and the audit gains no line. 43 tests, 43/43 mutations killed, every test killed by at least one mutation. Two defects found in my own work that way: a publication gate on costObservations that could not change any verdict (deleted -- an unfalsifiable guard is worse than none) and exploration wired but unproven. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(metrics): calibration loop, graph balance sheet in the report, allowlist 5->4 Does Layer 1's cheap LABEL predict Layer 2's expensive EFFECT? calibration() in the new hooks-core/crosslayer.mjs answers it, and on this machine its only honest answer is a refusal: Layer 1 publishes no rate (0 classifiable observations, 0 reference events) and Layer 2 has 0 observations, so the verdict names BOTH insufficient inputs and there is no `gap` key at all. Not gap: 0, which would read as "measured, and they agree" -- the same reason Task 7 removed a field rather than zero it. Seven distinct refusals, each with its own verdict, each pinned by a test and killed by its own mutation. Independence with Layer 2 is preserved by construction: only Layer 1's categorical label and only Layer 2's effect magnitude, joined on the finding key. Proven from both sides -- flipping the labels over identical Layer 2 data flips the gap's sign, and moving Layer 1's rate without moving a label leaves the gap byte-identical. balanceSheet() gains layer1, layer2, calibration and consolidation -- composed in crosslayer.mjs rather than metrics.mjs, which is the base of the import graph and cannot import back into loo/usage/consolidate. get_optimization_report serves the sheet as `graph` and renders it, with currency on the measured-counterfactual line and on no line saying "estimate". Three of this task's premises were stale. approxCost and the hardcoded $3/1M do not exist anywhere in the repo, so no change was manufactured to have one -- pricing.mjs and provider-pricing.ts were already right, and what was open was the RULE, now checked against the rendered text with a rate configured. hasOutstandingContradiction already has its consumer at staleness.mjs:497, and no promotion path exists for utility to feed: every confidence write in hooks-core happens at creation. So mayPromote was NOT added as an unreachable gate; the invariant is enforced instead, and a mutation that adds a real promotion kills both of its tests. consolidationRatio is wired: cost-to-derive against cost-to-carry, labelled an estimate and never priced, which is the most legible statement of what the graph is for. Its allowlist excuse was doubly stale -- expand.promote does persist derivedCost and src/server/disclosure.ts calls it. Allowlist 5 -> 4. Two measurement-bias defects found in my own new code by mutation, both flattering to the project: a publication gate of `gap > 0` would have published a calibration resting on a ~1e-13 floating-point gap as "suppress 0 more tokens/touch" (MIN_GAP_TOKENS = 1, tied to the unit the verdict prints), and the rendered re-read line printed the confirmed figure without the repeats the classifier could not judge. Also fixes the headline Task 7 left: "Nothing addressable found." is now scoped to the remediation queue, so it can no longer be a false negative printed directly above a published Layer 2 verdict. 30 mutations, 30 killed, 0 survived; all 29 new tests killed by at least one mutation. Full suite 224 suites / 2,849 passed, green twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(keepwarm): close the loop -- refreshes now learn whether they paid off keepWarmDecision spent money on refreshes and could never check whether one bought a read, because both recording halves had no call site. The turn-arrival signal was already there: gapDistribution is fitted to inter-turn gaps from metrics.jsonl, so the same log answers the outcome question directly -- a 5m refresh was used if a turn of the same session arrived within five minutes of it. scoreRefreshes scores every recorded refresh whose TTL has elapsed, on the Stop path, and shouldKeepWarm reads the rate back from the outcome array the tripwire has already read. Four refusals, each declining in the direction that costs this project its own good news: a hit is not scored before its window closes (scoring hits early while misses wait out the TTL is right-censoring, biased upward); an arrival log that no longer reaches the refresh records NOTHING rather than a miss, because an absent signal is not a miss; a refresh naming no session is unattributable; and one with no id cannot be paired, so scoring it would score it every turn. An observation may only ever LOWER a modelled probability, never raise it. A recorded hit is P(gap < ttl) -- 99.5% here -- while keepWarmDecision's probability is the narrow ping window P(ttl < gap < 2*ttl), 0.19% here, so substituting one for the other would have recommended refreshing forever. It is ttlTier, whose hit IS the same estimand, where the observation does real work, and it is bounded rather than substituted even there. recordRefresh STAYS on the reachability allowlist, and that is the finding: nothing in this repository issues a refresh. cache_audit is the only consumer of the decision and it prints a recommendation; the prompt cache belongs to the client. Recording a refresh at the recommendation site would compute a hit rate over refreshes never bought and make keep-warm look self-funding -- the ninth instance of the measurement-bias class on these plans, and the first avoided by declining to wire something. Allowlist 4 -> 3. 20 tests, 26 mutations, 26 killed, every new test killed by at least one. Full suite 224 suites / 2,869 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(wiki): a recall probe that can fail, so the no-embeddings stance is falsifiable The design says embeddings get added if measurement shows recall loss, and nothing measured recall. The plan's own probe could not have: findingsFor walks derived_from edges backwards into an anchor, and the edge F -> A is what MAKES A an anchor of F, so "retrieve each finding from its own anchor" asks whether an edge that exists exists. Its rate is 1.0 for every graph forever, and `recall: 100%` in the savings report reads as "retrieval is perfect, embeddings unnecessary" -- one unfalsifiable claim swapped for another. So the by-construction check ships as `integrity`, carrying its own "NOT a recall rate" disclaimer, and the rate comes from leave-one-edge-out instead: delete each anchor edge, then ask whether traversal from the anchor's contains-neighbourhood or BM25 over the anchor's own key still surfaces the finding. Across four fixtures the measured rate is 0, 0.5, 1 and null while integrity is 1.0 on every one of them. Four refusals, each costing this project its own good news: an observation floor of 10 distinct findings (a resolution argument, not an effect-size convention); a corpus no larger than the retrieval limit, where nothing is cut for budget and the lexical arm is more permissive than production; a dangling anchor edge as UNPROBEABLE rather than a miss; and strict aggregation over every anchor edge. On this machine it refuses. The project graph holds one finding, so `rate: null` with the count stated instead. The shared graph holds 14 active findings and all 14 are unprobeable -- every one anchored to a file id with no node in the graph -- so integrity is 1.0 while nothing is measurable, which is exactly why the two numbers are reported apart. 32 mutations, 32 killed. Two of the new tests were vacuous and only the mutation run found them: a cap assertion read a reported all.length while BM25 ranked over a separate corpus variable, and a fail-open test passed a nonexistent directory where load returns an empty graph rather than throwing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(metrics): significance-test the calibration headline * fix(metrics): preserve honest measurements after rebase * style: satisfy formatting and whitespace checks --------- Co-authored-by: franklinic <franklin@ivorycloud.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
hamsterish
pushed a commit
to hamsterish/token-optimizer-mcp
that referenced
this pull request
Oct 2, 2026
* feat: complete live graph integration * style: format mcp tool profile * feat: support live graph across cli clients * fix: address live graph review findings
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.
What changed
TOKEN_OPTIMIZER_TOOL_PROFILE=full.wiki_write; keep the external harvester as an optional fallback.Why
The Codex plugin used the wrong MCP manifest key and referenced an incomplete lifecycle, the server advertised all 98 schemas into every session, Node 25 could fail through the older native SQLite dependency, and semantic graph writes were not reachable from the active Codex model. Those gaps made installation appear complete while the intended cross-session/cross-project system was not proven end to end.
Impact
Codex can now load the plugin and use its own reasoning to harvest findings without sending transcripts to a second model. Default MCP catalog serialization falls from approximately 35,415 to 5,604 tokens while advanced users retain the complete catalog. Portable lessons can flow across sessions and repositories, and the dashboard has a CI-gated proof that the graph it displays is the graph the hooks wrote.
Validation
npm run buildnpm run doctor— 11/11 checkstools/liston Node 22, 24, 25, and 26npm run verify:live-graph— agent provenance, cross-session and cross-project delivery, dashboard routes, and +43,968 controlled net tokensnpm run verify:ui— 22/22 browser checksnpm run verify:interactions— 57/57 interaction/security checksgit diff --checkThis branch starts from
masterafter the PR #297 review fixes; those resolved threads and their commit remain separate.