Post commit hook impl#36
Conversation
Adds a new post_commit_patch_intersections table and insert capability to the agent trace Turso database. The table stores commit-level patch intersection results including the commit ID, timing window metrics, trace counts, and the intersection patch payload. This enables capturing the diff-trace intersection results that occur after each commit for later analysis of agent behavior around commit boundaries. Co-authored-by: SCE <sce@crocoder.dev>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds DB persistence and query support for post-commit patch intersections, a new DB table and index, helpers to map query rows, new agent-trace APIs to select recent diff-trace patches and insert intersection results, and a post-commit hook flow that captures git HEAD, combines/intersects recent patches, and persists the intersection. ChangesPost-Commit Patch Intersection Persistence
Sequence DiagramsequenceDiagram
participant Hook as Post-Commit Hook
participant Git as Git Repository
participant DB as AgentTraceDb
participant Patch as Patch Ops
Hook->>Git: read HEAD OID, commit time, patch text
Git-->>Hook: commit metadata + patch
Hook->>Patch: parse current patch
Patch-->>Hook: ParsedPatch (current)
Hook->>DB: query recent diff traces (time_ms window)
DB-->>Hook: RecentDiffTracePatches (loaded + skipped)
Hook->>Patch: combine loaded recent patches
Patch-->>Hook: CombinedParsedPatch
Hook->>Patch: intersect CombinedParsedPatch ∩ CurrentParsedPatch
Patch-->>Hook: IntersectionParsedPatch
Hook->>Patch: serialize intersection to JSON
Patch-->>Hook: intersection_payload
Hook->>DB: insert post_commit_patch_intersection(row)
DB-->>Hook: stored
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 155-157: The recent_diff_trace_patches API currently only filters
rows with time_ms >= cutoff_time_ms and thus includes any future-dated rows;
update the function signature (recent_diff_trace_patches and the helper
recent_diff_trace_patches_with) to accept an additional end_time_ms
(recent_window_end_ms) and change the DB query to enforce both bounds time_ms >=
cutoff_time_ms AND time_ms <= end_time_ms; also propagate the new end_time_ms
parameter from the caller (post-commit flow that records recent_window_end_ms)
so the query correctly restricts the "recent" window on both ends.
- Around line 319-362: The test
recent_diff_trace_patches_filters_by_cutoff_and_orders_chronologically currently
uses distinct time_ms values; add a same-timestamp ordering case by inserting
two DiffTraceInsert rows with identical time_ms (e.g., 1000) but different
session_id values and relying on insert_diff_trace_with to create distinct ids,
then call recent_diff_trace_patches_with(db, 1000) and assert the returned
result.patches order is deterministic (matches the id tie-breaker ordering used
by the query, e.g., session_ids in ascending id order); place the new assertions
alongside the existing ones in that test so equal-timestamp ordering is
validated.
In `@cli/src/services/db/mod.rs`:
- Around line 250-255: The row-mapper errors currently lose the SQL/DB context;
change the call in the loop so failures from map_row preserve context by using
anyhow::Context (e.g., call map_row(&row).with_context(...) and include
M::db_name() and the sql string in the message) before the ? so any mapping
error is annotated with "row mapping failed" plus the SQL and DB name; locate
this around the rows.next() loop where results.push(map_row(&row)?) is invoked
and update that expression to add the context.
In `@cli/src/services/hooks/mod.rs`:
- Around line 810-823: The function capture_head_timestamp_from_git currently
reads the git author timestamp (%at) in seconds and returns it directly; change
it to use the committer timestamp specifier "%ct", parse the seconds, convert to
milliseconds using a checked multiplication (e.g.,
seconds.checked_mul(1000).ok_or_else(...) ) to prevent overflow, and return that
i64 milliseconds value; keep the same error path using post_commit_patch_error
for parse/overflow errors so callers expecting commit_time_ms receive a
correctly scaled millisecond timestamp.
🪄 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: bc353e02-27ef-416e-943c-b24e0dfd65fc
⛔ Files ignored due to path filters (11)
context/architecture.mdis excluded by!context/**/*.mdcontext/cli/patch-service.mdis excluded by!context/**/*.mdcontext/context-map.mdis excluded by!context/**/*.mdcontext/glossary.mdis excluded by!context/**/*.mdcontext/overview.mdis excluded by!context/**/*.mdcontext/patterns.mdis excluded by!context/**/*.mdcontext/plans/post_commit_patch_intersection_db.mdis excluded by!context/**/*.mdcontext/sce/agent-trace-db.mdis excluded by!context/**/*.mdcontext/sce/agent-trace-hooks-command-routing.mdis excluded by!context/**/*.mdcontext/sce/agent-trace-post-commit-dual-write.mdis excluded by!context/**/*.mdcontext/sce/shared-turso-db.mdis excluded by!context/**/*.md
📒 Files selected for processing (6)
cli/migrations/agent-trace/002_create_post_commit_patch_intersections.sqlcli/migrations/agent-trace/003_add_diff_traces_time_ms_id_index.sqlcli/src/services/agent_trace_db/mod.rscli/src/services/db/mod.rscli/src/services/hooks/command.rscli/src/services/hooks/mod.rs
💤 Files with no reviewable changes (1)
- cli/src/services/hooks/command.rs
Adds TursoDb::query_map for row-mapping queries, and AgentTraceDb::recent_diff_trace_patches(cutoff_time_ms) for chronological diff_traces reads with patch parsing and malformed-row skip accounting. Co-authored-by: SCE <sce@crocoder.dev>
Co-authored-by: SCE <sce@crocoder.dev>
…nd intersect Replace the no-op post-commit entrypoint with an active intersection flow that captures the current commit patch via git, queries recent diff_traces from the past 7 days, combines valid patches, intersects the combined result with the post-commit patch, and persists the serialized intersection to the post_commit_patch_intersections table. Empty recent patch sets produce a deterministic empty intersection instead of crashing. Co-authored-by: SCE <sce@crocoder.dev>
…nctions Co-authored-by: SCE <sce@crocoder.dev>
The post-commit intersection query filters by time_ms and sorts by (time_ms, id). Without this composite index, SQLite performs a full table scan and an extra sort pass. The new index covers both the range filter and the sort order in a single pass. Co-authored-by: SCE <sce@crocoder.dev>
649ef0f to
811bc9a
Compare
Convert Git commit timestamps from seconds to milliseconds and bound recent diff trace queries to the hook execution time so post-commit intersection uses the intended inclusive window. Also improves DB row-mapping error context and simplifies nearby hook comments. Co-authored-by: SCE <sce@crocoder.dev>
Add DB-backed regression coverage for inclusive recent diff-trace query bounds, deterministic time/id ordering, raw patch parsing, and malformed-row skip accounting. Co-authored-by: SCE <sce@crocoder.dev>
Add a focused injectable seam around the post-commit intersection flow so the query window, persisted window metadata, skipped/loaded counts, and intersection output can be asserted deterministically. Co-authored-by: SCE <sce@crocoder.dev>
Align Agent Trace DB and patch-service docs with the inclusive recent diff-trace query window and raw patch parsing behavior. Update the migration query comment to show the upper time bound used by the runtime. Co-authored-by: SCE <sce@crocoder.dev>
ae13f29 to
65d1aa8
Compare
Record applied Turso migration IDs per database so setup can skip completed migrations while bringing metadata-less databases forward with the current idempotent migration set. Plan: one_off_agent_trace_db_migration_fix Task: one-off approved task (no T0X task ID) Co-authored-by: SCE <sce@crocoder.dev>
Summary by CodeRabbit
New Features
Performance
Reliability