Skip to content

fix(engine): push progress on maxIterations and percentComplete changes - #9400

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-9323-v2
Jul 27, 2026
Merged

fix(engine): push progress on maxIterations and percentComplete changes#9400
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-9323-v2

Conversation

@bitfathers94

Copy link
Copy Markdown
Contributor

fix(engine): push progress on maxIterations and percentComplete changes

progressChanged only diffed phase, status, iteration, and the activity
tail, but ProgressSnapshot also displays maxIterations and the derived
percentComplete. When an operator raised the iteration budget mid-run
those two moved while the other axes held, so no push was emitted and the
customer-facing progress bar went stale on a real, displayed change.

Compare both fields before the existing axis chain and cover the true and
false path of each with new push / no-push tests.

Closes #9323

Validation

Verified locally on this branch before opening:

  • npm run typecheck
  • npx turbo run build:tsc build:verify
  • npm run test:coverage — new if lines fully covered (line + both branch arms)

@bitfathers94
bitfathers94 requested a review from JSONbored as a code owner July 27, 2026 16:13
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.51%. Comparing base (b4064b6) to head (4418f63).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #9400       +/-   ##
===========================================
+ Coverage   75.46%   89.51%   +14.04%     
===========================================
  Files         275      838      +563     
  Lines       58030   109689    +51659     
  Branches     6199    26125    +19926     
===========================================
+ Hits        43790    98183    +54393     
+ Misses      13970    10239     -3731     
- Partials      270     1267      +997     
Flag Coverage Δ
backend 95.22% <100.00%> (?)
engine 65.80% <100.00%> (+0.08%) ⬆️

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

Files with missing lines Coverage Δ
packages/loopover-engine/src/loop-progress.ts 98.91% <100.00%> (+36.69%) ⬆️

... and 700 files with indirect coverage changes

@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-27 16:47:25 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This adds two early-return comparisons (maxIterations, percentComplete) to progressChanged before the existing phase/status/iteration/activity chain, closing the real gap where raising the iteration budget mid-run moved the displayed percent without touching any of the previously-compared axes. The fix is at the correct layer (the diff comparator itself, not a caller), and both new lines are exercised for true and false arms in three separate test files (root vitest suite, the engine's own node:test suite under dist, matching #9064's dual-flag coverage need). The percentComplete check is logically redundant given maxIterations and iteration are already both compared and percentComplete is purely derived from those two, but keeping it explicit is harmless and arguably clearer for a reader who doesn't want to reason about derivation.

Nits — 5 non-blocking
  • loop-progress.ts: the new `prev.percentComplete !== next.percentComplete` check is derivable from the existing `iteration`/new `maxIterations` comparisons (percentComplete is a pure function of those two), so it's dead weight — either drop it or add a one-line comment explaining why an explicit displayed-field check is preferred over relying on derivation.
  • The `codecov/patch` check reports 0.00% hit despite three new test files touching exactly these two lines; per the given CI status this is a real reported failure, but its cause can't be confirmed from what's provided (the PR branch is 2 commits behind the default branch, which is the documented likely explanation for an unexplained failure on an otherwise well-tested diff) — worth rebasing and re-running before merging.
  • test/unit/loop-progress.test.ts and the node:test duplicate near-identical assertions (same overrides, same expected 40/20 percentages) across two files; this is intentional per the PR description (dual coverage flags) but is worth calling out explicitly in the PR body rather than only in code comments so future readers don't 'DRY' one of them away.
  • Consider consolidating the percentComplete/maxIterations check into a single comment explaining the derivation relationship, so a future reader doesn't wonder why both are compared when one implies the other.
  • If the codecov/patch failure persists after a rebase, paste the actual coverage report in the PR so reviewers can see which branch arm is unhit rather than guessing from a 0.00% headline number.

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 #9323
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High 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: 80 registered-repo PR(s), 54 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bitfathers94; Gittensor profile; 80 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds explicit checks for prev.maxIterations !== next.maxIterations and prev.percentComplete !== next.percentComplete before the existing OR-chain in progressChanged, and adds the required true/false test cases in test/unit/loop-progress.test.ts (plus extra coverage in the engine package's own test suite) without altering activityChanged or the percentComplete/maxIterations computation.

Review context
  • Author: bitfathers94
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript
  • Official Gittensor activity: 80 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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

progressChanged only diffed phase, status, iteration, and the activity
tail, but ProgressSnapshot also displays maxIterations and the derived
percentComplete. When an operator raised the iteration budget mid-run
those two moved while the other axes held, so no push was emitted and the
customer-facing progress bar went stale on a real, displayed change.

Compare both fields before the existing axis chain and cover the true and
false path of each. loop-progress.ts is graded by two Codecov flags — the
root vitest suite (backend) and the package's own node:test suite under c8
(engine, JSONbored#9064) — so both get a matching case, otherwise the flags disagree
on the new lines and the merged patch report undercounts them.
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 27, 2026

@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.

@loopover-orb
loopover-orb Bot merged commit be99f3c into JSONbored:main Jul 27, 2026
8 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 27, 2026
7 tasks
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: progressChanged ignores maxIterations/percentComplete changes in the loop-progress snapshot diff

1 participant