Skip to content

fix(contract): restate MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES so the generated schemas compile - #9936

Merged
JSONbored merged 1 commit into
mainfrom
fix/contract-limits-priority-window
Jul 30, 2026
Merged

fix(contract): restate MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES so the generated schemas compile#9936
JSONbored merged 1 commit into
mainfrom
fix/contract-limits-priority-window

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

main is currently failing contract:api-schemas:check, which blocks every PR.

What happened

#9738 added .max(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES) to the settings schema in src/openapi/schemas.ts. gen-contract-api-schemas.ts copies that schema verbatim into @loopover/contract, which cannot import the Worker's src/ — it is a zod-only leaf, and that property is what lets every other surface depend on it. Referenced constants therefore have to be restated in the contract's own limits.ts. That step was missed, leaving both paths broken:

  • Regenerate → the emitted file references a name it never imports → TS2304: Cannot find name 'MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES', contract build fails.
  • Don't regeneratecontract:api-schemas:check stays red.

Verified against a pristine origin/main worktree, so this is not local drift.

The generator's own doc comment calls this "the loud failure this wants" — it worked exactly as designed. It just needs the constant it was asking for.

Also: actually pin the restated bounds

limits.ts says these entries are "pinned against their originals like every other entry here", but PREFLIGHT_LIMITS was the only group with a meta-test doing the pinning — the three single constants were restated on trust, which is the same as not being pinned.

That gap is one-sided and quiet: nothing at compile time relates the two copies, so raising a bound on the Worker side alone leaves the published schema rejecting input the server would now accept (and vice versa), surfacing as a confusing client-side validation error rather than a build failure. Note the compile-time failure this PR fixes only ever catches a missing constant — never a drifted value.

test/unit/contract-limits-pinned.test.ts closes that for all three, one case each so a failure names the specific bound and the file to reconcile it with.

… generated schemas compile

main is currently failing `contract:api-schemas:check` for every PR.

#9738 added `.max(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES)` to the settings
schema in src/openapi/schemas.ts. gen-contract-api-schemas.ts copies that
schema verbatim into @loopover/contract, which cannot import the Worker's
src/ -- so a referenced constant has to be restated in the contract's own
limits.ts. That step was missed, leaving both paths broken: regenerating
emits a file referencing a name it never imports (TS2304, contract build
fails), and NOT regenerating leaves the drift check red.

The generator's doc calls this "the loud failure this wants", and it worked --
it just needs the constant it was asking for.

Also pins the three restated Worker bounds against their originals. limits.ts
says they are "pinned against their originals like every other entry here",
but PREFLIGHT_LIMITS was the only group with a meta-test doing that; the
single constants were restated on trust. Nothing at compile time relates the
two copies (that is the whole reason the contract is a zod-only leaf), so a
one-sided edit would surface as a client-side validation error rather than a
build failure. The compile-time failure only catches a MISSING constant, never
a drifted VALUE.
@loopover-orb

loopover-orb Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-30 17:55:41 UTC

3 files · 1 AI reviewer · 2 blockers · CI green · unknown

⏸️ Suggested Action - Manual Review

Review summary
This PR restates MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES in the contract package's limits.ts and imports it into api-schemas.ts, fixing a broken contract:api-schemas:check build caused by #9738's unrestated constant. It also adds a pinning test (contract-limits-pinned.test.ts) covering all three previously-unpinned single constants (MAX_CONTRIBUTOR_OPEN_ITEM_CAP, MAX_REVIEW_NAG_COOLDOWN_DAYS, MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES) against their Worker-side originals, closing a real gap where values could silently drift. The fix is narrowly scoped, well-documented, and the new test exercises the real cross-package parity concern described in the PR.

Nits — 3 non-blocking
  • The `long-file` flag on packages/loopover-contract/src/api-schemas.ts (472 lines) is pre-existing generated-file growth from an unrelated constant addition, not something this PR should be expected to address.
  • limits.ts:125 comment says "Pinned against their originals like every other entry here," which was previously untrue for these three constants — worth double-checking no other restated groups in this file have the same untested-pin gap.
  • Consider whether future single-constant restatements in limits.ts should require a corresponding pin-test case added in the same PR, to prevent this gap from recurring (e.g. a lint rule or PR template checklist item).

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: 9 registered-repo PR(s), 8 merged, 303 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 9 PR(s), 303 issue(s).
Improvement ✅ Minor risk: clean · value: minor
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: 9 PR(s), 303 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.

Decision record
  • action: hold · clause: missing_linked_issue
  • config: 05fb6894b44060d3c253b884e1934b0006f6805b6db81099606e8c98df291a38 · pack: oss-anti-slop · ci: passed
  • record: b1d0a86894f6c0f2a8f568b4784d6321db89d95355a030d07b981be14bf3f244 (schema v6, head 85a4e48)

🟩 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

@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 30, 2026
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.01%. Comparing base (8248b88) to head (85a4e48).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9936      +/-   ##
==========================================
- Coverage   91.88%   91.01%   -0.88%     
==========================================
  Files         923      923              
  Lines      113407   113408       +1     
  Branches    27332    27332              
==========================================
- Hits       104205   103214     -991     
- Misses       7915     9104    +1189     
+ Partials     1287     1090     -197     
Flag Coverage Δ
backend 94.13% <100.00%> (-1.57%) ⬇️

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

Files with missing lines Coverage Δ
packages/loopover-contract/src/api-schemas.ts 100.00% <ø> (ø)
packages/loopover-contract/src/limits.ts 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant