fix: don't let //-comment stripping corrupt string values containing URLs#78
Merged
Merged
Conversation
…URLs parse_llm_response()'s comment-stripping regex (re.sub(r'(?m)//.*$', '', ...)) matched `//` anywhere on a line, including inside JSON string values. Any LLM- extracted entity whose text contains "//" -- most commonly a URL like "https://legal.example" -- had everything from the `//` to end-of-line stripped, corrupting the JSON and making a genuinely valid response unparseable. Root cause of the nightly/tag E2E red streak since 2026-07-10: the fixed CI test document contains "https://legal.example", which llama3.2:1b reliably extracts as an entity, so every extraction attempt for that chunk hit this corruption and failed after retries. Before the A4 fail-closed hardening (#58, 2026-07-09), the same chunk failure was silently swallowed and the run still reported success; A4 correctly started failing closed on it instead of shipping an incompletely-scanned document, which is what surfaced this as a hard CI failure. Reproduced deterministically outside CI (isolated local Ollama instance, 15/15 failures before the fix, 15/15 successes after) -- not CI-infra flakiness. Replaced the naive regex with a string-aware line-comment stripper that tracks JSON string-literal state (respecting \" escapes) and only treats `//` as a comment start outside of one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Not needed for CI green: the nightly/tag E2E red streak that would have been the reason to revisit it turned out to have a deterministic root cause unrelated to runner choice (see fix/json-comment-strip-url-corruption). Co-Authored-By: Claude Sonnet 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.
Summary
//-comment-stripping regex inparse_llm_response()(src/python/marcut/model.py) was blindly stripping anything after//, which corrupted any LLM-extracted entity whose value contained a URL (e.g.https://...).//comments outside of string literals, leaving URL-bearing values intact.tests/test_model.pycovering entities with URL values.docs/BACKLOG.mdto defer PR ci: switch workflows to self-hosted local macOS runner #34 (self-hosted runner work) with rationale, since it was blocked behind this same investigation.Test plan
🤖 Generated with Claude Code