Skip to content

fix(services): set finalAttempt in the e2e-test-gen and linked-issue-satisfaction retry loops - #8704

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/final-attempt-quiet-retry-8673
Jul 26, 2026
Merged

fix(services): set finalAttempt in the e2e-test-gen and linked-issue-satisfaction retry loops#8704
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/final-attempt-quiet-retry-8673

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

Closes #8673.

selfhost/ai.ts logs a failed ai.run() at error (Sentry-visible) unless the caller passes finalAttempt: false — reserving the loud path for genuinely single-shot callers; a retried attempt should log at quiet warn. ai-slop.ts, ai-review.ts, and issue-plan-draft.ts all correctly compute finalAttempt = last-attempt && last-model on their multi-attempt loops.

ai-e2e-test-gen.ts and linked-issue-satisfaction-run.ts run the identical 2-model × 3-attempt retry loop but never set finalAttempt. Per the contract, an unset value means single-shot → always loud. So every transient failure that eventually succeeds on a later retry still logged a Sentry-visible error for each earlier attempt in these two features — an observability/noise bug (inflated Sentry error volume), not a functional one.

The fix

Compute finalAttempt: attempt === ATTEMPTS_PER_MODEL - 1 && modelIndex === MODELS.length - 1 on both ai.run() calls (using .entries() to get the model index), matching the sibling files. Behavior-neutral for retry/fallback; only the log level of retried attempts changes (error → warn).

Tests (per function, mirroring ai-slop's convention)

  • warn-on-retry: fail once then succeed → the first (retried) call carries finalAttempt: false.
  • error-on-final: every call fails → 2×3 = 6 calls, and only the last carries finalAttempt: true ([false,false,false,false,false,true]).

Verified bug-catching: removing either finalAttempt line fails that file's tests.

Validation

  • ai-e2e-test-gen.test.ts + linked-issue-satisfaction-run.test.ts: 111 tests pass; typecheck clean; 100% of changed lines/branches covered.
  • Branched off current main, mergeable-clean.

…satisfaction retry loops

selfhost/ai.ts logs a failed ai.run() at error (Sentry-visible) unless the caller
passes finalAttempt:false, reserving the loud path for genuinely single-shot
callers. ai-slop/ai-review/issue-plan-draft all compute
finalAttempt = last-attempt && last-model on their retry loops, but
ai-e2e-test-gen.ts and linked-issue-satisfaction-run.ts (identical 2-model x
3-attempt shape) never set it -- so every transient failure that later succeeds
still logged a loud error for each retried attempt, inflating Sentry noise for
those two features. Compute finalAttempt the same way in both loops (last attempt
of the last model). Behavior-neutral for the retry/fallback logic; only the log
level of retried attempts changes. Adds warn-on-retry / error-on-final tests per
function, mirroring ai-slop's convention.
@shin-core
shin-core requested a review from JSONbored as a code owner July 25, 2026 23:54
@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 25, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.61%. Comparing base (b83b78c) to head (802f05e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8704      +/-   ##
==========================================
+ Coverage   90.56%   90.61%   +0.05%     
==========================================
  Files          96       98       +2     
  Lines       22490    22622     +132     
  Branches     3884     3946      +62     
==========================================
+ Hits        20367    20499     +132     
  Misses       1945     1945              
  Partials      178      178              
Flag Coverage Δ
backend 100.00% <100.00%> (?)

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

Files with missing lines Coverage Δ
src/services/ai-e2e-test-gen.ts 100.00% <100.00%> (ø)
src/services/linked-issue-satisfaction-run.ts 100.00% <100.00%> (ø)

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

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-26 00:02:35 UTC

4 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This PR fixes a real observability bug: `ai-e2e-test-gen.ts` and `linked-issue-satisfaction-run.ts` never set `finalAttempt`, so per `selfhost/ai.ts`'s contract (unset ⇒ single-shot ⇒ always loud) every retried-but-eventually-successful attempt logged at Sentry-visible error instead of warn. The fix correctly computes `finalAttempt: attempt === ATTEMPTS_PER_MODEL - 1 && modelIndex === MODELS.length - 1` using `.entries()` for the model index, exactly mirroring the working `ai-slop.ts`/`ai-review.ts` pattern, and is behavior-neutral for retry/fallback semantics — only the log level changes. Tests for both files correctly exercise both branches (warn-on-retry via a fail-then-succeed call, and error-on-final via 6 exhausted calls with `[false,false,false,false,false,true]`), so this is a genuine, not fabricated, coverage addition.

Nits — 4 non-blocking
  • The `finalAttempt` computation and its explanatory comment are duplicated verbatim across `ai-e2e-test-gen.ts` and `linked-issue-satisfaction-run.ts` (and already existed similarly in `ai-slop.ts`/`ai-review.ts`); a small shared helper (e.g. `isFinalAttempt(attempt, attemptsPerModel, modelIndex, modelCount)`) would remove the 4x repetition, though the existing convention in this codebase already inlines it per-file so this is consistent with prior art.
  • The CI 'Contributor trust' check failed with no detail provided — its cause could not be verified from what's given here, and given this branch is 4 commits behind the default branch that lag is a plausible contributor to any check anomaly, though this specific check is unlikely to be code-related.
  • Consider extracting the `finalAttempt` boolean computation into a small shared helper in `ai-review.ts` (where the other shared retry/budget helpers already live) since four files now duplicate the identical expression.
  • Rebase onto the current default branch (4 commits ahead) before merge to rule out any drift as the source of the failed 'Contributor trust' check.
Flagged checks (non-blocking)
  • Contributor trust — Contributor flagged for review

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

Addressed
The diff adds the exact finalAttempt computation (last attempt && last model) to both ai.run() calls in ai-e2e-test-gen.ts and linked-issue-satisfaction-run.ts, mirroring the pattern in ai-slop.ts, and includes tests for each function that verify finalAttempt:false on a retried attempt and finalAttempt:true only on the truly final attempt. All four stated deliverables appear covered by the diff as

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: 48 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 &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

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 26, 2026
@JSONbored
JSONbored merged commit 63497bd into JSONbored:main Jul 26, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(orb): ai-e2e-test-gen.ts and linked-issue-satisfaction-run.ts never set finalAttempt, so retried attempts log loud instead of quiet

2 participants