Skip to content

fix(github): extend repo-rename identity migration to REES/parity tables - #5954

Merged
JSONbored merged 1 commit into
mainfrom
fix/repo-rename-rees-tables
Jul 15, 2026
Merged

fix(github): extend repo-rename identity migration to REES/parity tables#5954
JSONbored merged 1 commit into
mainfrom
fix/repo-rename-rees-tables

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Follow-up to #5918 / #5950 / #5953. Extends renameRepositoryIdentity to review_audit, contributor_gate_history, and submitter_stats — raw-SQL-only REES/parity tables, never added to the Drizzle schema (see each migration's own header comment), so these three blocks use env.DB.prepare() directly instead of the query builder, matching how every real writer (parity-wire.ts, outcomes-wire.ts, contributor-calibration.ts, submitter-reputation.ts) already accesses them.

What changed

  • review_audit, contributor_gate_history: PK id alone, no separate unique index. project IS the repo full name at every live writer; target_id (${project}#${pullNumber}) and id both embed it. Folds on the id collision the rename would produce — same PK-collision-only shape as pull_request_reviews/collision_edges from earlier PRs in this series.
  • submitter_stats: no id column — PRIMARY KEY (project, submitter) directly. Folds on the other half of the composite key, submitter.

Deliberately out of scope

Two other raw-SQL REES tables:

  • review_targets: has no live writer anywhere in this codebase — explicitly confirmed orphaned by src/review/public-stats.ts's own comment ("the legacy review_targets ledger, which the convergence cutover orphaned"). Its project column is an agent/install-level slug that must never be renamed (not a per-repo value at all), and its repo column's real on-disk semantics for that historical, one-time-bulk-copied data can't be independently verified from current code alone — mutating identity columns on data no writer would ever repair carries real corruption risk for uncertain benefit.
  • repo_chunks: a rebuildable RAG chunk/embedding cache (same reasoning fix(github): extend repo-rename identity migration to analytics tables #5953 used to exclude the AI/LLM caches), whose project/repo columns hold the bare owner and bare repo name separately (not a single owner/repo string), and whose id is a lowercased, truncated-to-64-chars hash of the two — a safe in-place string rename isn't mechanically available without risking a silently-corrupted truncated id.

Both documented inline in the code so the omission reads as a deliberate call, not an oversight.

Test plan

  • npm run typecheck — clean
  • npx vitest run test/unit/repo-identity-rename.test.ts — 37/37 passing (7 new: a rename test for all 3 tables, a collision-fold regression test for each, plus an unrelated-row-untouched check for submitter_stats)
  • npm run test:ci — full local gate, green
  • npm audit --audit-level=moderate — 0 vulnerabilities

This is the last PR in the repo-rename-migration series (#5918, #5950, #5953, this one) — together they cover every repo-identity-bearing table in the schema except the deliberately-excluded rebuildable caches noted above.

@superagent-security

Copy link
Copy Markdown
Contributor

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

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

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-15 01:04:13 UTC

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

⏸️ Suggested Action - Manual Review

Review summary
This extends the repo-rename identity migration to three raw-SQL-only REES/parity tables (review_audit, contributor_gate_history, submitter_stats), following the exact same delete-then-update collision-fold pattern already established for pull_request_reviews/collision_edges in prior PRs (#5918/#5950/#5953). The ordering is correct in all three blocks: colliding new-name rows are deleted first (by id for the two id-keyed tables, by project+submitter for the composite-key table), then the UPDATE renames the old rows in place, avoiding PK violations. Tests exercise the happy path, the id/composite-key collision fold, and non-interference with unrelated rows for each table, matching the pattern's prior test shape.

Nits — 3 non-blocking
  • src/db/repo-identity-rename.ts: the JS-side `id.split(oldFullName).join(newFullName)` and the SQL `replace(id, ?, ?)` both replace ALL occurrences of oldFullName in the id — if oldFullName ever appears as a substring of an unrelated part of the id (e.g. embedded in a hash or unrelated token) this would over-rewrite; worth a one-line comment noting this is accepted given the id-construction scheme, similar to comments already given for target_id.
  • The description's 'deliberately out of scope' rationale for review_targets and repo_chunks is thorough but lives only in the PR body/code comments — consider a short doc note (or link) so future contributors extending this migration don't re-litigate the same exclusion decision.
  • Consider factoring the repeated 'select old ids -> compute renamed ids -> delete collisions -> update' block (used near-identically for review_audit and contributor_gate_history) into a small shared helper to reduce duplication, though the current explicitness may be intentional for auditability given the surrounding comments.

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 1 reviewer
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: 43 registered-repo PR(s), 35 merged, 373 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 43 PR(s), 373 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 43 PR(s), 373 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.
  • No action.
  • 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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat <question> 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 15, 2026
@JSONbored
JSONbored force-pushed the fix/repo-rename-rees-tables branch from bc0eb26 to 39f7753 Compare July 15, 2026 00:34
@JSONbored
JSONbored force-pushed the fix/repo-rename-rees-tables branch from 39f7753 to 23a30fe Compare July 15, 2026 00:59
@JSONbored
JSONbored merged commit 00215d8 into main Jul 15, 2026
15 checks passed
@JSONbored
JSONbored deleted the fix/repo-rename-rees-tables branch July 15, 2026 01:50
This was referenced Jul 15, 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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant