Skip to content

feat(selfhost): add retention for agent_context_snapshots - #3937

Merged
JSONbored merged 1 commit into
mainfrom
feat/bounded-retention-unbounded-tables
Jul 7, 2026
Merged

feat(selfhost): add retention for agent_context_snapshots#3937
JSONbored merged 1 commit into
mainfrom
feat/bounded-retention-unbounded-tables

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

This narrows significantly from the original issue after implementation research:

  • audit_events, ai_usage_events, product_usage_events already have working, cron-scheduled retention via RETENTION_POLICY in src/db/retention.ts (pruneExpiredRecords, dispatched by the prune-retention job type enqueued daily from src/index.ts). The original audit's grep -rn ".delete(" check missed this because the deletes are dynamic SQL strings (DELETE FROM ${rule.table} WHERE ...), not literal Drizzle .delete(table) calls.
  • webhook_events is deliberately excluded from retention by a prior fix (commit 67c11f2e, "fix(db): preserve webhook replay cache during retention" fix(db): exclude webhook_events from destructive retention policy #666) — GitHub can redeliver a webhook long after the original event, and this table is the idempotency/dedup record that detects that. There's a dedicated regression test (test/unit/retention.test.ts, "the policy only targets append-only/log/snapshot tables") asserting webhook_events is never in RETENTION_POLICY. Adding it back would reintroduce that fixed bug — so I did NOT touch it.
  • agent_context_snapshots (one payloadJson blob per agent run, capped only by a per-runId .limit(50) read, never deleted) was the one table genuinely still unaddressed. Added it to RETENTION_POLICY with a 30-day window (shorter than the audit/usage-log tables since it's a per-run diagnostic blob with no cross-run rollup depending on it), reusing the existing generic prune mechanism — no new code path, no new risk surface.

Posted a correction comment on #3896 before implementing so the tracking issue reflects this.

Found via a fresh performance/scalability/accuracy hardening audit of the self-host ORB stack. Tracked under #1667.

Scope

  • src/db/retention.ts — one new RETENTION_POLICY entry
  • test/unit/retention.test.ts — new test covering the prune behavior for agent_context_snapshots

Validation

  • npm run typecheck
  • npm run test:coverage (full unsharded) — 560 files / 11067 tests passed
  • git diff --check clean

Safety

  • Reuses the existing, already-tested pruneExpiredRecords mechanism verbatim — no new deletion code path. Explicitly did NOT touch webhook_events, avoiding a regression of a previously-fixed bug.

Closes #3896

Every table originally flagged as unbounded in #3896 except one turned
out already handled: audit_events/ai_usage_events/product_usage_events
already have cron-scheduled retention via RETENTION_POLICY (the
original audit's grep for literal .delete() calls missed the dynamic
DELETE FROM ${table} SQL this module uses), and webhook_events is
deliberately excluded per a prior fix (#666) since GitHub can redeliver
a webhook long after the original event and this table is the
idempotency record that detects that -- adding it back would
reintroduce that fixed bug.

agent_context_snapshots (one payloadJson blob per agent run, capped
only by a per-runId .limit(50) READ, never deleted) was genuinely the
only unaddressed table. Add it to RETENTION_POLICY with a 30-day
window, reusing the existing generic prune mechanism.

Closes #3896
@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-07 07:25:24 UTC

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

Review summary
Adds a single new RETENTION_POLICY entry for agent_context_snapshots (30-day window on created_at) plus a direct unit test exercising pruneExpiredRecords against real inserted rows. The change reuses the existing generic prune mechanism with no new code path: retentionWhere() falls through to the default `column < ?1` branch since the table isn't audit_events, the identifier passes SAFE_IDENTIFIER, and batching/cap logic is unchanged. The test inserts real rows and asserts real deletion (not a fabricated scenario), and the PR correctly avoids touching webhook_events, which is deliberately excluded per a prior regression fix already covered by an existing test.

Nits — 5 non-blocking
  • src/db/retention.ts:26 — the external brief flags `30` as an unexplained magic number, but the preceding comment already justifies the shorter window; consistent with how the other windows (90, 180, etc.) are also left as inline literals rather than named constants, so this is a style nit at most.
  • test/unit/retention.test.ts — the new agent_context_snapshots test only checks `results[0]?.deleted`/row survival; consider also asserting the returned `table`/`column` fields for symmetry with how other tests in the file check full result shape, though this is minor given the existing pattern is already loose here too.
  • If agent_context_snapshots ever gains a downstream consumer that reads snapshots beyond 30 days (e.g., a future analytics job), revisit the window before merging further readers — not a concern for this PR as scoped.
  • Consider a one-line addition to the RETENTION_POLICY doc comment header explicitly naming agent_context_snapshots alongside the other snapshot tables it already generically covers, for discoverability.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3896
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 343 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 343 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 343 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.66%. Comparing base (9ec0782) to head (f75ff58).
⚠️ Report is 10 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3937   +/-   ##
=======================================
  Coverage   93.66%   93.66%           
=======================================
  Files         372      372           
  Lines       34859    34859           
  Branches    12746    12746           
=======================================
  Hits        32652    32652           
  Misses       1588     1588           
  Partials      619      619           
Files with missing lines Coverage Δ
src/db/retention.ts 96.36% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit a20fbae into main Jul 7, 2026
10 checks passed
@JSONbored
JSONbored deleted the feat/bounded-retention-unbounded-tables branch July 7, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Development

Successfully merging this pull request may close these issues.

feat(selfhost): add bounded retention for webhook_events, audit_events, ai_usage_events, product_usage_events, agent_context_snapshots

1 participant