You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Keep soft-delete and real user-facing edits (comment, checksum, deleted, etc.)
One-shot DELETE of matching historical bookkeeping rows in the migration
Add coverage in tests/cleanup_swap_memory.test.ts
Motivation (AI generated)
Capgo-EU audit_logs toast was dominating Postgres shared_buffers (~63% / ~1.3 GB). Live breakdown showed most of the fat was internal pipeline noise (r2_path, storage_provider, dual-storage manifest/manifest_count updates), not console user actions. Storing that TOAST for ~800 active users is pure waste and thrash.
Business Impact (AI generated)
Reduces primary DB memory/disk pressure without changing customer-visible audit for deletes and intentional version edits. Improves stability of Capgo-EU under current compute size.
Test Plan (AI generated)
Migration applies cleanly on local Supabase
bun run supabase:with-env -- bunx vitest run tests/cleanup_swap_memory.test.ts
Confirm soft-delete of a bundle still creates an audit row
Confirm r2_path / storage_provider / dual-storage finalize UPDATEs do not create audit rows
Confirm comment/checksum updates still create audit rows
Your recent review volume is higher than typical usage, so adaptive limits are currently applied.
Next review available in:12 minutes
Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).
How can I continue?
After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.
How do review limits work?
CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.
For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.
The trigger resolves actors, computes changed fields, filters refresh and bookkeeping updates, strips large app-version fields, routes audit records, and returns the affected row.
Cap-go/capgo.app#2721: Both changes modify public.audit_log_trigger() for app-version bookkeeping and fat-field audit behavior, with related test coverage.
Suggested labels:codex
Suggested reviewers:dalanir
Sequence Diagram(s)
sequenceDiagram
participant app_versions
participant audit_log_trigger
participant audit_logs
app_versions->>audit_log_trigger: Update version row
audit_log_trigger->>audit_log_trigger: Resolve actor and compare fields
audit_log_trigger->>audit_log_trigger: Skip bookkeeping-only changes or slim payload
audit_log_trigger->>audit_logs: Insert row for auditable changes
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. Human review is needed for this audit_logs migration change. Reviewers assigned.
Sent by Cursor Approval Agent: Pull Request Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: high. Not approving: Cursor Bugbot skipped (usage limit reached), so automated review did not complete. This migration also changes audit trigger behavior and deletes historical audit_logs rows, which exceeds the low-risk approval threshold. Human review is needed.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: high. Not approving: Cursor Bugbot skipped (usage limit reached), so automated review did not complete. This migration changes audit trigger behavior and deletes historical audit_logs rows, which exceeds the low-risk approval threshold. Human review is needed; reviewers are already assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. Human review is needed for this audit_logs migration; Dalanir and WcaleNieWolny are already assigned.
Sent by Cursor Approval Agent: Pull Request Approver
The reason will be displayed to describe this comment to others. Learn more.
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 `@supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql`:
- Line 4: Update the historical-cleanup comment in the migration to comply with
SQLFluff’s line-length limit by wrapping the text across additional comment
lines, while preserving its meaning.
🪄 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: 01696ae2-5385-4d19-8ba2-ec852d815c50
📥 Commits
Reviewing files that changed from the base of the PR and between d3c4343 and 8f01f47.
Avoid NEW.column references that break the shared audit trigger on
apps/orgs/channels during migration apply.
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. Human review is needed for this audit_logs migration; Dalanir and WcaleNieWolny are already assigned.
Sent by Cursor Approval Agent: Pull Request Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. This migration changes audit_log_trigger behavior, which exceeds the low-risk approval threshold; Dalanir and WcaleNieWolny are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
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 `@supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql`:
- Around line 75-90: Remove native_packages from the excluded-field lists in the
comparison between v_old_record and v_new_record, while continuing to exclude
the other bookkeeping fields. Preserve any later logic that removes
native_packages from the persisted audit JSON.
🪄 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: 1e8bb0b6-8f2c-48e3-9a79-90cd259f5137
📥 Commits
Reviewing files that changed from the base of the PR and between 8f01f47 and fa0463c.
Do not treat native_packages-only updates as bookkeeping skips; still
strip that fat column from persisted audit JSON.
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. Human review is needed for this audit_logs migration; Dalanir and WcaleNieWolny are already assigned.
Sent by Cursor Approval Agent: Pull Request Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. This migration changes audit_log_trigger behavior, which exceeds the low-risk approval threshold; Dalanir and WcaleNieWolny are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 1 file (changes from recent commits).
Confidence score: 3/5
In supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, the bookkeeping-only fast path still serializes full manifest/native_packages rows before returning, so the intended optimization may not reduce hot-path TOAST CPU/memory pressure as expected — keep the pre-skip comparison on cheap fields and avoid full-row JSONB materialization before the return.
In supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, the early-return guard always runs to_jsonb(OLD/NEW) even when user-facing changes mean the check fails, which can add avoidable serialization overhead on normal updates and blunt performance gains — restructure the guard to use lightweight comparisons first and only serialize when actually needed for auditing.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql">
<violation number="1" location="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql:73">
P2: Bookkeeping-only updates still serialize the full `manifest` and `native_packages` rows before returning, so the hot path retains the TOAST CPU/memory work this skip is intended to avoid. Keep the pre-skip comparison column-based (or otherwise exclude fat values before conversion) and only build audit JSON after determining that an audit is needed.</violation>
<violation number="2" location="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql:73">
P2: The early-return check for bookkeeping-only updates unconditionally serializes OLD and NEW to JSONB via `to_jsonb()`. When a user-facing field change (comment, checksum, etc.) causes the check to not match, the function falls through to the generic ELSE branch which serializes both rows to JSONB a second time. Since `manifest` is a custom array type (`manifest_entry[]`) that can hold thousands of entries per AGENTS.md, this double-serialization adds unnecessary CPU/memory overhead to every user-facing UPDATE on `app_versions`.
Consider restructuring the early-return check inside the existing ELSE branch so that the `to_jsonb` call happens exactly once — the serialized value is then reused for both the bookkeeping comparison and the changed_fields walk. This avoids redundant work and recovers the performance characteristic the previous column-comparison approach had for the non-skipped path.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
The reason will be displayed to describe this comment to others. Learn more.
P2: Bookkeeping-only updates still serialize the full manifest and native_packages rows before returning, so the hot path retains the TOAST CPU/memory work this skip is intended to avoid. Keep the pre-skip comparison column-based (or otherwise exclude fat values before conversion) and only build audit JSON after determining that an audit is needed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 73:
<comment>Bookkeeping-only updates still serialize the full `manifest` and `native_packages` rows before returning, so the hot path retains the TOAST CPU/memory work this skip is intended to avoid. Keep the pre-skip comparison column-based (or otherwise exclude fat values before conversion) and only build audit JSON after determining that an audit is needed.</comment>
<file context>
@@ -66,34 +66,29 @@ BEGIN
+ -- changed_fields walk. Compare via to_jsonb only (this trigger is shared across
+ -- tables; never touch NEW.column names that only exist on app_versions).
+ IF TG_OP = 'UPDATE' AND TG_TABLE_NAME = 'app_versions' THEN
+ v_old_record := pg_catalog.to_jsonb(OLD);
+ v_new_record := pg_catalog.to_jsonb(NEW);
+ IF (
</file context>
The reason will be displayed to describe this comment to others. Learn more.
P2: The early-return check for bookkeeping-only updates unconditionally serializes OLD and NEW to JSONB via to_jsonb(). When a user-facing field change (comment, checksum, etc.) causes the check to not match, the function falls through to the generic ELSE branch which serializes both rows to JSONB a second time. Since manifest is a custom array type (manifest_entry[]) that can hold thousands of entries per AGENTS.md, this double-serialization adds unnecessary CPU/memory overhead to every user-facing UPDATE on app_versions.
Consider restructuring the early-return check inside the existing ELSE branch so that the to_jsonb call happens exactly once — the serialized value is then reused for both the bookkeeping comparison and the changed_fields walk. This avoids redundant work and recovers the performance characteristic the previous column-comparison approach had for the non-skipped path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 73:
<comment>The early-return check for bookkeeping-only updates unconditionally serializes OLD and NEW to JSONB via `to_jsonb()`. When a user-facing field change (comment, checksum, etc.) causes the check to not match, the function falls through to the generic ELSE branch which serializes both rows to JSONB a second time. Since `manifest` is a custom array type (`manifest_entry[]`) that can hold thousands of entries per AGENTS.md, this double-serialization adds unnecessary CPU/memory overhead to every user-facing UPDATE on `app_versions`.
Consider restructuring the early-return check inside the existing ELSE branch so that the `to_jsonb` call happens exactly once — the serialized value is then reused for both the bookkeeping comparison and the changed_fields walk. This avoids redundant work and recovers the performance characteristic the previous column-comparison approach had for the non-skipped path.</comment>
<file context>
@@ -66,34 +66,29 @@ BEGIN
+ -- changed_fields walk. Compare via to_jsonb only (this trigger is shared across
+ -- tables; never touch NEW.column names that only exist on app_versions).
+ IF TG_OP = 'UPDATE' AND TG_TABLE_NAME = 'app_versions' THEN
+ v_old_record := pg_catalog.to_jsonb(OLD);
+ v_new_record := pg_catalog.to_jsonb(NEW);
+ IF (
</file context>
The reason will be displayed to describe this comment to others. Learn more.
2 issues found and verified against the latest diff
Confidence score: 3/5
In supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, the current skip logic appears too broad and can suppress audit rows for manifest create/correct operations and native_packages-only edits, which weakens traceability and can hide real change history — narrow the condition to only the non-NULL→NULL manifest transition as noted.
In supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, the SECURITY DEFINER function’s execution rights may remain overly permissive if prior grants persist, creating avoidable exposure on a privileged entry point — explicitly REVOKE from PUBLIC and GRANT EXECUTE only to service_role in this migration.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql">
<violation number="1" location="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql:77">
P2: Manifest creation/correction and native_packages-only edits now disappear from audit history, not just the reclaim write. Restrict the skip to the non-NULL-to-NULL manifest transition and leave native_packages outside the ignored set.
(Based on your team's feedback about preserving native_packages audit field history.) [FEEDBACK_USED]</violation>
<violation number="2" location="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql:203">
P2: Function privileges remain whatever the target database previously had because this migration only changes ownership. Revoke PUBLIC and grant EXECUTE only to service_role so this SECURITY DEFINER entry point has an enforced, self-contained ACL.</violation>
</file>
Reply with feedback, questions, or to request a fix.
The reason will be displayed to describe this comment to others. Learn more.
P2: Function privileges remain whatever the target database previously had because this migration only changes ownership. Revoke PUBLIC and grant EXECUTE only to service_role so this SECURITY DEFINER entry point has an enforced, self-contained ACL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 205:
<comment>Function privileges remain whatever the target database previously had because this migration only changes ownership. Revoke PUBLIC and grant EXECUTE only to service_role so this SECURITY DEFINER entry point has an enforced, self-contained ACL.</comment>
<file context>
@@ -0,0 +1,205 @@
+$$;
+
+
+ALTER FUNCTION "public"."audit_log_trigger"() OWNER TO "postgres";
</file context>
The reason will be displayed to describe this comment to others. Learn more.
P2: Manifest creation/correction and native_packages-only edits now disappear from audit history, not just the reclaim write. Restrict the skip to the non-NULL-to-NULL manifest transition and leave native_packages outside the ignored set.
(Based on your team's feedback about preserving native_packages audit field history.)
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 77:
<comment>Manifest creation/correction and native_packages-only edits now disappear from audit history, not just the reclaim write. Restrict the skip to the non-NULL-to-NULL manifest transition and leave native_packages outside the ignored set.
(Based on your team's feedback about preserving native_packages audit field history.) </comment>
<file context>
@@ -0,0 +1,205 @@
+ v_new_record := pg_catalog.to_jsonb(NEW);
+ IF (
+ v_old_record
+ - 'manifest'
+ - 'native_packages'
+ - 'updated_at'
</file context>
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. This migration changes audit_log_trigger behavior, which exceeds the low-risk approval threshold; Dalanir and WcaleNieWolny are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Not approving: Cursor Bugbot was present but skipped (usage limit reached), so automated review did not complete. Human review is needed for this audit_logs migration; Dalanir and WcaleNieWolny are already assigned.
Sent by Cursor Approval Agent: Pull Request Approver
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files
Confidence score: 4/5
In supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, the migration only stops future bookkeeping audit writes and leaves existing rows intact, so the historical TOAST/storage bloat called out in the PR remains and expected space/perf gains may not materialize; add the promised historical cleanup/backfill step in this migration (or a paired follow-up migration).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql">
<violation number="1" location="supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql:4">
P2: Existing bookkeeping audit rows are left untouched, so this migration only prevents future writes and does not remove the historical TOAST footprint described in the PR. The migration should include the promised historical cleanup, or ship an explicit, bounded rollout step that is guaranteed to run alongside it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
The reason will be displayed to describe this comment to others. Learn more.
P2: Existing bookkeeping audit rows are left untouched, so this migration only prevents future writes and does not remove the historical TOAST footprint described in the PR. The migration should include the promised historical cleanup, or ship an explicit, bounded rollout step that is guaranteed to run alongside it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 4:
<comment>Existing bookkeeping audit rows are left untouched, so this migration only prevents future writes and does not remove the historical TOAST footprint described in the PR. The migration should include the promised historical cleanup, or ship an explicit, bounded rollout step that is guaranteed to run alongside it.</comment>
<file context>
@@ -0,0 +1,203 @@
+-- Slim audit_logs: do not store internal app_versions upload/migrate bookkeeping.
+-- Capgo-EU evidence: r2_path/storage_provider/manifest pipeline updates were hundreds
+-- of MB of TOAST with no user-facing audit value. Soft-delete and real edits stay.
+-- Historical cleanup stays out of this migration to avoid a WAL/lock storm.
+-- Run bounded ops deletes separately if needed.
+
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary (AI generated)
audit_log_triggersoapp_versionsUPDATEs that only touch upload/migrate bookkeeping fields are not written toaudit_logsmanifest,manifest_count,storage_provider,r2_path,updated_atcomment,checksum,deleted, etc.)tests/cleanup_swap_memory.test.tsMotivation (AI generated)
Capgo-EU
audit_logstoast was dominating Postgresshared_buffers(~63% / ~1.3 GB). Live breakdown showed most of the fat was internal pipeline noise (r2_path,storage_provider, dual-storagemanifest/manifest_countupdates), not console user actions. Storing that TOAST for ~800 active users is pure waste and thrash.Business Impact (AI generated)
Reduces primary DB memory/disk pressure without changing customer-visible audit for deletes and intentional version edits. Improves stability of Capgo-EU under current compute size.
Test Plan (AI generated)
bun run supabase:with-env -- bunx vitest run tests/cleanup_swap_memory.test.tsr2_path/storage_provider/ dual-storage finalize UPDATEs do not create audit rowsGenerated with AI
Made with Cursor
Summary by CodeRabbit
Bug Fixes
Tests