Skip to content

orb(recap): wire the built-and-tested config-drift section into the maintainer digest #9698

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

src/services/maintainer-recap-drift.ts implements buildDriftRecapSection (line 71) in full with unit tests, and formatMaintainerRecap already has the render slot (src/services/maintainer-recap.ts:246-248). Nothing ever produces the DriftRecapSource it needs: buildDriftRecapSection has no caller in src/, apps/ or packages/, and runMaintainerRecap's configDrift option is documented at maintainer-recap.ts:312-316 as "Deliberately NOT sourced here", with its single production caller (src/review/maintainer-recap-wire.ts:215) never passing it.

This is the shape maintainer-recap.ts:17-18 already names for the sibling sections: "#8372: these three section builders shipped fully implemented + unit-tested but were never composed into the delivered digest -- the same 'built, tested, never called from production' shape as #6636."

The drift file's header (maintainer-recap-drift.ts:9-13) deferred the wiring because "a caller wires it the moment the sentinel persists episodes." The sentinel now persists episodes. runConfigDriftSentinel (src/services/knob-loosening-run.ts:407-458) writes one system_flags row per drifting knob keyed config_drift_fingerprint:<knobId> (DRIFT_FINGERPRINT_FLAG_PREFIX, :394), and only writes when the fingerprint changes (:426-435) — so that row's updated_at is the episode's first-fingerprint timestamp, precisely the episodeSince anchor DriftRecapKnob asks for (maintainer-recap-drift.ts:19-23). The live KnobDriftReport per knob is already computed on read by loadKnobStatus (knob-loosening-run.ts:568-576, exposed as KnobStatus.drift), and isConfigDriftSentinelEnabled(env) (:389) is the sentinelEnabled input.

Today a standing drift is visible only as a one-off config_drift_detected log line at the moment the fingerprint changes. The weekly digest — the surface the section exists for, so "a STANDING drift should be impossible to miss" (maintainer-recap-drift.ts:4-5) — never mentions drift, and cannot even render the explicit "sentinel disabled" line (:75-78) that exists so absence of data is distinguishable from absence of drift.

Requirements

  • Add a private loadDriftRecapSection(env, generatedAt): Promise<DriftRecapSection | null> to src/services/maintainer-recap.ts, placed next to and shaped exactly like loadRoutingRecapSection (:275-298): one try, returning null on any read error.
  • Source DriftRecapSource as follows and no other way:
    • generatedAt — the caller's generatedAt (never a fresh clock read inside the loader).
    • sentinelEnabledisConfigDriftSentinelEnabled(env).
    • drifting — for each loadLiveKnobStatuses(env) entry whose drift is non-null and whose drift.direction !== "looser", a { report: status.drift, episodeSince } where episodeSince is the updated_at of that knob's config_drift_fingerprint:<knobId> system_flags row. A knob with a live drift report but no persisted fingerprint row is excluded.
    • cleanKnobs — count of loadLiveKnobStatuses entries with a null drift report.
  • runMaintainerRecap (:300) calls it and assigns the result into recapOptions.configDrift using the same present-or-absent key assignment already used for routingShadow at :335-337 (exactOptionalPropertyTypes forbids an explicit undefined).
  • An explicitly-passed options.configDrift still wins over the loaded one.
  • The system_flags read is a single bounded query (WHERE key LIKE 'config_drift_fingerprint:%'), mirroring the per-repo-override read at knob-loosening-run.ts:534-537. Not one query per knob.
  • No new env flag, no new table, no migration.

⚠️ Required pattern: loadRoutingRecapSection and its wiring at src/services/maintainer-recap.ts:277-298 and :329-337 is the precedent to mirror exactly — a private async loader that fail-safes to null, called from runMaintainerRecap, assigned key-by-key into recapOptions. It does NOT satisfy this issue to source the section in src/review/maintainer-recap-wire.ts, splitting the recap's data sourcing across two files; to invent a drift_episodes table or migration when the system_flags row already carries the timestamp; to pass configDrift only from a test; or to add a new flag gating the section — sentinelEnabled is already the disabled-arm signal.

Deliverables

  • buildDriftRecapSection has a production caller in src/services/maintainer-recap.ts (grep outside test/ returns the definition plus the new call site).
  • With the sentinel flag off, runMaintainerRecap produces a digest containing ## Config drift and drift sentinel disabled — no drift evaluation ran this window. — new case in the recap format test.
  • With the flag on, one knob drifting tighter, and a matching config_drift_fingerprint:<knobId> row, the digest contains one knob line carrying that knob's id, live value, dominating value and a standing N day(s) suffix — new case.
  • A knob with a live drift report but no persisted fingerprint row is excluded from drifting — new case.
  • A thrown system_flags read makes loadDriftRecapSection return null and the digest render with no ## Config drift section rather than throwing — new case.
  • An explicitly-passed options.configDrift still overrides the loaded section — new case.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example adding the loader and its unit tests without assigning it into recapOptions, leaving buildDriftRecapSection still uncalled in production — does not resolve this issue.

Test Coverage Requirements

99%+ Codecov patch coverage, branch-counted, on src/**; src/services/maintainer-recap.ts is inside coverage.include. Both arms of every new conditional: sentinel enabled/disabled, drift present/absent, fingerprint row present/absent, direction === "looser" filtered vs kept, the catch returning null, and supplied-vs-loaded configDrift. Because this is a "no production caller" bug, an end-to-end digest assertion (not just a loader unit test) is required.

Expected Outcome

The weekly maintainer digest carries a Config drift section: an explicit disabled line when the sentinel is off, an explicit clean line when nothing drifts, and one line per standing episode ordered longest-standing first. A knob silently sitting at a Pareto-dominated value for weeks becomes visible on the surface built for it.

Links & Resources

src/services/maintainer-recap-drift.ts (whole file); src/services/maintainer-recap.ts:16, :244-248, :275-298, :300-343; src/services/knob-loosening-run.ts:389-458, :534-537, :568-576; src/review/maintainer-recap-wire.ts:215. Closed #8214 built this section, closed #8372 wired the three siblings.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions