fix: classify runtime checkpoint rounds#2152
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Review: LGTM ✅Well-structured and thorough change that correctly classifies runtime checkpoint rounds as private continuity state. Summary of review:
No blocking issues found. Waiting for CI to pass. |
There was a problem hiding this comment.
Review: Approve ✅
Well-structured change that correctly introduces a semantic distinction between ordinary agent response rounds and runtime-private checkpoint rounds, then enforces that distinction consistently across every operator-visible projection layer.
Correctness: Round purpose determination is correct — snapshotting pending_checkpoint_metadata before response processing ensures classification reflects runtime intent. The last_assistant_round_id guard correctly prevents checkpoint rounds from being recorded as terminal text sources. Max-output recovery correctly excludes checkpoint text from truncated history. The = None initialization for last_assistant_round_id is a latent bug fix.
Test coverage: Comprehensive — unit tests for operator events, presentation reducer, web-GUI reducer; integration test for full turn execution with checkpoint metadata verification. Test budgets refactored from hardcoded to dynamically computed.
Documentation: Three RFCs updated with clear semantic boundary docs.
Nit (non-blocking): The same five checkpoint metadata fields (round_purpose, visibility, checkpoint_request_id, checkpoint_mode, checkpoint_requested_at_round) are repeated across four serde_json::json! blocks in execution.rs. A small helper could reduce this repetition surface.
No correctness, security, or safety issues found.
Holon Run Report
|
Summary
agent_responseorruntime_checkpointand persist checkpoint request metadata in transcript/audit evidenceVerification
cargo fmt --all -- --checkRUSTFLAGS="-D warnings" cargo check --all-targetscargo testnpm test(web-gui/app)npm run typecheck(web-gui/app)