Skip to content

fix(badge): classify save-state from transcript structure, not raw text - #45

Merged
s-annam merged 1 commit into
mainfrom
fix/badge-structural-classify-issue-44
Jul 24, 2026
Merged

fix(badge): classify save-state from transcript structure, not raw text#45
s-annam merged 1 commit into
mainfrom
fix/badge-structural-classify-issue-44

Conversation

@s-annam

@s-annam s-annam commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

The statusline save-state badge classified sessions by regexing the raw transcript JSONL as flat text, so its patterns matched the same words wherever they appeared — in a user message, in assistant prose, or inside the contents of any file read into context. Classification now reads the transcript's structure (tool_use blocks, user-authored text), which prose cannot reach.

The dangerous direction was a false saved: once the words /okfmem-save appeared after the last real edit, the badge went green over genuinely uncaptured work. That fires easily — the global CLAUDE.md proactive-reminder text contains the literal string. Measured on a real transcript: 0 Edit/Write tool uses but 11 git commit and 27 okfmem-save prose matches → badge saved, 100% false.

Refs #44

What changed

  • iter_tool_uses() walks parsed dicts/lists only, never into strings — that is what isolates file contents; a tool_result carrying a file holds it as an opaque string, so it can't be read as a tool call.
  • user_text() reads only text blocks of user-role messages, skipping tool_result, so a command echoing /okfmem-save isn't the user invoking it.
  • Save signals are exact: Skill{skill: okfmem-save|primer}, Bash okfmem sync, or a real /okfmem-save / /primer invocation.
  • runs_command() requires command position, so grep -o 'git commit' no longer counts. Quoted spans are blanked before segmenting — a separator inside a quoted regex would otherwise split there and leave the tail in apparent command position, itself a path to a false saved.
  • Position is line index; work + save in one record ties to unsaved (a redundant save is cheap, a lost one is not).
  • touched_files() replaces the FILE_PATH_RE breadcrumb scan, which also listed paths quoted in prose or in files the session only read.

Accepted trade-off: a command inside bash -c "..." is missed, since quoting is not parsed. Catching it needs a mini shell lexer; Edit/Write is the dominant exactly-structural signal and a miss costs only a nag, never a false green.

Test plan

  • python3 scripts/check-leaks.py exits 0 — Leak gate OK — scanned 62 tracked files, no leaks.
  • python3 -m pytest tests/ -q197 passed, 1 skipped (10 new cases in tests/test_save_state_badge.py, incl. regressions for both false directions)
  • ruff check . reviewed (advisory) — 2 × E702, both in untouched plugins/memory_search.py; ruff format --check flags 29 files. Identical on unmodified HEAD (verified via git stash) — pre-existing, out of scope.
  • Smoke: okfmem status exits 0
  • Validated against 8 real transcripts, not just fixtures: states resolve sanely (saved / unsaved / None) and save detection fires on real signals (<command-name>/okfmem-save, Bash okfmem sync)
  • Audited iter_tool_uses's permissive fallback branch across every real transcript — zero nodes match it outside type == "tool_use"; no false-positive surface in practice
  • Quoted-span ordering verified by reproducing the leak: on grep -o 'okfmem[- ]save\|okfmem sync' f, segment-before-quote yields a false saved; the shipped order does not

Provenance

Code implementation via: Claude Opus 4.8 (1M context), high effort
Review: Claude Opus 4.8 (1M context), high effort — diff review, adversarial probing of COMMIT_RE against 16 command variants, real-transcript validation
Verification: CI verify

compute_save_state() regexed the transcript JSONL as flat text, so its
patterns matched their own words wherever they appeared: in a user message,
in assistant prose, in tool output, or inside the contents of any file read
into context. That was wrong in both directions.

False "unsaved" was the benign one — reading any file containing "git commit"
faked work, and WORK_RE matched its own pattern string, so reading this module
self-triggered. False "saved" was the dangerous one: once the words
"/okfmem-save" appeared after the last real edit, the badge went green over
genuinely uncaptured work. That fires easily, since the global CLAUDE.md
proactive-reminder text contains the literal string. Measured on a real
transcript: 0 Edit/Write tool uses but 11 "git commit" and 27 "okfmem-save"
prose matches, badge "saved" — 100% false.

Classify from structure instead:

- iter_tool_uses() walks parsed dicts/lists only, never into strings. That is
  what isolates file contents — a tool_result carrying a file holds it as an
  opaque string, so it can never be read as a tool call.
- user_text() reads only text blocks of user-role messages, skipping
  tool_result, so a command echoing "/okfmem-save" is not the user invoking it.
- Save signals are a Skill{skill: okfmem-save|primer} use, a Bash `okfmem sync`,
  or an actual /okfmem-save invocation.
- runs_command() requires command position, so `grep -o 'git commit'` no longer
  counts as committing. Quoted spans are blanked BEFORE segmenting: a separator
  inside a quoted regex would otherwise split there and leave the quote's tail
  in apparent command position — a real path to a false "saved".
- Position is line index; work and save on one record tie to "unsaved", because
  a redundant save is cheap and a lost one is not.

touched_files() replaces the FILE_PATH_RE breadcrumb scan for the same reason:
that regex also listed paths quoted in prose or in a file the session only read.

Known and accepted: a command inside `bash -c "..."` is missed, since quoting is
not parsed. Edit/Write is the dominant signal and a miss costs only a nag.

Refs #44
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.

1 participant