Skip to content

fix(db): drop the stale migration-90 grandfather entry (its second file was renumbered to 0092) - #9093

Closed
kai392 wants to merge 1 commit into
JSONbored:mainfrom
kai392:fix/critical-issue-migration90-grandfather
Closed

fix(db): drop the stale migration-90 grandfather entry (its second file was renumbered to 0092)#9093
kai392 wants to merge 1 commit into
JSONbored:mainfrom
kai392:fix/critical-issue-migration90-grandfather

Conversation

@kai392

@kai392 kai392 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

KNOWN_MIGRATION_DUPLICATES's [90, {0090_contributor_cap_label.sql, 0090_pull_request_detail_sync_head_sha.sql}]
entry is stale: 0090_pull_request_detail_sync_head_sha.sql was later renumbered to migrations/0092_*,
so only 0090_contributor_cap_label.sql exists at 0090 today. detectMigrationCollisions only consults
the list when a number has >1 file, so this changes no behavior — but it's factually wrong about what's
grandfathered.

  • Removes the [90, …] entry (check-migrations.ts imports this same constant, so both stay in lockstep).
  • Updates the two tests that pin the grandfather set: migration-collisions.test.ts (keys [15,17,74,156],
    plus a new assertion that 90 is no longer grandfathered and 0090's single file is not a collision) and
    check-migrations-script.test.ts (the success summary now reports 4 grandfathered duplicates: 0015, 0017, 0074, 0156).

The four remaining entries (15, 17, 74, 156) all still reference real duplicate files at those numbers.

Test plan

  • npm run db:migrations:check186 migrations OK — contiguous 0001..0182 (4 grandfathered duplicates: 0015, 0017, 0074, 0156)
  • npx vitest run test/unit/migration-collisions.test.ts — 15/15 pass; tsc --noEmit clean
  • Verified no other test/script references the old count/list (grep repo-wide)

Closes #8897

@kai392
kai392 requested a review from JSONbored as a code owner July 26, 2026 16:57
@superagent-security

Copy link
Copy Markdown
Contributor

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

…le was renumbered to 0092)

KNOWN_MIGRATION_DUPLICATES's [90, {0090_contributor_cap_label.sql, 0090_pull_request_detail_sync_head_sha.sql}]
entry is stale: 0090_pull_request_detail_sync_head_sha.sql was later renumbered to 0092, so only
0090_contributor_cap_label.sql exists at 0090 today. detectMigrationCollisions only consults the list when a
number has >1 file, so this changes no behavior — but it's factually wrong about what's grandfathered. Remove
the entry (check-migrations.ts imports this same constant, so both stay in lockstep) and update the pinning
test; the four remaining entries (15, 17, 74, 156) all still reference real duplicate files.

Closes JSONbored#8897

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 26, 2026
@loopover-orb

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-26 17:05:08 UTC

3 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This PR removes the stale [90,...] grandfather entry from KNOWN_MIGRATION_DUPLICATES on the premise that 0090_pull_request_detail_sync_head_sha.sql was already renumbered to 0092 in a prior change, and updates the two tests that pin the exact grandfather set/count accordingly. The unit-test logic (migration-collisions.test.ts, check-migrations-script.test.ts) is internally consistent with that premise, but the diff contains no migrations/ directory change, so the actual renumbering on disk cannot be verified from what's shown here — and that gap plausibly explains the FAILED validate-tests check.

Nits — 5 non-blocking
  • scripts/check-migrations.ts's own header comment ('0090 — both 0090_contributor_cap_label (feat(agent-actions): auto-close a contributor's PR over the open-PR cap (#2270) #2479) and 0090_pull_request_detail_sync_head_sha (fix(selfhost): cap historical PR file hydration and cache by head SHA #2527) merged... Preserve both filenames') is now stale relative to the removed entry it describes and should be updated in the same PR to avoid confusing the next contributor who reads it as justification to re-add the entry.
  • The new inline comment in migration-collisions.ts documents the renumbering rationale well, but doesn't cross-reference the (now-stale) prose in scripts/check-migrations.ts, so the two files can drift in their narrative even though the data structure stays in lockstep.
  • If migrations/0090_pull_request_detail_sync_head_sha.sql has NOT actually been renamed to 0092 on disk (no migrations/ file change appears in this diff), then check-migrations-script.test.ts's 'reports every grandfathered duplicate...' test — which runs scripts/check-migrations.ts against the REAL migrations/ directory, not a fixture — would hit two real files at 0090 with no grandfather entry to cover them, causing detectMigrationCollisions (src/db/migration-collisions.ts) to flag a genuine unresolved collision, the script to exit 1, and execFileSync to throw; this is a strong candidate for why the 'validate-tests' check is FAILED with no detail provided. (demoted: CI state is decided deterministically, not by review)
  • Confirm (e.g. via `ls migrations/0090_*` and `migrations/0092_*`) that the renumbering actually landed on disk before merging, since check-migrations-script.test.ts's first test reads the real migrations/ directory and would surface a real collision if it hasn't.
  • Update the 0090 bullet in scripts/check-migrations.ts's header comment to reflect the new single-file state, matching the comment already added in migration-collisions.ts.

CI checks failing

  • validate
  • validate-tests

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 #8897
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: 127 registered-repo PR(s), 68 merged, 6 issue(s).
Contributor context ✅ Confirmed Gittensor contributor kai392; Gittensor profile; 127 PR(s), 6 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The stale [90, ...] entry is removed from KNOWN_MIGRATION_DUPLICATES, and both the unit test and the check-migrations script test are updated with a new assertion confirming migration 90's single real file produces no collision and 90 is no longer grandfathered.

Review context
  • Author: kai392
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, Cuda, JavaScript, Kotlin, Perl, TypeScript, Vue
  • Official Gittensor activity: 127 PR(s), 6 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 26, 2026
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.

fix(db): stale KNOWN_MIGRATION_DUPLICATES entry for migration 90 references a file that no longer exists

2 participants