fix(db): skip updated_at and stats bookkeeping in audit_logs - #2905
Conversation
Stop writing audit rows for pure updated_at bumps and apps/orgs stats refresh fields, and delete matching historical noise in bounded batches. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1adafaa6-179f-4d2d-979e-bfaed38edb24) |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe migration updates ChangesAudit log filtering and cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MutatingStatement
participant audit_log_trigger
participant AuthContext
participant audit_logs
participant cleanup_scheduler
MutatingStatement->>audit_log_trigger: invoke row trigger
audit_log_trigger->>AuthContext: resolve user or API key
audit_log_trigger->>audit_log_trigger: evaluate changed fields
audit_log_trigger->>audit_logs: insert qualifying audit event
cleanup_scheduler->>audit_logs: delete historical bookkeeping rows
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/20260807093220_skip_updated_at_audit_noise.sql`:
- Around line 216-270: Replace the unbounded transactional cleanup loop in the
migration with only a strict, small deployment-budget cleanup. Move the
remaining audit-log processing into process_all_cron_tasks, register the task
through a follow-up migration rather than creating a new cron job, and enforce a
per-run batch limit so each invocation performs bounded work.
- Around line 153-172: The audit_log_trigger() table-mapping CASE must branch on
TG_OP before accessing trigger records, using only OLD for DELETE and NEW/OLD
handling for other operations; return OLD directly on DELETE instead of
COALESCE(NEW, OLD). Add DELETE assertions covering orgs, apps, channels,
app_versions, and org_users, including the currently untested apps and channels
paths.
- Around line 5-208: Make the identifier-resolution CASE in audit_log_trigger
DELETE-safe by using OLD-only values for deletes or returning before the block;
do not dereference NEW on DELETE. Add the required audit-path and cleanup
analysis documenting execution frequency, roles, cardinalities, indexes, and
worst-case EXPLAIN (ANALYZE, BUFFERS) plans. Replace the 1,000-row migration
loop with the existing resumable cleanup mechanism if total WAL or lock duration
must be bounded.
In `@tests/audit-logs.test.ts`:
- Around line 364-422: The organization stats_updated_at-only and
updated_at-only tests currently modify shared ORG_ID and compare aggregate
counts, making them vulnerable to parallel interference. Create a uniquely named
organization fixture for this test scope, use its ID in both tests, and verify
the row exists before recording each baseline audit-log count.
- Around line 364-454: Extend the audit-log coverage alongside the existing
stats_updated_at-only tests by adding organization and app UPDATE cases for
stats_refresh_requested_at. Assert that changing only this field creates no
UPDATE audit log, and include updated_at in the update payload and expected
behavior when that matches the production update shape. Reuse the existing
audit_logs queries and identifiers from the organization and app test cases.
🪄 Autofix
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: d548c999-c6c0-4269-a114-0c9436f4b492
📒 Files selected for processing (2)
supabase/migrations/20260807093220_skip_updated_at_audit_noise.sqltests/audit-logs.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| DO $$ | ||
| DECLARE | ||
| batch_no integer := 0; | ||
| deleted_batch integer; | ||
| deleted_total bigint := 0; | ||
| v_batch_size integer := 1000; | ||
| v_max_batches integer := 1000000; | ||
| BEGIN | ||
| PERFORM pg_catalog.set_config('statement_timeout', '0', true); | ||
|
|
||
| LOOP | ||
| batch_no := batch_no + 1; | ||
| EXIT WHEN batch_no > v_max_batches; | ||
|
|
||
| DELETE FROM public.audit_logs | ||
| WHERE ctid IN ( | ||
| SELECT al.ctid | ||
| FROM public.audit_logs AS al | ||
| WHERE al.operation = 'UPDATE' | ||
| AND al.changed_fields IS NOT NULL | ||
| AND ( | ||
| -- Pure updated_at-only rows on any audited table | ||
| NOT EXISTS ( | ||
| SELECT 1 | ||
| FROM pg_catalog.unnest(al.changed_fields) AS changed_field(field_name) | ||
| WHERE changed_field.field_name IS DISTINCT FROM 'updated_at' | ||
| ) | ||
| OR ( | ||
| -- Stats refresh bookkeeping on apps/orgs | ||
| al.table_name = ANY (ARRAY['apps', 'orgs']::text[]) | ||
| AND al.changed_fields && ARRAY['stats_refresh_requested_at', 'stats_updated_at']::text[] | ||
| AND NOT EXISTS ( | ||
| SELECT 1 | ||
| FROM pg_catalog.unnest(al.changed_fields) AS changed_field(field_name) | ||
| WHERE changed_field.field_name <> ALL ( | ||
| ARRAY['stats_refresh_requested_at', 'stats_updated_at', 'updated_at']::text[] | ||
| ) | ||
| ) | ||
| ) | ||
| ) | ||
| ORDER BY al.created_at | ||
| LIMIT v_batch_size | ||
| ); | ||
|
|
||
| GET DIAGNOSTICS deleted_batch = ROW_COUNT; | ||
| deleted_total := deleted_total + deleted_batch; | ||
| EXIT WHEN deleted_batch = 0; | ||
| END LOOP; | ||
|
|
||
| RAISE NOTICE | ||
| 'skip_updated_at_audit_noise: deleted=% batches=% batch_size=%', | ||
| deleted_total, | ||
| LEAST(batch_no, v_max_batches), | ||
| v_batch_size; | ||
| END; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Move the full cleanup out of the migration transaction.
Each DELETE is limited to 1,000 rows, but every loop iteration remains in the same migration transaction. The loop can delete up to 1,000,000,000 rows with statement_timeout disabled. This does not bound total WAL, lock lifetime, rollback cost, or replication impact.
Keep only a strict deployment budget in this migration. Process the remaining rows through process_all_cron_tasks, registered by a migration, with a per-run batch limit.
Based on learnings, Supabase migrations run inside a transaction. As per coding guidelines, “Do not create new cron jobs; add required work to process_all_cron_tasks through a new migration.”
🤖 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 `@supabase/migrations/20260807093220_skip_updated_at_audit_noise.sql` around
lines 216 - 270, Replace the unbounded transactional cleanup loop in the
migration with only a strict, small deployment-budget cleanup. Move the
remaining audit-log processing into process_all_cron_tasks, register the task
through a follow-up migration rather than creating a new cron job, and enforce a
per-run batch limit so each invocation performs bounded work.
Sources: Coding guidelines, Learnings
Address review: DELETE-safe identifier mapping, cron-drained cleanup with a small migration budget, and isolated bookkeeping skip tests including stats_refresh_requested_at. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_99d13a59-27dc-4709-b1c3-1eb959e6b330) |
|
1 participant
Summary (AI generated)
audit_logswrites when an UPDATE only touchesupdated_aton any audited tablestats_updated_at/stats_refresh_requested_at+updated_at)Motivation (AI generated)
Stats refresh and pure
updated_atbumps were pollutingaudit_logswith bookkeeping noise that has no user-facing audit value and wastes TOAST/WAL on Capgo-EU.Business Impact (AI generated)
Smaller, clearer org audit feeds and less storage/WAL pressure from high-frequency dashboard/stats bookkeeping writes.
Test Plan (AI generated)
bun run supabase:with-env -- bunx vitest run tests/audit-logs.test.ts tests/chart-refresh-rpc.test.tsupdated_at-only / stats-only UPDATE rows inaudit_logsGenerated with AI
Made with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit