Skip to content

fix: recover native parser evidence and successful reads (#128) - #130

Merged
lukemaj merged 5 commits into
mainfrom
feat/128-parser-gaps
Oct 19, 2026
Merged

lukemaj merged 5 commits into
mainfrom
feat/128-parser-gaps

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Worktree: /home/luke/dev/toolboxmd/agent-observer-worktrees/128-parser-gaps; branch feat/128-parser-gaps.
  • Base: 1fbbb0fe3098842d19e04bc5a6b35a3667ecda90; head: 8647f156de05bc506f574ae94f28024fd5d3d5d7, pushed to origin.
  • VERSION and all plugin manifests unchanged at 1.3.0 by explicit user instruction; Unreleased changelog.
  • No ContextMD import or report.py changes. 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

  • Failing first: python3 -m unittest tests.test_parser_gaps on 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.
  • Expanded fixtures exercise numeric duration compatibility, malformed durations, legacy/sectioned direction boundaries, reimport idempotency, incremental Read output, MCP joins and timeout/retries, locationless completions, opaque JSON file contents and late instruction-version evidence.
  • Existing test fixture changes mark synthetic successful skill reads explicitly and expect current import revision 10; historical revision-9 seeds remain unchanged.
  • python3 -m unittest discover -s tests: exit 0 on 8647f156de05bc506f574ae94f28024fd5d3d5d7, one suite at a time:
Ran 871 tests in 61.361s

OK
  • Fifteen parser-gap fixture tests are included. No test runs remain active or planned.
  • 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 for agent-observer --version to print 1.3.0, then used sqlite3.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, and grok.import_grok_session with the same shared _SyncCtx parent 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' failed lists 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

SELECT harness,COUNT(*),SUM(instructions_sha256 IS NOT NULL),SUM(direction_status='ready') FROM sessions WHERE harness IN ('codex','claude') GROUP BY harness;

Before: [["claude", 238, 0, null], ["codex", 1064, 0, null]]

After: [["claude", 238, 217, 200], ["codex", 1064, 920, 901]]

codex_exec_duration

SELECT COUNT(*),SUM(duration_ms IS NULL) FROM events WHERE session_key LIKE 'codex:%' AND family='tool_result' AND name='exec';

Before: [[61683, 61683]]

After: [[61896, 0]]

grok_mcp_errors

SELECT COUNT(*) FROM import_errors WHERE harness='grok' AND error='unknown_record' AND line_excerpt IN ('call_id,server_name,timeout_sec,tool_name,ts,type','auth_retry_attempted,call_id,duration_ms,is_timeout,reconnect_attempted,server_name,success,tool_name,ts,type','auth_retry_attempted,call_id,duration_ms,error,is_timeout,reconnect_attempted,server_name,success,tool_name,ts,type');

Before: [[2192]]

After: [[0]]

reads_by_state

SELECT substr(session_key,1,instr(session_key,':')-1),status,COUNT(*) FROM events WHERE family='skill_read' GROUP BY 1,2;

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

SELECT COUNT(*) FROM events WHERE session_key LIKE 'codex:%' AND family='tool_call' AND name='Read';

Before: [[0]]

After: [[0]]

grok_tool_results

SELECT COUNT(*),SUM(duration_ms IS NOT NULL) FROM events WHERE session_key LIKE 'grok:%' AND family='tool_result';

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 Read occurrences remain zero, so that shape is fixture-verified only.

SELECT family,COUNT(*) FROM events WHERE session_key LIKE 'grok:%' AND ((family='tool_call' AND json_type(detail_json,'$.timeout_sec')='integer') OR (family='tool_result' AND json_type(detail_json,'$.is_timeout') IN ('true','false'))) GROUP BY family;

Result: [["tool_call", 554], ["tool_result", 553]]

SELECT COUNT(*) FROM (SELECT session_key,family,native_id,COUNT(*) n FROM events GROUP BY session_key,family,native_id HAVING n>1);

Result: [[0]]

SELECT error,COUNT(*) FROM import_errors WHERE harness='grok' GROUP BY error;

Result: [["missing_id", 1], ["unknown_record", 116], ["usage_conflict", 110]]

PRAGMA integrity_check;

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, dropping 17.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_item already accepts call_id or id for tool-result events and Read call events, but its Read output lookup used only call_id. A real-adapter fixture with id-only call/result records reproduced attempted instead of ok. 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

  • Failing first at 634fb26: all three new regression tests failed for the intended evidence/state mismatch.
  • python3 -m unittest tests.test_parser_gaps tests.test_grok: 64 tests passed.
  • Final full suite on 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.
  • Current delta touches only both adapters and tests/test_parser_gaps.py; VERSION/manifests unchanged.
  • Previous ledger-copy aggregate evidence remains recorded above for 634fb26. This follow-up's newly reproduced shapes are fixture evidence; no live ledger was accessed in this follow-up.

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 has review/independent=success from lukemaj. 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.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Independent review round 1. Reviewed d60845d against 1fbbb0f. Verdict: request changes. Findings relayed unchanged:

  1. P2: Successful JSON file reads are misclassified. codex.py:2167 decodes every string result as JSON before deciding success. A successful Read returning file text {"version":1}, [], or 123 becomes attempted; file text {"error":"example"} becomes failed. _ingest_response_item persists that classification, losing successful-read evidence. The fixture covers ordinary text and a content wrapper, but no raw JSON file contents.

  2. P2: Successful Grok completions without locations lose instruction-version evidence. grok.py:2669 updates the prior read’s status but never observes its stored path. Given an in_progress read of a versioned AgentsMD Skill followed by completed without locations, neither record calls identity.observe_path: the first is pending, and the second has no locations. Sessions relying on this fallback miss agentsmd_version; privacy-version replay can also clear previously recorded identity. The new locationless-completion fixture tests only failure.

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.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

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.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Independent review round 3, relayed unchanged:

Verdict: PASS for 634fb2671ece0522a40b7b45d3bf9827fedac4c0 against 1fbbb0fe3098842d19e04bc5a6b35a3667ecda90.

  • Blocking findings: none.
  • Nonblocking findings: none.
  • All prior objections are addressed: opaque Codex Read contents, locationless Grok success identity, and duplicate version observations.
  • Reviewed the cumulative diff and eleven regression tests; VERSION/manifests remain untouched.

No tests run, file changes, or GitHub writes. Final-suite and candidate replay proof remain author-owned.

@lukemaj
lukemaj marked this pull request as ready for review October 19, 2026 12:28
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

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.
Why: A single stored representative path lost version evidence, while the Codex lookup omitted the event's existing id fallback.
So what: Final full-suite and independent review are running on 1ed0847; no merge or version change is performed.

Review response

The Grok 4.7 medium finding against 634fb26 is reproduced: the exact two-path, two-update case observed only 17.3.1, dropping 17.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_item already accepts call_id or id for tool-result events and Read call events, but its Read output lookup used only call_id. A real-adapter fixture with id-only call/result records reproduced attempted instead of ok. 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

  • Failing first at 634fb26: all three new regression tests failed for the intended evidence/state mismatch.
  • python3 -m unittest tests.test_parser_gaps tests.test_grok: 64 tests passed.
  • Final full suite and compileall: pending.
  • Current delta touches only both adapters and tests/test_parser_gaps.py; VERSION/manifests unchanged.
  • Previous ledger-copy aggregate evidence remains recorded above for 634fb26. This follow-up's newly reproduced shapes are fixture evidence; no live ledger was accessed in this follow-up.

Elon record

Requirements and who asked: The user requests the Grok multipath review repair and resolution of the Codex identity-fallback uncertainty on PR #130.
Deleted: New persistence, broader event detail, extra source I/O, ContextMD changes and version/release changes.
Bottleneck: Preserve the complete pending read evidence until its result and use one identity for the Codex result and its join.
Checked myself: Traced cached-prefix/delta replay and event identities; ran three new fixtures failing first, then passed all 64 focused adapter checks.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Independent round 4 verdict, relayed unchanged:

Verdict: REQUEST CHANGES for 1ed08473af56237cdeb4a93a48cedaa5a55e3260.

Blocker P2: Incremental identity reconstruction does not replace existing evidence. The new prefix replay reconstructs counts correctly, but grok.py:1836 passes them to upsert_session, whose db.py:903 preserves non-NULL identity_json and agentsmd_version through COALESCE.

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.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Verdict: PASS for 8647f156de05bc506f574ae94f28024fd5d3d5d7 against base 1fbbb0fe3098842d19e04bc5a6b35a3667ecda90.

  • Blocking findings: none.
  • Nonblocking findings: none.
  • Grok now replaces reconstructed path counts, preserves other stored identity fields, and gives resolved instruction hashes precedence.
  • Prior objections remain resolved: multipath/locationless completions, once-per-call/path observations, opaque Codex file contents, and consistent call_id or id joins.
  • VERSION/manifests and ContextMD code remain unchanged.

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.

@lukemaj
lukemaj merged commit fcdb91c into main Oct 19, 2026
2 checks passed
@lukemaj
lukemaj deleted the feat/128-parser-gaps branch October 19, 2026 14:49
lukemaj added a commit that referenced this pull request Oct 19, 2026
Bumps VERSION, the three plugin manifests and the changelog heading for
the merges of #130 (#128 parser gaps) and #131 (#129 Cue import).

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parse records Observer already reads: direction blocks, Codex durations, Grok MCP, read success, Codex Read

1 participant