Skip to content

fix(notifications): wire the calibration/gate-outcomes/per-repo builders into the recap digest#8511

Merged
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
kai392:fix/critical-issue-recap-wire-sections
Jul 24, 2026
Merged

fix(notifications): wire the calibration/gate-outcomes/per-repo builders into the recap digest#8511
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
kai392:fix/critical-issue-recap-wire-sections

Conversation

@kai392

@kai392 kai392 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #8372

Re-file of #8465, which the bot closed for a base-branch conflict while it sat green — #8229's routing-shadow work landed on formatMaintainerRecap in the meantime. This branch is cut from current main (4bfb322f) with that change merged in properly, not worked around.

Root cause

buildCalibrationRecapSection, buildGateOutcomesRecapSection, and buildPerRepoRecapSection shipped fully implemented and unit-tested, but nothing in production called them — formatMaintainerRecap imported none of the three. The same "built, tested, never called" shape as #6636.

formatMaintainerRecap instead carried its own inline per-repo map — a second implementation that had drifted from the dedicated builder: no sorting, no cap, no "(+N more)" remainder, so a maintainer watching many repos got an unbounded, arbitrarily-ordered block.

Separately, runMaintainerRecap had no way to pass a drift projection through, leaving formatMaintainerRecap's configDrift option (#8214) unreachable from the orchestrator.

Fix approach

  • Compose all three builders. ## Per-repo keeps its header and renders the builder's .lines through the existing recapSectionLines/redactRecapLine path; the inline map is deleted.
  • ## Calibration and ## Gate outcomes are unconditional rather than flag-gated — both builders read only report.totals/report.windowDays, which every RecapReport always carries, so there is nothing to opt into.
  • Add configDrift?: DriftRecapSection to runMaintainerRecap's options and forward it. Deliberately not sourced here — reading the sentinel state is its own data-sourcing concern. Plumbing only, so the section stays absent until a caller passes one.

Merging with #8229. That change added a routingShadow option resolved on the same call. Rather than pick a side, runMaintainerRecap now assembles the options object key-by-key:

const recapOptions: { routingShadow?: ; configDrift?: DriftRecapSection } = {};
if (routingShadow) recapOptions.routingShadow = routingShadow;
if (options.configDrift) recapOptions.configDrift = options.configDrift;

exactOptionalPropertyTypes forbids passing either key an explicit undefined, and the two are independent — a single ternary can't express all four combinations.

Every added line goes through the existing redaction path; no new text reaches a channel unredacted.

Impact

  • Confined to the digest's rendered text — no data sourcing, delivery, or schema changes.
  • Two existing assertions describe output this issue asks to change, so I updated them rather than the source: the ## Per-repo empty state is now the builder's own No repo activity in the last N day(s)., and rows adopt repo: reviewed N, merged N, closed N. The gate/override/reversal counts those rows carried are unchanged in ## Totals and now broken out by ## Gate outcomes. Both tests gained assertions rather than losing them.

Coverage note worth a reviewer's eye

Rewriting the options assembly made #8229's routingShadow arms changed lines, so they now need coverage that never existed. I added both:

  • present — inserts a real reviewer_routing_shadow audit row and asserts the section renders
  • absent — a proxy env that throws only on the routing query, asserting loadRoutingRecapSection's fail-safe null leaves the rest of the digest intact

Chasing this corrected a misreading on my part: loadRoutingRecapSection returns a section even with zero decisions, so the truthy arm was always taken; the genuinely uncovered path was the error return.

Validation

  • npm run typecheck — clean; oxlint clean
  • 76/77 across all seven maintainer-recap* suites. The one failure (maintainer-recap-wire.test.ts, the already_sent_this_period dedup case) is identical on unmodified 4bfb322f — I verified by checking out clean upstream/main and re-running. Not introduced here.
  • lcov scoped to src/services/maintainer-recap.ts: zero uncovered lines; the only uncovered branches (131, 136) are pre-existing and outside every hunk in this diff
  • git diff --check upstream/main HEAD — clean

@kai392
kai392 requested a review from JSONbored as a code owner July 24, 2026 16:53
RealDiligent and others added 2 commits July 25, 2026 00:54
…the digest

The three section builders shipped fully implemented and unit-tested but were
never composed into the delivered digest, and formatMaintainerRecap's per-repo
body was a second, drifted inline copy of buildPerRepoRecapSection.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
runMaintainerRecap now assembles formatMaintainerRecap's options key-by-key,
so the routingShadow present/absent arms are both changed lines. JSONbored#8229 shipped
that path with no test producing a decision, and none covering the fail-safe
null return.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@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.61%. Comparing base (c096106) to head (63712fb).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #8511       +/-   ##
===========================================
+ Coverage   78.52%   89.61%   +11.08%     
===========================================
  Files         791       98      -693     
  Lines       79348    22783    -56565     
  Branches    23969     3893    -20076     
===========================================
- Hits        62307    20416    -41891     
+ Misses      13869     2187    -11682     
+ Partials     3172      180     -2992     
Flag Coverage Δ
shard-1 74.02% <75.00%> (+20.37%) ⬆️
shard-2 96.10% <100.00%> (+45.00%) ⬆️
shard-3 3.89% <0.00%> (?)

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

Files with missing lines Coverage Δ
src/services/maintainer-recap.ts 97.40% <100.00%> (+0.52%) ⬆️

... 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 17:31:12 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR wires three previously-unused builders (buildCalibrationRecapSection, buildGateOutcomesRecapSection, buildPerRepoRecapSection) into formatMaintainerRecap, replacing the drifted inline per-repo map, and adds configDrift plumbing through runMaintainerRecap merged correctly with #8229's routingShadow via key-by-key options assembly. The change is well-scoped to its stated intent, closes #8372, and the test suite covers both present/absent arms for configDrift and routingShadow, including a real audit_events insert for the routing-shadow present arm and a Proxy-based DB failure injection for its absent arm. The key-by-key options object correctly sidesteps the exactOptionalPropertyTypes issue that a conditional-spread literal would hit with two independent optional keys.

Nits — 3 non-blocking
  • src/services/maintainer-recap.ts: the Calibration and Gate outcomes sections are now unconditional per the PR description's reasoning (both read only report.totals/windowDays which every RecapReport always carries) — worth double-checking that no caller ever constructs a RecapReport with a missing totals shape, though the type system should already guarantee this.
  • test/unit/maintainer-recap.test.ts: the routing-shadow-present test pins the audit_events created_at to the same GEN instant as generatedAt and relies on `created_at >= sinceIso` being inclusive at the boundary — worth a one-line comment noting that's intentional rather than fragile.
  • Consider a follow-up test asserting the relative ordering of the new ## Calibration / ## Gate outcomes sections against ## Per-repo, since consumers parsing the digest by section order could be affected by this reshuffle.

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 #8372
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: 143 registered-repo PR(s), 70 merged, 10 issue(s).
Contributor context ✅ Confirmed Gittensor contributor kai392; Gittensor profile; 143 PR(s), 10 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff wires buildPerRepoRecapSection, buildCalibrationRecapSection, and buildGateOutcomesRecapSection into formatMaintainerRecap (replacing the inline per-repo map, adding unconditional Calibration and Gate outcomes sections) and adds a configDrift option to runMaintainerRecap that is forwarded to formatMaintainerRecap, matching all five stated requirements including section order and the delib

Review context
  • Author: kai392
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, Cuda, JavaScript, Kotlin, Perl, TypeScript, Vue
  • Official Gittensor activity: 143 PR(s), 10 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step 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

@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 fe816af into JSONbored:main Jul 24, 2026
12 checks passed
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.

fix(notifications): wire the maintainer-recap's calibration/gate-outcomes/per-repo section builders into the actual digest output

2 participants