Skip to content

review(public-stats): trim LOOPOVER_PUBLIC_STATS before the truthy-flag regex test #10329

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/review/public-stats.ts's isPublicStatsEnabled checks the LOOPOVER_PUBLIC_STATS env flag with an
anchored truthy-value regex:

export function isPublicStatsEnabled(
  env: { LOOPOVER_PUBLIC_STATS?: string | undefined },
  manifestOverride?: PublicStatsManifestOverride | undefined,
): boolean {
  if (manifestOverride?.present) return manifestOverride.enabled;
  return /^(1|true|yes|on)$/i.test(env.LOOPOVER_PUBLIC_STATS ?? "");
}

Every sibling boolean-env-flag checker in this codebase trims the raw value before running the same
anchored regex — e.g. src/review/pr-reconciliation.ts's equivalent function:

return /^(1|true|yes|on)$/i.test((env.LOOPOVER_PR_RECONCILIATION ?? "").trim());

isPublicStatsEnabled is the odd one out: it tests the raw, untrimmed value. A trailing newline or
whitespace character in the env value — plausible via wrangler secret put reading from a file, or CI/CD
injecting an env var with a trailing newline — makes the anchored regex fail to match even though the
operator's intent was clearly to enable the flag: /^(1|true|yes|on)$/i.test("true\n") is false. The
sibling function's own trimmed behavior is deliberately regression-tested
(test/unit/pr-reconciliation.test.ts); isPublicStatsEnabled has no equivalent test and no .trim() call.

Requirements

  • isPublicStatsEnabled must trim env.LOOPOVER_PUBLIC_STATS before testing it against the truthy-value
    regex, matching the established convention pr-reconciliation.ts's equivalent function (and any other
    sibling flag-checkers in this codebase) already use.
  • Must not change behavior for an already-clean value (no leading/trailing whitespace) — this is a
    whitespace-robustness fix only.
  • Must not change the manifestOverride?.present early-return branch at all.

Deliverables

  • isPublicStatsEnabled trims env.LOOPOVER_PUBLIC_STATS before the regex test, verified by a new test
    asserting isPublicStatsEnabled({ LOOPOVER_PUBLIC_STATS: "true\n" }) (and a leading-whitespace
    variant) returns true.
  • An existing-behavior regression test confirms an already-clean value (e.g. "true", "0", "",
    undefined) still evaluates exactly as before.

Both Deliverables are required in the same PR.

Test Coverage Requirements

This repo's Codecov patch gate requires 99%+ patch coverage on every changed line and branch under
src/**. src/review/public-stats.ts is inside src/**. The new test must exercise the actual
whitespace-trimming behavior end-to-end through isPublicStatsEnabled, not just assert on the regex in
isolation.

Expected Outcome

isPublicStatsEnabled is robust to a trailing/leading-whitespace LOOPOVER_PUBLIC_STATS value, matching
the established whitespace-robustness convention every sibling boolean-env-flag checker in this codebase
already follows.

Links & Resources

  • src/review/public-stats.tsisPublicStatsEnabled (around line 77-83).
  • src/review/pr-reconciliation.ts — the already-correct, already-tested sibling pattern to mirror (around
    line 42).

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