feat: add compatible turn boundary markers - #262
Conversation
ralf0131
left a comment
There was a problem hiding this comment.
Summary
Centralized fill-only TurnBoundaryProcessor that enriches entries with reliable gen_ai.turn.start/end markers before output dispatch. Well-designed with proper fail-open semantics, producer-marker preservation, bounded state tracking, and comprehensive test coverage across unit/integration/e2e layers.
Findings
- [Info]
src/normalization/turn-boundary-processor.ts:11—TERMINAL_FINISH_REASONSis duplicated inscripts/e2e/lib/e2e-scenarios.mjs:797. Consider extracting to a shared constant if the e2e validator needs to stay in sync as terminal reasons evolve. - [Info]
src/normalization/turn-boundary-processor.ts—persist()is called on everyenrich()batch. For high-throughput inputs this could add I/O overhead. The current design is correct; just flagging for awareness if profiling ever surfaces this as a hotspot.
Suggestions
The implementation correctly handles the key edge cases:
- Subagent entries are skipped (no false boundaries)
- Producer-owned markers are never overwritten (inspect-before-fill pattern)
- StateStore restore validates version and TTL
compact()caps at 4096 tracked turns with FIFO eviction
LGTM — clean, well-tested, backward-compatible change.
Automated review by github-manager-bot
…rkers # Conflicts: # src/core/input-manager.ts # tests/unit/core/input-manager.test.ts
🔍 Code Review Summary
Lifecycle Verdict
无 FAIL 项。补标器无 timer/句柄; Merge Gate(合入门禁)APPROVE-READY ✅(阻断级别:Critical / High / Medium;Low 不阻断;Lifecycle 任一 FAIL 也阻断) 本轮无 open 阻断项。 总体结论中央 fill-only ✅ Medium/High 及以上问题已全部解决,本轮评审通过(approved)。 Highlights(正向实践)
评审报告详见: |
linrunqi08
left a comment
There was a problem hiding this comment.
APPROVE-READY: no open Critical/High/Medium findings. Low items are follow-ups only.
Generated by LoongSuite-Pilot Code Review Agent
Summary
Compatibility guarantees
Accepted first-stage boundaries
Validation
Fixes #261