Skip to content

fix(signals): guard focus-manifest's preferred linked-issue nudge on bodyObserved (#8326)#8448

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
galuis116:fix/focus-manifest-preferred-body-observed
Jul 24, 2026
Merged

fix(signals): guard focus-manifest's preferred linked-issue nudge on bodyObserved (#8326)#8448
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
galuis116:fix/focus-manifest-preferred-body-observed

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Summary

Closes #8326.

buildFocusManifestGuidance in src/signals/focus-manifest.ts builds contributor-facing findings from a repo's maintainer focus manifest. Its "required" linked-issue branch already carries a bodyObserved guard:

if (manifest.linkedIssuePolicy === "required" && linkedIssueCount === 0 && bodyObserved && !hasNoIssueRationale) {

That guard exists to avoid a false positive on a sparse webhook payload where the PR body was never actually observed -- linkedIssueCount === 0 then only means "we don't know", not "there really is no linked issue".

The "preferred" sibling immediately below was missing it, so a repo configured with linkedIssuePolicy: "preferred" could surface the "Maintainer prefers a linked issue" nudge for a PR whose body was never observed -- the exact false-positive class the "required" branch was already fixed to avoid.

Adds && bodyObserved to the "preferred" condition so it matches its sibling.

Per the issue's guidance, hasNoIssueRationale is deliberately not added to the "preferred" branch -- only the concretely-verified bodyObserved gap is closed, leaving the stricter "required" contract's extra exemption where it is.

Tests

Adds a regression test asserting a preferred-policy repo with bodyObserved: false and linkedIssueCount: 0 does not produce the manifest_linked_issue_preferred finding. The existing "prefers a linked issue under the preferred policy" test directly above already covers the bodyObserved: true arm, so both sides of the new operand are exercised.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (Closes #8326).

Validation

  • git diff --check
  • npm run actionlint - no workflow files touched.
  • npm run typecheck
  • npx vitest run test/unit/focus-manifest.test.ts - 793 pass (was 792).
  • Changed-line coverage verified locally: the modified line executes 299x with the new bodyObserved operand exercised on both arms (true via the existing preferred test, false via the new one).
  • npm run test:coverage (full) / test:workers / build:mcp / ui:* - not run: this is a one-line condition change plus one test, touching no worker, MCP, or UI surface.
  • npm audit --audit-level=moderate - no dependency changes.
  • New or changed behavior has unit tests for new branches and fallback paths.

If any required check was skipped, explain why:

  • The one changed src/** line is covered on both branch arms locally, so codecov/patch has real coverage for the diff.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests - n/a, none touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed - n/a; this only narrows an advisory finding's condition.
  • UI changes use live API data or real empty/error/loading states - n/a, no UI change.
  • Visible UI changes include a UI Evidence section - n/a: no visible UI/frontend/docs/extension change.
  • Public docs/changelogs are updated where needed - n/a.

Notes

  • This strictly narrows when an advisory info finding is emitted; it can never introduce a new blocker.

…bodyObserved (JSONbored#8326)

buildFocusManifestGuidance's "required" linked-issue branch already carries a
bodyObserved guard so a sparse webhook payload (body never observed, so
linkedIssueCount === 0 only means "unknown", not "no issue") can't trip a
false positive. Its "preferred" sibling immediately below was missing that
same guard, so a linkedIssuePolicy: "preferred" repo could surface the
"Maintainer prefers a linked issue" nudge for a PR whose body was never
actually observed.

Adds `&& bodyObserved` to the "preferred" condition, matching the "required"
branch. Per the issue, hasNoIssueRationale is intentionally NOT added here --
only the concretely-verified bodyObserved gap.

Adds a regression test asserting a preferred-policy repo with bodyObserved:
false and linkedIssueCount: 0 does NOT produce the manifest_linked_issue_
preferred finding; the existing "prefers a linked issue" test already covers
the bodyObserved: true arm.
@galuis116
galuis116 requested a review from JSONbored as a code owner July 24, 2026 13:25
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.69%. Comparing base (add1f78) to head (7ba71a8).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8448      +/-   ##
==========================================
- Coverage   92.42%   89.69%   -2.74%     
==========================================
  Files         791       98     -693     
  Lines       79294    22949   -56345     
  Branches    23952     4021   -19931     
==========================================
- Hits        73291    20583   -52708     
+ Misses       4866     2187    -2679     
+ Partials     1137      179     -958     
Flag Coverage Δ
shard-1 66.66% <100.00%> (+8.90%) ⬆️
shard-2 30.45% <100.00%> (-17.23%) ⬇️
shard-3 91.35% <100.00%> (+34.78%) ⬆️

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

Files with missing lines Coverage Δ
src/signals/focus-manifest.ts 99.58% <100.00%> (ø)

... and 693 files with indirect coverage changes

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

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-24 13:59:27 UTC

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a minimal, well-targeted one-line fix that adds a missing `bodyObserved` guard to the `preferred` linked-issue-policy branch of `buildFocusManifestGuidance`, matching the same guard already present on its `required` sibling. The change correctly closes the false-positive gap described in issue #8326 (a sparse webhook payload where `linkedIssueCount === 0` only means 'unknown', not 'no issue') and deliberately withholds `hasNoIssueRationale` per the issue's stated scope. The accompanying test exercises the new `bodyObserved: false` arm directly, and the adjacent existing test already covers the `bodyObserved: true` arm, so both sides of the added operand are verified.

Nits — 3 non-blocking
  • The new test's comment (test/unit/focus-manifest.test.ts:704-706) is a bit verbose for a one-line assertion; a shorter inline comment referencing the required-branch precedent would suffice.
  • Consider adding a comment at src/signals/focus-manifest.ts:786 noting the two branches are intentionally kept in sync on `bodyObserved` but diverge on `hasNoIssueRationale`, so a future editor doesn't 'fix' the asymmetry.
  • No functional suggestions beyond the nits — the fix is narrowly scoped and directly traceable to the linked issue.

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 #8326
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: 1897 registered-repo PR(s), 1235 merged, 56 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1897 PR(s), 56 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff adds the exact `&& bodyObserved` guard to the "preferred" branch as requested, deliberately omits `hasNoIssueRationale` per the issue's guidance, and includes a regression test proving the finding is suppressed when bodyObserved is false while relying on an existing test for the bodyObserved:true case.

Review context
  • Author: galuis116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 1897 PR(s), 56 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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 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 14ad838 into JSONbored:main Jul 24, 2026
12 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 24, 2026
12 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.

focus-manifest.ts's preferred linked-issue branch is missing the bodyObserved false-positive guard its required sibling has

1 participant