fix(miner): make governor-run-halt's ledger recording a genuine transition detector - #9367
Conversation
…ition detector The old recorded condition (newlyHalted || (!wasHalted && !shouldHalt)) appended a governor-ledger row on every single healthy iteration of a never-halted run, while the resume transition (wasHalted=true, shouldHalt=false) -- symmetric with the halt trip the old condition did correctly capture -- was never recorded at all. Replaced with wasHalted !== verdict.shouldHalt: true exactly on the two real transitions, false on both steady states.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9367 +/- ##
=======================================
Coverage 75.38% 75.38%
=======================================
Files 275 276 +1
Lines 58023 58036 +13
Branches 6181 6189 +8
=======================================
+ Hits 43739 43752 +13
Misses 14014 14014
Partials 270 270
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-27 14:43:13 UTC
Review summary Nits — 6 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk 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.
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.
|
Summary
evaluateRunLoopBoundaryGateinpackages/loopover-miner/lib/governor-run-halt.tsdecided whether to append a governor-ledger row usingnewlyHalted || (!wasHalted && !verdict.shouldHalt). Tracing all four(wasHalted, shouldHalt)combinations: the steady "never halted, still not halted" case appended a row on every single iteration of a healthy run (pure ledger noise, no transition), while the resume transition(wasHalted=true, shouldHalt=false)— symmetric with the halt-trip case the old condition did correctly capture — was never recorded at all, leaving no ledger trace when a run recovered from a halted state.wasHalted !== verdict.shouldHalt: a genuine transition detector, true exactly on the halt-trip and resume cases, false on both steady states.evaluateRunLoopHalt's own halt/no-halt determination logic,newlyHalted, thereleasedItem/markFailedlogic, orbuildRunLoopHaltGovernorLedgerEvent's signature/contents — only therecordedcondition.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm --workspace @loopover/miner run build(typecheck +check-syntax.mjs)tsc --noEmit(roottsconfig.json's full flag set) against both changed files — cleannpx vitest run test/unit/miner-governor-run-halt.test.ts test/unit/miner-loop-cli.test.ts— 53/53 passing. Covers: the steady never-halted case appends nothing (asserted directly against a spy, not just the returned field); the resume transition now records (verified by forcing the collaborator's verdict for one call, sinceevaluateRunLoopHalt's own real implementation latchesshouldHalt:trueunconditionally oncerunHalted:trueis passed in — a genuine resume only happens once an operator externally clears that latch outside this call, so this isolatesevaluateRunLoopBoundaryGate's own transition-detection logic from that collaborator's current coupling); the existing halt-trip and still-halted-latched cases continue passing unmodified;loop-cli.ts(the one real caller) still passes its full 46-test suite unmodified.packages/loopover-miner/lib/governor-run-halt.ts— 100% statements/branches/functions/linesIf any required check was skipped, explain why:
npm run typecheck/npm run test:coverage(unsharded) reliably OOM in this sandbox regardless of diff size — a known sandbox resource constraint, not a signal about this change.packages/loopover-miner/**is not included invitest.config.ts'scoverage.include(confirmed empirically), so it is not Codecov-gated; the gate here isnpm run test:cistaying green with real branch coverage, which the checks above satisfy directly.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
This is a backend-only fix (
packages/loopover-miner/lib/governor-run-halt.ts, a ledger-recording condition) with no rendered UI delta — before and after are the same production capture at each required viewport.apps/loopover-uiis dark-mode-only, so only the Dark row per viewport applies.