Repository navigation
fix: recover native parser evidence and successful reads (#128) - #130
Conversation
|
Independent review round 1. Reviewed d60845d against 1fbbb0f. Verdict: request changes. Findings relayed unchanged:
Non-blocking findings: none. Author response: both findings are corrected in d9b717c with regressions. Final full-suite rerun is waiting for #129 to release the machine's suite slot. Exact-head independent re-review is running. |
|
Independent review round 2. Reviewed d9b717c against 1fbbb0f. Verdict: request changes. Finding relayed unchanged: P2 blocker: Grok double-counts version evidence for successful updates containing locations. grok.py:2677 observes the persisted path after line 2630 already observed that same update’s location. Locationless completions count once. Consequently, one completed read of version A with locations and two reads of version B with locationless completions produce counts A=2, B=2; identity.py:404 selects A when encountered first, despite B having more successful reads. Ensure each successful read contributes once while retaining locationless completion support. Both prior objections are addressed: Codex string contents remain opaque, and Grok locationless success now observes the persisted path. Non-blocking findings: none. Author response: 634fb26 deduplicates version observations by native call ID and path within the reader. Both successful location paths and persisted paths use the same owner. A regression checks one A read against two B reads, with repeated witnesses, asserting A=1, B=2 and version B. Round 3 is reviewing the exact candidate; the final full suite is running after #129 released the slot. |
|
Independent review round 3, relayed unchanged: Verdict: PASS for
No tests run, file changes, or GitHub writes. Final-suite and candidate replay proof remain author-owned. |
|
What changed: Retain every pending Grok read location through a later locationless success, and join id-only Codex Read outputs using their accepted event identity. Review responseThe Grok 4.7 medium finding against 634fb26 is reproduced: the exact two-path, two-update case observed only Codex check: Proof
Elon recordRequirements and who asked: The user requests the Grok multipath review repair and resolution of the Codex identity-fallback uncertainty on PR #130. |
|
Independent round 4 verdict, relayed unchanged: Verdict: REQUEST CHANGES for Blocker P2: Incremental identity reconstruction does not replace existing evidence. The new prefix replay reconstructs counts correctly, but grok.py:1836 passes them to Concrete source trace: first sync imports one completed version-A read and two pending version-B reads; second sync appends locationless B completions. The reader computes A=1, B=2, but the ledger retains A=1 and version A. A fresh single-pass import selects B, so identity still depends on sync sequence. The new incremental fixture starts without existing identity and misses this case. Nonblockers: none. Codex identity fallback, opaque file contents, malformed-prefix guards, and once-per-call/path deduplication look correct in the inspected paths; VERSION/manifests remain unchanged. Static review only: no tests, file changes, GitHub writes, or children. Full-suite proof remains author-owned. Author response: reproduced the finding with another failing-first fixture. The Grok adapter now replaces the reconstructed plugin-path counts after a complete source replay, preserving other identity evidence and preferring the resolved instruction-hash version. The regression confirms both late A-to-B version correction and instruction-hash precedence. Focused checks: 65 tests passed. Final suite and new exact-head review follow. |
|
Verdict: PASS for
Static review only: no tests, file changes, GitHub writes, or children. Final-suite proof remains author-owned; id-only Codex Read coverage establishes adapter handling, not native-writer behavior. |
What changed: The five native parser gaps are implemented in PR #130, with read-state reports and old-source replay.
Why: Existing records were dropping direction identity and timing or overstating successful procedure reads.
So what: The tested and independently reviewed implementation is ready for the merge decision; merge and release remain unperformed.
Closes #128.
Candidate and authority
/home/luke/dev/toolboxmd/agent-observer-worktrees/128-parser-gaps; branchfeat/128-parser-gaps.1fbbb0fe3098842d19e04bc5a6b35a3667ecda90; head:8647f156de05bc506f574ae94f28024fd5d3d5d7, pushed to origin.report.pychanges. Shared ingest/privacy changes are limited to replayed durations, read-state enums and MCP metadata.Elon record
Requirements and who asked: The user and #128 require the five measured parser repairs, failing-first fixtures, successful procedure-read counts and idempotent old-ledger repair, including the follow-up multipath review repair and Codex identity check.
Deleted: New hooks, new importers, ContextMD work, release/version changes and new schema tables are unnecessary.
Bottleneck: Correct derivation from existing native records.
Checked myself: Read the exact parser paths, observed six new fixture tests fail on the base, passed the final 871-test suite, inspected native key/type aggregates and replayed only a read-only backup of the 1.3.0 ledger.
Tests
python3 -m unittest tests.test_parser_gapson the original code ran six tests: five failures and one missing-identity error, one or more for each of the five Issue gaps. The same six then passed.python3 -m unittest discover -s tests: exit 0 on8647f156de05bc506f574ae94f28024fd5d3d5d7, one suite at a time:python3 -m compileall -q agent_observer tests: exit 0 on 8647f15.git diff --check: passed.Ledger-copy proof (historical candidate 634fb26)
Audited bestia on 2026-10-19 at
634fb2671ece0522a40b7b45d3bf9827fedac4c0. These retained aggregates were not rerun for the follow-up; its new edge cases are fixture-verified and no live ledger was accessed. Waited foragent-observer --versionto print1.3.0, then usedsqlite3.connect("file:<live-ledger>?mode=ro", uri=True).backup(scratch_connection)into~/.local/state/agent-observer-scratch/128/observer.db. No live-ledger writes or sync. No prompt/transcript contents were printed.Replayed the captured source set using
db.connect(scratch)/db.init_db,claude.import_claude_file,codex.import_codex_file, andgrok.import_grok_sessionwith the same shared_SyncCtxparent index used by sync. The set contains all 238 Claude and 1,064 Codex sources plus 391 Grok sessions owning tool/read evidence or unknown records: 1,693 import operations, 0 failed, 0 missing. Grok sources were restaged at import revision 9 in scratch to verify final-code repair; fixtures verify repeated imports without duplicate rows.The first disposable measurement counter mistakenly added the adapters'
failedlists to an integer; this affected the instrument only. The corrected measurement completed with 0 failures. Reusing a shared Grok parent index avoids repeatedly scanning the containing tree.Native logs were still appendable while the ledger copy stayed isolated, so totals grew between baseline and replay. Count changes are reported explicitly rather than attributing all new events to parser repair.
direction_identity
Before:
[["claude", 238, 0, null], ["codex", 1064, 0, null]]After:
[["claude", 238, 217, 200], ["codex", 1064, 920, 901]]codex_exec_duration
Before:
[[61683, 61683]]After:
[[61896, 0]]grok_mcp_errors
Before:
[[2192]]After:
[[0]]reads_by_state
Before:
[["claude", null, 329], ["codex", "completed", 5421], ["codex", "failed", 109], ["grok", null, 2994], ["opencode", "ok", 66]]After:
[["claude", "ok", 336], ["codex", "failed", 109], ["codex", "ok", 5436], ["grok", "attempted", 7], ["grok", "failed", 13], ["grok", "ok", 2979], ["opencode", "ok", 66]]codex_Read_calls
Before:
[[0]]After:
[[0]]grok_tool_results
Before:
[[31471, 31145]]After:
[[32056, 31728]]Direction columns are harness, session count, non-NULL instruction hash count and ready-direction count. Codex duration columns are total exec results and NULL durations. Skill-read columns are harness, state and count. Grok result columns are total results and results with timing. Successful procedure reports now exclude failed/attempted/pending states. Native Codex
Readoccurrences remain zero, so that shape is fixture-verified only.Result:
[["tool_call", 554], ["tool_result", 553]]Result:
[[0]]Result:
[["missing_id", 1], ["unknown_record", 116], ["usage_conflict", 110]]Result:
[["ok"]]The 554 MCP call rows and 553 result rows are distinct native call identities; repeated start/completion witnesses join them rather than adding counts. Residual Grok diagnostics remain visible and outside these five fixes.
Review follow-up
Review response
The Grok 4.7 medium finding against 634fb26 is reproduced: the exact two-path, two-update case observed only
17.3.1, dropping17.1.0. The same loss occurred when completion arrived in a subsequent sync. Both cases have new real-adapter fixtures. The reader now remembers all validated paths by call ID and observes each once only on success. An incremental import rebuilds that state from the complete prefix already cached for usage replay. This introduces no new disk reads, schema or privacy detail fields.Codex check:
_ingest_response_itemalready acceptscall_id or idfor tool-result events and Read call events, but its Read output lookup used onlycall_id. A real-adapter fixture with id-only call/result records reproducedattemptedinstead ofok. The output identity is now normalized once and used by both the result event and Read lookup/update. This proves the accepted adapter shape; it does not establish that a native Codex writer emits id-only Read outputs. Native Read occurrences in the previous bestia audit were zero.Proof
python3 -m unittest tests.test_parser_gaps tests.test_grok: 64 tests passed.8647f156de05bc506f574ae94f28024fd5d3d5d7: 871 tests, OK, 61.361s.python3 -m compileall -q agent_observer tests: exit 0. Four new regressions in this follow-up ran failing first before their repairs.tests/test_parser_gaps.py; VERSION/manifests unchanged.The incremental path reconstruction also exposed the ledger's first-known-value policy: an existing version-A identity prevented newly completed version-B evidence from replacing it. The fourth new fixture reproduced this. The adapter now refreshes reconstructed path counts after replay while retaining other identity evidence and preferring a release mapped from the instruction hash. This preserves full-import/incremental equivalence without changing generic session-upsert policy or adding storage.
Delivery state
PR #130 is implemented and proved at
8647f156de05bc506f574ae94f28024fd5d3d5d7. Exact-head independent review round 5 passed with no blocking or nonblocking findings; the head hasreview/independent=successfromlukemaj. No merge, version preparation, release, installation or deployment. Website/SEO impact: none, because no website or URL changes.The scratch database and its task directory were deleted and verified absent after retaining these aggregates. Disposable local proof files are removed after this record is published. Worktree and branch remain for the open PR; the merger retires them. Residual non-MCP diagnostics and seven attempted Grok reads remain visible. Native Codex Read coverage on bestia is fixture-only because no such calls occur in this ledger.