Skip to content

fix(engine): correct the iterate-policy ceiling field docs to match the real precedence - #10131

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/iterate-policy-ceiling-doc-precedence-9997
Jul 31, 2026
Merged

fix(engine): correct the iterate-policy ceiling field docs to match the real precedence#10131
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/iterate-policy-ceiling-doc-precedence-9997

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

IterationState's two ceiling fields documented an unconditional abandon:

/** … `iterationNumber >= maxIterations` abandons regardless of self-review outcome. */
maxIterations: number;

But decideNextActionWithReason's own numbered precedence list — twelve lines down — puts the selfReview.kind === "pass" branch (step 3) ahead of both ceilings (steps 4-5). The code matches the list, not the field doc: the pass branch returns "handoff" before control ever reaches the iterationNumber >= maxIterations or costCeilingReached checks. So

decideNextAction({ iterationNumber: 5, maxIterations: 5, costCeilingReached: true, selfReview: { kind: "pass" },})

returns "handoff" — which the field doc calls impossible. costCeilingReached's doc had the same contradiction, and iterate-policy.test.ts had no case combining a pass with a reached ceiling, so the precedence between step 3 and steps 4-5 was unpinned — a future reordering would break nothing and silently discard a passing attempt.

The code's ordering is the correct one (the module header says the ceilings exist "so a stuck loop stops wasting turns … chasing a submission that was never going to land" — which does not describe an attempt that has just reached a clean predicted-gate pass). The defect is that the field docs lie to a reader and the precedence is untested.

The fix

Correct both field docs to state the real contract — the ceiling abandons only when the self-review has not reached a clean pass; a clean pass at or past the ceiling still hands off (subject to autonomyLevel) — and point both at decideNextActionWithReason's numbered precedence list as the one canonical statement of the ladder. The phrase "regardless of self-review outcome" is removed from maxIterations.

Unchanged: the precedence list, decideNextActionWithReason / decideNextAction behaviour, AbandonReason, every reason string, and the autonomyLevel handling are all byte-identical — this is a doc correction plus precedence-pinning tests, no behaviour change.

Tests

  • Engine (packages/loopover-engine/test/iterate-policy.test.ts): a clean pass at the iteration ceiling, with the cost ceiling reached, and with both reached each still returns { action: "handoff" } with no abandonReason; and the two branches that DO win over a pass still do (rejectionSignaledrejection_signaled; autonomyLevel: "observe" at a reached ceiling → autonomy_observe_only, not max_iterations_reached) — pinning the ladder end to end.
  • Root (test/unit/engine-iterate-policy-autonomy.test.ts, the vitest mirror that imports the engine src directly for codecov): the same pass-precedes-ceiling assertions.
  • Every existing assertion in both suites keeps passing unmodified.

Validation

  • The src change is comment-only (no coverable changed lines); the precedence-pinning tests are the deliverable and are added to both the engine's own node:test suite (26 green) and the root vitest mirror (14 green).
  • npm run typecheck clean for these files; npm run engine-parity:drift-check passes; npm run dead-exports:check clean.
  • git diff --check clean; no schema/migration/generated-artifact change.

Closes #9997

…he real precedence

IterationState.maxIterations and costCeilingReached both documented an
unconditional abandon ("regardless of self-review outcome"), but
decideNextActionWithReason's own numbered precedence list puts the
selfReview.kind === 'pass' branch (step 3) AHEAD of both ceilings (steps 4-5).
So a clean predicted-gate pass at or past a ceiling still hands off, which the
field docs said was impossible -- and the test suite had no case combining a pass
with a reached ceiling, so the precedence between step 3 and steps 4-5 was unpinned.

Correct both field docs to state the real contract (the ceiling abandons only when
the self-review has not reached a clean pass; a clean pass still hands off, subject
to autonomyLevel) and point at the precedence list as the one canonical statement.
The precedence list, decideNextAction behaviour, reason strings and autonomyLevel
handling are all unchanged -- this is a doc correction plus precedence-pinning tests.

Closes JSONbored#9997
@shin-core
shin-core requested a review from JSONbored as a code owner July 31, 2026 08:20
@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 08:50:28 UTC

3 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a docs-only fix (plus tests) that corrects two contradictory JSDoc comments on `IterationState.maxIterations`/`costCeilingReached` to match the actual, unchanged precedence in `decideNextActionWithReason` (pass-branch, step 3, precedes both ceilings, steps 4-5). The new tests in both the engine's node:test suite and the vitest mirror correctly exercise the real code path (a passing `selfReview` at/past each ceiling) and pin the previously-untested precedence, closing the gap called out in issue #9997. No production logic changes; the fix is purely at the doc/test layer, which is the correct layer since the code's existing behavior was already the intended one per the module's own rationale comment.

Nits — 2 non-blocking

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 #9997
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: 65 registered-repo PR(s), 50 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 65 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff corrects both field docs to remove the 'regardless of self-review outcome' claim, points them at the numbered precedence list, and adds regression tests in iterate-policy.test.ts (plus a bonus test file) covering pass-vs-ceiling precedence for both maxIterations and costCeilingReached as required. The precedence list and decideNextAction behavior are left untouched, matching the issue's c

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript, Solidity, Dart, Python, CSS, PHP, Rust
  • Official Gittensor activity: 65 PR(s), 0 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 <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

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

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.95%. Comparing base (3d7fdbf) to head (16d5b90).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10131      +/-   ##
==========================================
- Coverage   91.95%   91.95%   -0.01%     
==========================================
  Files         931      931              
  Lines      113937   113943       +6     
  Branches    27505    27505              
==========================================
+ Hits       104774   104776       +2     
  Misses       7863     7863              
- Partials     1300     1304       +4     
Flag Coverage Δ
backend 95.66% <ø> (-0.01%) ⬇️
engine 72.72% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...ckages/loopover-engine/src/miner/iterate-policy.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

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.

engine(miner): the maxIterations/costCeilingReached field docs contradict decideNextAction's real precedence, and neither ordering is tested

1 participant