Skip to content

miner(ams-policy): readMinRankAutotuneEnabled probes one filename while the attempt-time resolver probes the documented four #10009

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

The operator's AMS policy file has a documented four-candidate discovery order —
packages/loopover-engine/src/ams-policy-spec.ts:412:

export const AMS_POLICY_SPEC_FILENAMES = [".loopover-ams.yml", ".github/loopover-ams.yml", ".loopover-ams.json", ".github/loopover-ams.json"] as const;

ams-policy.ts honours all four. resolveLocalAmsPolicyReadPath
(packages/loopover-miner/lib/ams-policy.ts:94-107):

  const configDir = resolveAmsPolicyConfigDir(env);
  const canonicalPath = resolveAmsPolicyConfigPath(env);
  if (canonicalPath !== join(configDir, AMS_POLICY_FILENAME)) {
    return existsSync(canonicalPath) ? canonicalPath : null;
  }
  const relativePath = discoverAmsPolicySpecPath((candidate) => existsSync(join(configDir, candidate)));
  return relativePath === null ? null : join(configDir, relativePath);

readMinRankAutotuneEnabled does not. packages/loopover-miner/lib/ams-calibration.ts:311-324:

export function readMinRankAutotuneEnabled(
  env: Record<string, string | undefined>,
  deps: { readFileSync?: typeof fsReadFileSync; existsSync?: typeof fsExistsSync } = {},
): boolean {
  try {
    const path = resolveAmsPolicyConfigPath(env);
    const exists = deps.existsSync ?? fsExistsSync;
    if (!exists(path)) return false;
    const read = deps.readFileSync ?? fsReadFileSync;
    return parseAmsPolicySpecContent(String(read(path, "utf8"))).spec.minRankAutotuneEnabled;
  } catch {
    return false;
  }
}

resolveAmsPolicyConfigPath (packages/loopover-miner/lib/ams-policy.ts:62-64) resolves the canonical
filename only — AMS_POLICY_FILENAME = AMS_POLICY_SPEC_FILENAMES[0]
(packages/loopover-miner/lib/ams-policy.ts:17). So candidates 2–4 (.github/loopover-ams.yml,
.loopover-ams.json, .github/loopover-ams.json) are never probed.

Its own docstring claims the opposite (packages/loopover-miner/lib/ams-calibration.ts:307-310): "the
calibration commands and discover's consumption point need only this one boolean and must stay synchronous, so
this reuses the same path resolution + tolerant parser."
It reuses the parser; it does not reuse the path
resolution.

What breaks for an operator whose policy lives at any candidate other than the first:

  • discover silently ignores their earned min-rank override.
    packages/loopover-miner/lib/discover-cli.ts:539 gates the override read on this exact boolean:
          minRankScore = readMinRankOverride(overrideLedger, { enabled: readMinRankAutotuneEnabled(ledgerEnv) }) ?? AMS_MIN_RANK_SHIPPED;
    and readMinRankOverride returns null outright when enabled is false
    (packages/loopover-miner/lib/ams-calibration.ts:338-339), so the shipped default is used.
  • calibration apply-min-rank refuses with flag_off
    (packages/loopover-miner/lib/calibration-cli.ts:280, packages/loopover-miner/lib/ams-calibration.ts:369),
    and calibration revert-min-rank likewise (packages/loopover-miner/lib/calibration-cli.ts:366).
  • Meanwhile the same file is read at attempt time by resolveAmsPolicy
    (packages/loopover-miner/lib/attempt-cli.ts:706), so its slopThreshold / submissionMode fields take
    effect. The operator sees their policy honoured in one command and silently ignored in another, from the same
    file.

This is the same defect the closed #8863 ("AMS_POLICY_SPEC_FILENAMES documents a 4-path discovery order the
real resolver never uses"
) fixed in ams-policy.tsreadMinRankAutotuneEnabled, added by #8187, is the
sibling call site that fix did not reach.

The existing coverage cannot catch it: test/unit/miner-ams-calibration.test.ts:199-215 only ever writes the
canonical .loopover-ams.yml.

Requirements

  • readMinRankAutotuneEnabled must resolve its file with the identical rules resolveAmsPolicy uses: an explicit
    LOOPOVER_MINER_AMS_POLICY_PATH points at one exact file and bypasses discovery; otherwise the first
    AMS_POLICY_SPEC_FILENAMES candidate that exists under resolveAmsPolicyConfigDir(env) wins.
  • The two resolvers must share one implementation, not two parallel copies. Export the existing
    resolveLocalAmsPolicyReadPath from packages/loopover-miner/lib/ams-policy.ts and call it from
    readMinRankAutotuneEnabled, so a future filename addition cannot re-open this divergence.
  • readMinRankAutotuneEnabled must stay synchronous and must keep its deps.existsSync / deps.readFileSync
    injection seam working — the shared resolver already takes an injected existsSync
    (packages/loopover-miner/lib/ams-policy.ts:96), so thread the injected one through rather than closing over
    the real node:fs.
  • Fail-closed behaviour is unchanged: no file, an unreadable file, or a parse that yields no
    minRankAutotuneEnabled still returns false. A thrown error inside the resolver must still be swallowed by
    the existing catch and return false.
  • The discover and calibration call sites must not change shape — they keep calling
    readMinRankAutotuneEnabled(env) / readMinRankAutotuneEnabled(env, deps).
  • Do NOT change resolveAmsPolicy, AMS_POLICY_SPEC_FILENAMES, discoverAmsPolicySpecPath, the double-gating in
    applyMinRankOverride (packages/loopover-miner/lib/ams-calibration.ts:363-388), or the hard bounds in
    isValidMinRankOverride.

⚠️ Required pattern: reuse packages/loopover-miner/lib/ams-policy.ts:94-107
(resolveLocalAmsPolicyReadPath) verbatim by exporting it — that function is already the #8863 fix. What
does NOT satisfy this issue: (a) inlining a second for (const name of AMS_POLICY_SPEC_FILENAMES) loop into
ams-calibration.ts, which recreates the exact drift #8863 closed; (b) making readMinRankAutotuneEnabled
async so it can call resolveAmsPolicy, which forces an async cascade through
packages/loopover-miner/lib/discover-cli.ts:539 and both calibration-cli.ts call sites; (c) a docs-only PR
that narrows AMS_POLICY_SPEC_FILENAMES or documents "only .loopover-ams.yml enables autotune".

Deliverables

  • resolveLocalAmsPolicyReadPath is exported from packages/loopover-miner/lib/ams-policy.ts and is the
    single path resolver used by both resolveAmsPolicy and readMinRankAutotuneEnabled.
  • With LOOPOVER_MINER_CONFIG_DIR pointing at a temp dir containing only
    .github/loopover-ams.yml with minRankAutotuneEnabled: true, readMinRankAutotuneEnabled(env) returns
    true — asserted in test/unit/miner-ams-calibration.test.ts.
  • The same assertion for .loopover-ams.json and for .github/loopover-ams.json (all four documented
    candidates covered) — asserted in test/unit/miner-ams-calibration.test.ts.
  • First-match-wins: with both .loopover-ams.yml (minRankAutotuneEnabled: false) and
    .github/loopover-ams.yml (minRankAutotuneEnabled: true) present, readMinRankAutotuneEnabled(env)
    returns false — the same precedence resolveAmsPolicy applies.
  • An explicit LOOPOVER_MINER_AMS_POLICY_PATH pointing at a file outside the config dir still wins outright,
    and a LOOPOVER_MINER_AMS_POLICY_PATH pointing at a nonexistent file still returns false without probing
    the discovery candidates — asserted in test/unit/miner-ams-calibration.test.ts.
  • runDiscover applies an earned min-rank override when the enabling flag lives in
    .github/loopover-ams.yml: extend the #9679 fixture at test/unit/miner-discover-cli.test.ts:2307
    (which currently writes resolveAmsPolicyConfigPath(env)) with a sibling case writing
    .github/loopover-ams.yml, asserting the enqueue call still receives minRankScore: 0.2.
  • A regression test at test/unit/miner-ams-calibration.test.ts named for this bug (e.g. REGRESSION: the autotune flag is honoured from every documented AMS policy filename, not just the canonical one) that
    fails against the current code.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one that
makes readMinRankAutotuneEnabled probe all four filenames with its own private loop instead of sharing
resolveLocalAmsPolicyReadPath — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include lists
packages/loopover-miner/lib/**/*.ts, so ams-calibration.ts and ams-policy.ts are measured and gated. Every
branch the change introduces needs both arms tested: the explicit-override vs discovery split in
resolveLocalAmsPolicyReadPath (packages/loopover-miner/lib/ams-policy.ts:98-100), including the
override-exists and override-missing arms; the relativePath === null vs found arms at
packages/loopover-miner/lib/ams-policy.ts:106; the injected vs default existsSync / readFileSync arms of
readMinRankAutotuneEnabled; and its catch fail-closed arm, which must remain reachable (exercise it with a
readFileSync stub that throws). The discoverAmsPolicySpecPath loop in
packages/loopover-engine/src/ams-policy-spec.ts:420 must not be modified; if it is, engine lines are credited by
two uploads whose hits are unioned — add the test to packages/loopover-engine/test/** as well as any root
test/** coverage, or the patch gate can still fail.

Expected Outcome

An operator whose AMS policy file sits at any of the four documented candidate paths gets minRankAutotuneEnabled
honoured by discover and by calibration apply-min-rank / revert-min-rank, exactly as the same file is
already honoured at attempt time — instead of the flag silently reading false and the earned min-rank override
being discarded on every run.

Links & Resources

  • packages/loopover-miner/lib/ams-calibration.ts:307-324readMinRankAutotuneEnabled and its
    "reuses the same path resolution" claim
  • packages/loopover-miner/lib/ams-policy.ts:17, :62-64, :94-107 — the canonical filename constant, the
    single-file path resolver, and the four-candidate resolver
  • packages/loopover-engine/src/ams-policy-spec.ts:412-420AMS_POLICY_SPEC_FILENAMES and
    discoverAmsPolicySpecPath
  • packages/loopover-miner/lib/discover-cli.ts:539 — discover's gate on the flag
  • packages/loopover-miner/lib/calibration-cli.ts:280, :366 — apply / revert gating on the flag
  • packages/loopover-miner/lib/ams-calibration.ts:338-339, :369readMinRankOverride / applyMinRankOverride
    behaviour when the flag is false
  • test/unit/miner-ams-calibration.test.ts:199-215 — the coverage that only writes the canonical filename
  • Closed #8863 — the identical defect fixed in ams-policy.ts

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