fix: strip ANSI escapes before parsing token counts and model banners - #33
Merged
Conversation
… banners
Workers are spawned on a pipe, but a harness that colorizes unconditionally —
or is told to by FORCE_COLOR/CLICOLOR_FORCE — writes SGR codes into the exact
banners and summaries these regexes read. Two distinct failures:
parse_token_count("tokens used: \x1b[1m1,234\x1b[0m") -> None
parse_reported_model("model: \x1b[36mgpt-5.6-sol\x1b[0m") -> "\x1b[36mgpt-5.6-sol\x1b[0m"
The second is the dangerous one. An escape around the LABEL breaks the match
and the fact is merely lost; an escape around the VALUE still matches and
captures the escapes INTO the value, so a model id that can never match a
known model is written to the eval row. That is silent corruption of exactly
the attribution PR #29 worked to make reliable, and it lands in the taxonomy's
unattributed quarantine looking like a different problem.
Strip at PARSE time, never at capture time: worker.log is the raw record of
what the worker emitted, and rewriting it would break the documented invariant
that captured worker output is never modified. strip_ansi short-circuits when
no ESC is present, so the common path is one substring scan.
Covers CSI, OSC (BEL- or ST-terminated) and two-character escapes, and applies
to codex_usage_from_log too — a colorized JSON stream puts escapes ahead of the
'{' and `ask` reports no model use at all. A guard test pins that text merely
containing "[1m]" without ESC is left alone, so this cannot over-strip.
Matches upstream PR NateBJones-Projects#99, which is unmerged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The third defect from the upstream audit (see #32 for the first two). Confirmed live on
main— not theoretical:Why this matters more than a missing token count
The two failure modes are not equally bad.
An escape around the label breaks the literal match, and the fact is simply lost — annoying, visible as a blank.
An escape around the value still matches. The capture group swallows the escape codes and writes them into
reported_model, so the eval row carries a model id that can never match a known model. That is silent corruption of exactly the attribution PR #29 worked to make reliable, and it surfaces in the taxonomy's unattributed quarantine looking like an entirely different problem.Workers are spawned on a pipe, so this needs a harness that colorizes unconditionally or is told to by
FORCE_COLOR/CLICOLOR_FORCE— but nothing in Ringer prevents that, and the failure is invisible when it happens.The fix
Strip at parse time, never at capture time.
worker.logis the raw record of what the worker emitted, and rewriting it would break the documented invariant that captured worker output is never modified (README,redact_spec).strip_ansishort-circuits when no ESC byte is present, so the common path is a single substring scan. Coverage: CSI (\x1b[...m), OSC (BEL- or ST-terminated, e.g. terminal-title sequences), and two-character escapes.Also applied to
codex_usage_from_log: a colorized JSON stream puts escapes ahead of the{, the event is skipped, andaskreports no model use at all.Verification
ringer.pyreverted and the new tests kept, all four fail — includingAssertionError: '\x1b[36mgpt-5.6-sol\x1b[0m' != 'gpt-5.6-sol'[1m]without an ESC byte is returned untouched, so ordinary prose and log lines are never mangledUpstream: PR NateBJones-Projects#99 (unmerged).
🤖 Generated with Claude Code