Skip to content

feat(calibration): phase-2 hardening from the production applies — plan-file replay, class policy, rate + statement limits - #8248

Merged
JSONbored merged 1 commit into
mainfrom
feat/calibration-backfill-phase2
Jul 23, 2026
Merged

feat(calibration): phase-2 hardening from the production applies — plan-file replay, class policy, rate + statement limits#8248
JSONbored merged 1 commit into
mainfrom
feat/calibration-backfill-phase2

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 23, 2026

Copy link
Copy Markdown
Owner

What

One production-earned fix (the rest of the phase-2 hardening turned out to already be on main — this PR is now the minimal remaining delta): the first cloud raw-context apply failed at row 37 with SQLITE_TOOBIG.

Why

wrangler d1 execute --command cannot bind parameters, so the wrapper inlines each UPDATE as literal SQL — and quote-doubling can double a 120KB diff past D1's per-statement length limit.

How

The literal (D1 CLI) path caps the diff at 45KB with a self-describing truncation marker appended inside the diff text itself, so any consumer of the fixture sees the truncation inline. The pg driver binds parameters and keeps full diffs — the production Postgres corpus is unaffected (460/460 full diffs verified there; the cloud resume completed 460/460 with this fix, results on #8170).

Advances #8170

@superagent-security

Copy link
Copy Markdown
Contributor

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

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

loopover-orb Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-23 14:42:08 UTC

3 files · 1 AI reviewer · 2 blockers · CI pending · dirty

⏸️ Suggested Action - Manual Review

Review summary
This PR adds a phase-2 GitHub-truth backfill CLI (successors + raw-context passes) plus a pure, well-tested core (matching/patching/report logic) and a plan-file replay mode that lets a second store apply a scan's results without re-spending GitHub API budget. The pure-core logic (windowing, earliest-successor tie-breaking, idempotent patchers, D1 45KB truncation vs pg full-diff) is correct as traced against its callers and is covered by the 11 unit tests; the thin IO wrapper (scripts/backfill-calibration-corpus-phase2.ts) is exempted from the coverage bar per the repo's scripts convention. One real behavioral inconsistency exists between the live dry-run pass and the plan-replay dry-run: they count 'shared_issue_only' matches differently in the patched total.

Nits — 5 non-blocking
  • scripts/backfill-calibration-corpus-phase2.ts:279-283 vs :246-249 — in the live `runSuccessorsPass` dry-run (no `--apply`), a `shared_issue_only` match still increments `report.patched` (falls through to the unconditional dry-run branch), but `runSuccessorsFromPlan` (:238-240) `continue`s past the same class whenever `!args.includeSharedIssueOnly` regardless of `--apply`; the two dry-run reports for the same conceptual scan will disagree on the 'would patch' count for this class — worth aligning so an operator comparing scan vs. replay reports isn't confused.
  • The many magic numbers in scripts/backfill-calibration-corpus-phase2.ts (retry counts, timeouts, byte caps, HTTP status codes) are inline literals without named constants — consider hoisting the repeated ones (403/429/404/410, the 5-attempt retry cap, 45_000/90_000/300_000ms) for readability, mirroring how REQUEST_FLOOR_MS and GITHUB_TIMEOUT_MS are already named.
  • scripts/backfill-calibration-corpus-phase2.ts is 489 lines, over the repo's informal 400-line file-size guideline — the GitHub IO block (githubFetch/pace/githubJson/fetchPullFiles/fetchMergedSuccessors) could split out to its own module the way pg-cli.js is already separated.
  • The PR description says 'Advances calibration: backfill phase 2 — GitHub-truth reversed labels (retro successor scan) + raw-context re-fetch #8170' rather than closing an issue directly — confirm calibration: backfill phase 2 — GitHub-truth reversed labels (retro successor scan) + raw-context re-fetch #8170 is the maintainer-authorized issue this hardening work is scoped against, since the repo requires every contributor PR to close or clearly link an eligible issue.
  • Align the shared_issue_only dry-run counting between runSuccessorsPass and runSuccessorsFromPlan (scripts/backfill-calibration-corpus-phase2.ts:279-283, :238-240) so both report the same semantics for 'would patch'.

Concerns raised — review before merging

  • No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked 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: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example `Closes #123`) before opening the PR.

Decision drivers

  • ❌ Code review — 2 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
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: 14 registered-repo PR(s), 14 merged, 255 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 255 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
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, Ruby, Go, MDX, Shell, Solidity, JavaScript
  • Official Gittensor activity: 14 PR(s), 255 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 2 steps in the Signals table above.
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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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

@JSONbored JSONbored self-assigned this Jul 23, 2026
…OOBIG hit live

wrangler --command cannot bind parameters, so the UPDATE inlines the diff
as literal SQL and quote-doubling can double it past D1's per-statement
length (failed live at row 37 of the first cloud apply). The literal path
now caps at 45KB with a self-describing truncation marker inside the diff
text; the pg driver binds parameters and keeps full diffs — production is
unaffected.

Advances #8170
@JSONbored
JSONbored force-pushed the feat/calibration-backfill-phase2 branch from 2d2dcb1 to c75fc49 Compare July 23, 2026 14:58
@JSONbored
JSONbored merged commit 95b758c into main Jul 23, 2026
12 checks passed
@JSONbored
JSONbored deleted the feat/calibration-backfill-phase2 branch July 23, 2026 15:13
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.12%. Comparing base (7a8b9b6) to head (c75fc49).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8248   +/-   ##
=======================================
  Coverage   92.12%   92.12%           
=======================================
  Files         783      783           
  Lines       78551    78551           
  Branches    23723    23724    +1     
=======================================
  Hits        72367    72367           
  Misses       5062     5062           
  Partials     1122     1122           
Flag Coverage Δ
shard-1 56.52% <ø> (ø)
shard-2 53.99% <ø> (ø)
shard-3 49.94% <ø> (-0.01%) ⬇️

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

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.

1 participant