Skip to content

fix(db): add $defaultFn(nowIso) to upstreamDriftReports.updatedAt#8404

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
claytonlin1110:fix/upstream-drift-updatedAt-defaultFn-8369
Jul 24, 2026
Merged

fix(db): add $defaultFn(nowIso) to upstreamDriftReports.updatedAt#8404
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
claytonlin1110:fix/upstream-drift-updatedAt-defaultFn-8369

Conversation

@claytonlin1110

Copy link
Copy Markdown
Contributor

What

upstreamDriftReports.updatedAt was the only updatedAt in src/db/schema.ts missing .$defaultFn(() => nowIso()), despite the file header's house rule (and ~30 sibling columns that already follow it). Current writers always pass updatedAt explicitly, so nothing breaks today — but any future insert that omits the column would hit the same client-side corruption class the header documents.

Changes

  • Added .$defaultFn(() => nowIso()) to upstreamDriftReports.updatedAt only — generatedAt and repository writers left untouched.
  • Regression in test/unit/schema-timestamp-defaults.test.ts: drizzle insert that omits updatedAt gets a real ISO-8601 timestamp (not CURRENT_TIMESTAMP / empty).

Closes

Closes #8369

Verification

  • npx vitest run test/unit/schema-timestamp-defaults.test.ts — 7 passed (including the new omit-path case)

…ONbored#8369)

Match the schema house rule every sibling updatedAt already follows so an
insert that omits the column gets a real ISO-8601 timestamp. Existing
writers keep their explicit updatedAt writes; this is only the schema-level
safety net. Regression covers the omit path in schema-timestamp-defaults.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.45%. Comparing base (af270b9) to head (4f7031c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8404      +/-   ##
==========================================
- Coverage   92.42%   89.45%   -2.97%     
==========================================
  Files         791       98     -693     
  Lines       79250    22892   -56358     
  Branches    23946     3872   -20074     
==========================================
- Hits        73243    20477   -52766     
+ Misses       4868     2237    -2631     
+ Partials     1139      178     -961     
Flag Coverage Δ
shard-1 66.12% <0.00%> (+9.76%) ⬆️
shard-2 66.66% <0.00%> (+13.58%) ⬆️
shard-3 70.43% <100.00%> (+17.38%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/db/schema.ts 73.11% <100.00%> (+0.14%) ⬆️

... and 693 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 24, 2026
@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-24 12:04:52 UTC

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Small, correct schema fix: adds `.$defaultFn(() => nowIso())` to `upstreamDriftReports.updatedAt`, matching the house rule already applied to ~30 sibling timestamp columns in this file, and closes the last gap referenced by issue #8369. The accompanying test correctly inserts via drizzle omitting `updatedAt` and asserts a real ISO-8601 value (not `CURRENT_TIMESTAMP` or empty), which is the right regression shape for this class of bug. No migration is needed since this changes only client-side insert behavior, not the column's type/nullability/default at the SQL level.

Nits — 3 non-blocking
  • No migration file is added, which is correct here since `$defaultFn` is a drizzle-side JS default (no SQL schema change) — worth a one-line note in the PR description for future reviewers who reflexively look for a migration on any `schema.ts` diff.
  • The upstreamDriftReports repository writer (`upsertUpstreamDriftReport` per test/unit/db-persistence.test.ts) still always passes `updatedAt` explicitly, so this defaultFn is currently dead code on all real write paths — fine per the PR's own framing as forward-looking hardening, but worth confirming no other in-repo caller already relies on omission behaving differently.
  • Consider a short follow-up sweep confirming `generatedAt`-style non-updatedAt timestamp columns across the file don't have similar gaps, since the PR explicitly scoped itself to only `updatedAt`.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8369
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: 394 registered-repo PR(s), 251 merged, 92 issue(s).
Contributor context ✅ Confirmed Gittensor contributor claytonlin1110; Gittensor profile; 394 PR(s), 92 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff adds `.$defaultFn(() => nowIso())` to `upstreamDriftReports.updatedAt` matching the sibling pattern, leaves `generatedAt` and the repository writers untouched, and includes a regression test that inserts a row omitting `updatedAt` and asserts a real ISO-8601 timestamp is produced.

Review context
  • Author: claytonlin1110
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, JavaScript, TypeScript, Rust
  • Official Gittensor activity: 394 PR(s), 92 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 8a6632b into JSONbored:main Jul 24, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upstreamDriftReports.updatedAt is missing $defaultFn, unlike ~30 sibling updatedAt columns

1 participant