feat: Complete post-commit agent trace pipeline#37
Conversation
…sult Move post-commit intersection orchestration to return typed data (PostCommitIntersectionFlowResult) instead of formatted text, and shift command-surface message rendering to run_post_commit_subcommand. This keeps persistence/intersection behavior unchanged while making the flow easier to test and compose through explicit typed outputs. Co-authored-by: SCE <sce@crocoder.dev>
📝 WalkthroughWalkthroughThis PR adds persistent storage of agent traces captured during post-commit hooks: a new SQL migration creates the ChangesAgent Trace Persistence in Post-Commit Hook
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Route post-commit execution through a dedicated agent-trace flow that consumes the typed intersection result, builds Agent Trace metadata from commit-time context, and persists a pretty JSON payload under context/tmp with collision-safe semantics. This keeps the post-commit success contract stable while adding explicit error propagation and test coverage for timestamp validation, persistence failures, and flow wiring behavior. Co-authored-by: SCE <sce@crocoder.dev>
Add an agent_traces migration and typed insert path so built post-commit Agent Trace payloads are stored in the Agent Trace database alongside filesystem artifacts. Update post-commit hook flow orchestration and tests to require successful dual persistence (artifact + DB row), and sync SCE context docs to the new runtime behavior. Co-authored-by: SCE <sce@crocoder.dev>
Keep post-commit Agent Trace persistence DB-only by removing context/tmp artifact writes from the post-commit flow while preserving agent_traces inserts and commit metadata mapping. Update related tests and context contracts to reflect the active runtime behavior (post-commit DB-only, diff-trace still dual-persisted). Co-authored-by: SCE <sce@crocoder.dev>
Co-authored-by: SCE <sce@crocoder.dev>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/src/services/agent_trace_db/mod.rs`:
- Around line 170-176: Add a focused unit test next to insert_agent_trace that
opens the same test database used by other tests, calls insert_agent_trace with
a single AgentTraceInsert (set commit_id, commit_time_ms, and trace_json), then
query agent_traces to fetch that row and assert the stored commit_id,
commit_time_ms, and trace_json match the input; use the insert_agent_trace
method name and the agent_traces table in the test, ensure cleanup or
transaction rollback per existing test patterns, and keep the test deterministic
and minimal to catch SQL binding regressions for INSERT_AGENT_TRACE_SQL.
In `@cli/src/services/hooks/mod.rs`:
- Around line 455-457: The current orchestration calls
run_intersection_flow(...) and then run_agent_trace_flow(...), which can leave
persisted intersection rows if the second call fails; make these two post-commit
writes atomic by performing both writes inside a single transaction (or by
adding a compensating rollback on failure). Concretely, modify
run_intersection_flow and run_agent_trace_flow (or add a new wrapper like
run_intersection_and_agent_trace_transactional) to accept a shared DB
transaction/context, execute the intersection write and the agent-trace write
within that transaction, and only commit after both succeed (or if your DB layer
cannot do that, delete/undo the intersection rows when run_agent_trace_flow
returns an error). Ensure references to run_intersection_flow,
run_agent_trace_flow, result and the final commit/rollback are updated
accordingly.
- Around line 459-462: The log message reports "intersection_files" but uses
result.combined_recent_patch.files.len(); change it to report the real
intersection count (e.g., use the actual intersection collection's length such
as result.post_commit_data.intersection_files.len() or
result.intersection_files.len. wherever the intersection vector/collection is
stored) or else rename the message key to "combined_files" to match
combined_recent_patch.files.len(); update the call that builds the log
(referencing result.post_commit_data.commit_oid and
result.combined_recent_patch.files.len()) so the metric name matches the value
returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a0054fbf-0bb1-49aa-b65a-98dd3055a49b
⛔ Files ignored due to path filters (7)
context/architecture.mdis excluded by!context/**/*.mdcontext/cli/cli-command-surface.mdis excluded by!context/**/*.mdcontext/context-map.mdis excluded by!context/**/*.mdcontext/glossary.mdis excluded by!context/**/*.mdcontext/overview.mdis excluded by!context/**/*.mdcontext/sce/agent-trace-db.mdis excluded by!context/**/*.mdcontext/sce/agent-trace-hooks-command-routing.mdis excluded by!context/**/*.md
📒 Files selected for processing (4)
cli/Cargo.tomlcli/migrations/agent-trace/004_create_agent_traces.sqlcli/src/services/agent_trace_db/mod.rscli/src/services/hooks/mod.rs
| /// Insert a built agent trace payload into the `agent_traces` table. | ||
| pub fn insert_agent_trace(&self, input: AgentTraceInsert<'_>) -> Result<u64> { | ||
| self.execute( | ||
| INSERT_AGENT_TRACE_SQL, | ||
| (input.commit_id, input.commit_time_ms, input.trace_json), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add a focused unit test for insert_agent_trace.
Please add a test that inserts one AgentTraceInsert row and asserts stored commit_id, commit_time_ms, and trace_json in agent_traces. The current migration-only assertions won’t catch insert SQL binding regressions.
As per coding guidelines, "Add unit tests close to the code they exercise."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/src/services/agent_trace_db/mod.rs` around lines 170 - 176, Add a focused
unit test next to insert_agent_trace that opens the same test database used by
other tests, calls insert_agent_trace with a single AgentTraceInsert (set
commit_id, commit_time_ms, and trace_json), then query agent_traces to fetch
that row and assert the stored commit_id, commit_time_ms, and trace_json match
the input; use the insert_agent_trace method name and the agent_traces table in
the test, ensure cleanup or transaction rollback per existing test patterns, and
keep the test deterministic and minimal to catch SQL binding regressions for
INSERT_AGENT_TRACE_SQL.
| let result = run_intersection_flow(repository_root)?; | ||
| let _agent_trace = run_agent_trace_flow(repository_root, &result)?; | ||
|
|
There was a problem hiding this comment.
Prevent partial persistence across the two post-commit write stages.
This orchestration can leave persisted intersection rows without corresponding agent traces when stage 2 fails. Please make the two writes atomic (single transaction or explicit compensating strategy) to avoid inconsistent trace state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/src/services/hooks/mod.rs` around lines 455 - 457, The current
orchestration calls run_intersection_flow(...) and then
run_agent_trace_flow(...), which can leave persisted intersection rows if the
second call fails; make these two post-commit writes atomic by performing both
writes inside a single transaction (or by adding a compensating rollback on
failure). Concretely, modify run_intersection_flow and run_agent_trace_flow (or
add a new wrapper like run_intersection_and_agent_trace_transactional) to accept
a shared DB transaction/context, execute the intersection write and the
agent-trace write within that transaction, and only commit after both succeed
(or if your DB layer cannot do that, delete/undo the intersection rows when
run_agent_trace_flow returns an error). Ensure references to
run_intersection_flow, run_agent_trace_flow, result and the final
commit/rollback are updated accordingly.
| "post-commit hook processed intersection: commit={}, intersection_files={}", | ||
| result.post_commit_data.commit_oid, | ||
| result.combined_recent_patch.files.len() | ||
| )) |
There was a problem hiding this comment.
Fix the reported intersection_files count.
intersection_files currently uses combined_recent_patch.files.len(), which is not the intersection file count. Either compute/report the actual intersection count or rename the field text to match the metric.
Proposed minimal wording fix
- "post-commit hook processed intersection: commit={}, intersection_files={}",
+ "post-commit hook processed intersection: commit={}, combined_recent_files={}",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/src/services/hooks/mod.rs` around lines 459 - 462, The log message
reports "intersection_files" but uses result.combined_recent_patch.files.len();
change it to report the real intersection count (e.g., use the actual
intersection collection's length such as
result.post_commit_data.intersection_files.len() or
result.intersection_files.len. wherever the intersection vector/collection is
stored) or else rename the message key to "combined_files" to match
combined_recent_patch.files.len(); update the call that builds the log
(referencing result.post_commit_data.commit_oid and
result.combined_recent_patch.files.len()) so the metric name matches the value
returned.
Co-authored-by: SCE <sce@crocoder.dev>
Co-authored-by: SCE <sce@crocoder.dev>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/src/services/hooks/mod.rs`:
- Around line 465-486: The post-commit flow in run_post_commit_agent_trace_flow
currently only validates and inserts into AgentTraceDb via
run_post_commit_agent_trace_flow_with and never writes the serialized trace to
disk; update run_post_commit_agent_trace_flow to call
persist_serialized_trace_payload(repository_root, &serialized_payload,
&trace_meta) after successful validation/insert (use the same serialized payload
produced in the closure passed to run_post_commit_agent_trace_flow_with),
ensuring you pass repository_root so the artifact is written; mirror the same
change for the other occurrence noted (lines ~522-533) so both post-commit paths
persist the agent-trace file artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ecde60a9-f7fc-40f3-ae7f-0c5ec549ef68
📒 Files selected for processing (1)
cli/src/services/hooks/mod.rs
| fn run_post_commit_agent_trace_flow( | ||
| _repository_root: &Path, | ||
| flow_result: &PostCommitIntersectionFlowResult, | ||
| ) -> Result<AgentTrace> { | ||
| let db = AgentTraceDb::new().context("Failed to open Agent Trace DB for post-commit trace.")?; | ||
|
|
||
| run_post_commit_agent_trace_flow_with( | ||
| flow_result, | ||
| |trace_value| { | ||
| validate_agent_trace_value(trace_value) | ||
| .map_err(|error| anyhow!(error.to_string())) | ||
| .context("Failed to verify built post-commit Agent Trace payload.")?; | ||
|
|
||
| Ok(()) | ||
| }, | ||
| |insert_input| { | ||
| db.insert_agent_trace(insert_input) | ||
| .context("Failed to persist built post-commit Agent Trace payload.")?; | ||
|
|
||
| Ok(()) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Persist the built post-commit agent trace as a file artifact.
This helper takes repository_root, but the new flow only validates the trace and inserts it into AgentTraceDb. There is no call to persist_serialized_trace_payload(...), so the post-commit path never emits the agent-trace artifact on disk even though this is the integration point for that stage of the pipeline.
Also applies to: 522-533
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/src/services/hooks/mod.rs` around lines 465 - 486, The post-commit flow
in run_post_commit_agent_trace_flow currently only validates and inserts into
AgentTraceDb via run_post_commit_agent_trace_flow_with and never writes the
serialized trace to disk; update run_post_commit_agent_trace_flow to call
persist_serialized_trace_payload(repository_root, &serialized_payload,
&trace_meta) after successful validation/insert (use the same serialized payload
produced in the closure passed to run_post_commit_agent_trace_flow_with),
ensuring you pass repository_root so the artifact is written; mirror the same
change for the other occurrence noted (lines ~522-533) so both post-commit paths
persist the agent-trace file artifact.
Summary
Build the full post-commit agent trace pipeline across six scoped changes:
Changes
agent_tracestable migration — Added004_create_agent_traces.sqlmigration.PostCommitIntersectionFlowResultintobuild_agent_trace()via a dedicatedrun_post_commit_agent_trace_flowhelper.agent_traces(DB).validate_agent_trace_valueinto the post-commit flow as a callable argument, invoked afterbuild_agent_trace()and before DB persistence, reusing the same failure-path as insert errors (cli/src/services/hooks/mod.rs).Summary by CodeRabbit
New Features
Database
Tests
Chores