Skip to content

feat(db): drop the dead gate_check_mode column - #5446

Merged
JSONbored merged 1 commit into
mainfrom
chore/gatecheckmode-drop-db-column-stage2-6
Jul 12, 2026
Merged

feat(db): drop the dead gate_check_mode column#5446
JSONbored merged 1 commit into
mainfrom
chore/gatecheckmode-drop-db-column-stage2-6

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Stage 2.6 of #5373's staged removal plan.

  • gate_check_mode has been a pure computed read-back of review_check_mode since Reduce redundant config surface: linked-issue knob, gateCheckMode, label fields #4618 (migration 0107 backfilled review_check_mode from it and made it the real publish authority) -- getRepositorySettings/upsertRepositorySettings have re-derived it from review_check_mode on every read and write since then, self-healing any stale stored value. The stored column has carried no independent information for any live row since that migration.
  • migrations/0146_drop_gate_check_mode.sql drops it (SQLite 3.35+/D1 supports DROP COLUMN directly -- same precedent as 0122_drop_private_trust_enabled.sql). Stops writing/reading the physical column in repositories.ts; the in-memory RepositorySettings.gateCheckMode field itself is untouched and keeps being derived from reviewCheckMode on every read -- this is not the full field removal yet (a later Stage and remove the derived-only gateCheckMode field (naming sprawl, #5355 follow-up) #5373 stage).
  • Pre-flight: grepped the full repo for raw SQL against gate_check_mode outside src/db/repositories.ts and found none in production code (also checked scripts/**, review-enrichment/**, self-host observability/Grafana configs -- clean). Two test files needed updates: repository-settings-review-check-mode.test.ts had one test whose whole premise was self-healing over a stale physical column (removed -- there's no column left to drift); queue-5.test.ts had a raw SQL assertion on the dropped column (switched to review_check_mode, the value it was actually trying to verify) plus 43 occurrences of gateCheckMode as pure fixture noise never asserted on elsewhere in that file -- not caught by the earlier Stage and remove the derived-only gateCheckMode field (naming sprawl, #5355 follow-up) #5373 discovery audit, found here while fixing the real column-drop fallout. migration-0107-review-check-mode.test.ts is correctly unaffected: it replays migrations up to a historical cutoff, never applying 0146.
  • Deploy note: this repo has no automated API-deploy workflow -- npm run deploy:api (which applies migrations then deploys the Worker in one invocation) is run manually. No separate atomicity concern beyond that existing, already-used pattern.

Test plan

  • npm run typecheck
  • npm run db:migrations:check (149 migrations, contiguous 0001-0146)
  • npm run db:schema-drift:check (schema.ts matches migrations/)
  • test/unit/repository-settings-review-check-mode.test.ts, migration-0107-review-check-mode.test.ts, queue-5.test.ts, backfill.test.ts, backfill-2.test.ts -- 445/445
  • Repo-wide grep confirms zero remaining raw-SQL references to gate_check_mode outside the intentionally-unaffected historical-replay test

gate_check_mode has been a pure computed read-back of review_check_mode
since #4618 (migrations/0107 backfilled review_check_mode from it and
made it the real publish authority) -- getRepositorySettings and
upsertRepositorySettings have re-derived it from review_check_mode on
every read and write since then, self-healing any stale stored value.
The stored column has carried no independent information for any live
row since that migration.

migrations/0146_drop_gate_check_mode.sql drops it (SQLite 3.35+/D1
supports DROP COLUMN directly, same as the 0122 precedent). Stops
writing/reading the physical column in repositories.ts -- the in-memory
RepositorySettings.gateCheckMode field itself is untouched and keeps
being derived from reviewCheckMode on every read, so this is not yet
the full field removal (tracked as a later #5373 stage).

Pre-flight: grepped the full repo for raw SQL against gate_check_mode
outside src/db/repositories.ts and found none in production code. Two
test files needed updates: repository-settings-review-check-mode.test.ts
had one test whose whole premise was self-healing over a stale physical
column (removed, since there's no column left to drift), and
queue-5.test.ts had a raw SQL assertion on the dropped column (switched
to review_check_mode, the value it was actually trying to verify) plus
43 occurrences of gateCheckMode as pure fixture noise never asserted on
elsewhere in that file -- not caught by the earlier #5373 discovery
audit, found here while fixing the real column-drop fallout.
migration-0107-review-check-mode.test.ts is correctly unaffected: it
replays migrations up to a historical cutoff, never applying 0146.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.46%. Comparing base (e0c1000) to head (a1b79d0).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5446      +/-   ##
==========================================
- Coverage   94.46%   94.46%   -0.01%     
==========================================
  Files         554      554              
  Lines       44438    44437       -1     
  Branches    14663    14662       -1     
==========================================
- Hits        41979    41978       -1     
  Misses       1784     1784              
  Partials      675      675              
Flag Coverage Δ
shard-1 44.10% <ø> (-0.01%) ⬇️
shard-2 34.87% <ø> (-0.01%) ⬇️
shard-3 31.58% <ø> (-0.01%) ⬇️
shard-4 31.40% <ø> (-0.01%) ⬇️
shard-5 33.27% <ø> (-0.01%) ⬇️
shard-6 43.37% <ø> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
src/db/repositories.ts 96.69% <ø> (-0.01%) ⬇️
src/db/schema.ts 73.07% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

loopover-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-12 19:55:59 UTC

5 files · 2 AI reviewers · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This drops the dead `gate_check_mode` column (migration 0146), removes its schema.ts declaration and all read/write handling in repositories.ts, and updates test fixtures accordingly — consistent with the prior #4618 finding that the column was a pure derived read-back with no independent state since migration 0107. The migration uses `ALTER TABLE ... DROP COLUMN`, which is D1/SQLite 3.35+ safe and matches the cited precedent (0122). Test changes look correct: the queue-5.test.ts SQL assertion was properly redirected from the dropped column to `review_check_mode` (the value it was actually verifying), and the removed self-healing test in repository-settings-review-check-mode.test.ts is legitimately obsolete since there's no longer a physical column to drift.

Nits — 5 non-blocking
  • The external brief's note about 'still-running deployments that read the old schema' is a generic DROP COLUMN caveat, not a real issue here — repositories.ts is updated in the same commit to stop reading the column, so there's no code path left referencing it post-deploy; worth confirming deploy ordering (migration before code, or vice versa) doesn't create a brief window, per the PR's own deploy note.
  • repositories.ts is a large file (per the external brief) but this diff only removes lines from it, so the size smell predates this PR and isn't something this diff should be asked to fix.
  • Consider a one-line changelog/migration comment cross-referencing Stage and remove the derived-only gateCheckMode field (naming sprawl, #5355 follow-up) #5373 stage numbering for future stages tracking the same column-removal sequence (already partially done in the migration header).
  • nit: migrations/0146_drop_gate_check_mode.sql:1 carries a long explanatory comment that mostly duplicates the PR description; consider keeping only the invariant and historical migration reference so the migration stays easier to scan.
  • nit: test/unit/queue-5.test.ts:2848 now asserts review_check_mode directly, which is correct, but the surrounding comment still describes the persisted state in gate terms; tightening that wording would make the test intent clearer.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 2 reviewers, synthesized
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 44 registered-repo PR(s), 36 merged, 457 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 457 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: minor — This is a well-scoped, low-risk dead-column cleanup that reduces schema surface area but doesn't change any behavior, matching its stated 'stage 2.6 of a staged removal' intent.
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: not available
  • Official Gittensor activity: 44 PR(s), 457 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.
[BETA] Chat with Gittensory

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

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

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 12, 2026
@JSONbored
JSONbored merged commit 08500b5 into main Jul 12, 2026
18 checks passed
@JSONbored
JSONbored deleted the chore/gatecheckmode-drop-db-column-stage2-6 branch July 12, 2026 20:08
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant