Repository navigation
fix: YAML-escape frontmatter scalars (harvest→review break found in UAT) - #111
Conversation
|
Warning Review limit reached
More reviews will be available in 14 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughAdds a ChangesYAML Frontmatter Serialization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
…sing End-to-end UAT (harvest -> review -> eval) surfaced a real break the unit tests missed: stage_pending_decisions built frontmatter with raw f-string interpolation, so a value containing a colon — e.g. harvest's "reasoning: Imported from Codex native memory: memory_summary.md" — produced invalid YAML. `spanweave review` then crashed with "mapping values are not allowed here" and ALL 80 harvested native-codex records failed to route (Accepted 0). gemma extractions rarely contain colons, so this stayed latent until native-memory harvest (#110) reliably produced them. Fix: a `_yaml_scalar()` helper serializes type/source/reasoning/tags via yaml.safe_dump(flow style), so colons, quotes, and newlines are escaped. Regression test asserts the staged frontmatter round-trips through yaml.safe_load with a colon+quote-laden reasoning value. Verified: full UAT now shows review "Accepted 82", records routed by type (decisions/findings), and eval reports Track A=80 / Track B=2. 326 tests pass, ruff + pyright clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a723fbd to
2b8fc79
Compare
…mend) The regression test guarding the colon-in-frontmatter fix didn't make it into the prior commit (the Edit failed silently, so only extractor.py was committed). Add it now. RED-proven: fails on raw-interpolation, passes with _yaml_scalar. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…001) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The bug (found by end-to-end UAT, not unit tests)
Running the full harvest → review → eval chain on real Codex data exposed a break:
stage_pending_decisionsbuilt YAML frontmatter via raw f-string interpolation, so any value with a colon broke it. Harvest reliably produces such values:spanweave reviewthen crashed (mapping values are not allowed here) and all 80 harvestednative-codexrecords failed to route — Accepted 0. gemma extractions rarely contain colons, so this stayed latent until native-memory harvest (#110) started producing them.Fix
_yaml_scalar()serializestype/source/reasoning/tagsthroughyaml.safe_dump(flow style) so colons, quotes, and newlines are escaped. Plus a regression test asserting staged frontmatter round-trips throughyaml.safe_loadwith a colon+quote-laden value.Verification — full UAT now green
review --auto-accepton 82 records (2 gemma + 80 native)evaldual-track326 tests pass · ruff clean · pyright 0 errors.
★ Lesson— this is the "verify at the level the bug manifests" rule in action: harvest's unit tests asserted records were written; only running them through review revealed they couldn't be parsed back.🤖 Generated with Claude Code
Summary by CodeRabbit