Remove vestigial progress-chain parent repair#175
Conversation
The _scan_file_progress / _scan_progress_chains / _repair_parent_chains trio was superseded by the PassthroughTranscriptEntry mechanism. Since `progress` entries carry uuid+sessionId, they are parsed as Passthrough nodes that sit in the DAG as valid parents — so the repair only ever fired for a progress entry lacking a sessionId, a case that occurs in no real transcript. In that (unreachable) case the new behavior is not byte-identical to the old repair: instead of re-nesting the orphaned child under the dropped progress entry's nearest surviving ancestor, build_dag clears the dangling parentUuid and promotes the child to a root (dag.py:190-222) — a tree-shape difference, plus possibly a non-suppressed orphan warning (the debug-suppression only covers progress passthroughs). It degrades gracefully, never crashes. Verified behavior-neutral on all reachable inputs: neutering _repair_parent_chains to a no-op left the full unit + snapshot suite passing unchanged. Removing the trio and its two call sites (directory + single-file load) drops a whole parse-time pass and a redundant directory re-scan. Tests: drop the unit tests that exercised the deleted helpers (they already asserted no-op behavior) and keep the genuine behavioral guarantees — no orphan warnings and progress-as-Passthrough on real data — under a renamed TestProgressEntryPassthrough. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR removes internal progress chain repair logic from the converter module and updates integration tests. Three helper functions and their invocation sites are deleted, eliminating post-load parent-link rewrites for progress entries. Tests are refactored to validate that progress entries now function as PassthroughTranscriptEntry DAG nodes without requiring repair. ChangesProgress chain repair removal and validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
Removes the
_scan_file_progress/_scan_progress_chains/_repair_parent_chainstrio fromconverter.pyand its two call sites (directory and single-file load).Net: −90 lines of code, plus a test-file trim (+20 / −244 overall).
Why it's safe
The trio was superseded by the
PassthroughTranscriptEntrymechanism. Sinceprogressentries carryuuid+sessionId, they are parsed as Passthrough nodes that sit in the DAG as valid parents — so_repair_parent_chains(which explicitly skips entries already present in the parsed set) could only ever fire for aprogressentry lacking asessionId. That case occurs in none of the real fixtures (0/11 progress entries), and even the synthetic test helper sets asessionId, so the prior unit tests already asserted no-op behavior.In that (unreachable) case the new behavior is not byte-identical to the old repair: instead of re-nesting the orphaned child under the dropped entry's nearest surviving ancestor,
build_dagclears the danglingparentUuidand promotes the child to a root — a tree-shape difference, plus possibly an orphan warning. It degrades gracefully and never crashes.Removing the trio also eliminates a redundant directory re-scan in the directory-load path.
Verification
_repair_parent_chainsto a no-op left the full unit + snapshot suite passing unchanged, confirming behavior-neutrality on all reachable inputs.pyright claude_code_log/converter.pyclean;ruffcheck + format clean.Tests
Drops the unit tests that exercised the deleted helpers (they asserted no-op behavior). Keeps the genuine behavioral guarantees — no orphan warnings, and progress-as-Passthrough on real data — under a renamed
TestProgressEntryPassthrough.🤖 Generated with Claude Code
Summary by CodeRabbit