diff --git a/ModularBot_garage/src/env_report.nim b/ModularBot_garage/src/env_report.nim index 8739332..eab2a85 100644 --- a/ModularBot_garage/src/env_report.nim +++ b/ModularBot_garage/src/env_report.nim @@ -311,6 +311,166 @@ proc printBuildIdentity() = discard echo "[env] build binary=", bin, " size=", size, " mtime=", mtime +# ── Part 2: warnings about env mistakes ────────────────────────────────────── +# +# This is the point of the report. A typo must be LOUD, not a silent no-op. +# Two mistakes are caught: +# +# 1. an unknown TR_*/GUN_* name (e.g. TR_TMHORIZON_NSTATE, missing S); +# 2. a compile-time `{.intdefine.}`/`{.strdefine.}`/`{.booldefine.}` symbol +# exported as an env var (e.g. `TMH_NSTATES=2`), which is NEVER read at +# runtime and therefore does nothing. +# +# The known-name set is assembled from the modules' exported env-name +# constants; the remaining inline reads are listed once here. The guard test +# `common_libs/tests/test_env_report.nim` scans the tree for env reads and for +# intdefine/strdefine/booldefine symbols and fails loudly if a name read +# anywhere (or a new define) is missing from these tables, so neither can +# silently drift. + +proc knownEnvNames*(): seq[string] = + ## Every TR_*/GUN_* name this project reads at runtime. Order is irrelevant; + ## callers put it in a set. Keep in sync with the tree scan in the guard test. + result = @[ + # exported constants — the single source of truth (do not re-spell these) + MetricEnvVar, SelectorModeEnvVar, TieBreakEnvVar, + PowerPolicyEnvVar, PowerFarDistEnvVar, PowerFarCapEnvVar, PowerMidCapEnvVar, + PowerRefEnvVar, PowerEnergyHiEnvVar, PowerEnergyLoEnvVar, + PowerEnergyMinEnvVar, PowerEnergyMaxEnvVar, PowerFinishKillEnvVar, + PatternRadOffsetEnvVar, PatternRadScaleEnvVar, + TMH_SHIFT_ENV, TMH_BIG_MULT_ENV, TMH_LOG_ENV, TMH_RESET_ON_TARGET_ENV, + TMH_WINDOW_ENV, TMH_RESET_DROP_ENV, TMH_NSTATES_ENV, TMH_ACCURVE_ENV, + TMH_RETRAIN_EVERY_ENV, TMH_EPOCHS_ENV, + ] + # rack names are constructed from the prefix + gun table, not spelled out + for g in RackGunNames: + result.add RackEnvPrefix & g + # inline reads with no exported constant (the guard test scans for these) + for n in [ + "GUN_SELECTOR_WINDOW", "GUN_SELECTOR_MINOBS", "GUN_SELECTOR_TIE", + "GUN_SELECTOR_FLOOR", "GUN_SELECTOR_POOL", "GUN_SELECTOR_RANK", + "GUN_SELECTOR_SHRINK", "GUN_SELECTOR_DWELL", "GUN_SELECTOR_MARGIN", + "GUN_SELECTOR_POINT_TIE", "GUN_SELECTOR_SEED", + "GUN_RACK_DISABLE", "GUN_STATS_PATH", "GUN_SHOTLOG_PATH", + "TR_MOVEMENT", "TR_MOVEMENT_LOG", "TR_RECORD_WORLDSTATE", + "TR_RADAR_FORCE_SPIN", "TR_RADAR_SCANLOG", "TR_RADAR_SCAN_LOG_PATH", + "TR_TRACKER_PROBE", "TR_TRACKER_PROBE_PATH", "TR_VBULLET_ADMIT_ONLY", + "TR_POWER_LOG", + "TR_RAM_OPPORTUNITY", "TR_RAM_OPP_DIST", "TR_RAM_OPP_MARGIN", + "TR_RAM_ABORT_DMG", "TR_RAM_PLAN", "TR_RAM_PLAN_DIST", + "TR_RAM_PLAN_MARGIN", "TR_RAM_PLAN_HITRATE", "TR_RAM_LOG", + "TR_TFIL_RANGE_LO", "TR_TFIL_RANGE_HI", "TR_TFIL_RANGE_TEMP", + "TR_TFIL_RANGE_K", "TR_TFIL_CORRIDOR_HEAT", "TR_TFIL_WALL_HOTNESS", + # harness vars (read by the test framework, inherited by the bot, so they + # must NOT be reported as typos) + "TR_SERVER_JAR", "TR_BATTLE_RUNNER", "TR_BATTLE_RUNNER_DIR", + # the report's own switch + EnvReportEnableEnvVar, + ]: + result.add n + +proc unknownEnvNames*(pairs: openArray[(string, string)]): seq[string] = + ## Pure: every TR_*/GUN_* name in `pairs` the bot does not read, sorted. + ## Anything else (e.g. PATH, JAVA_HOME) is deliberately ignored. + let known = knownEnvNames().toHashSet() + for (key, _) in pairs: + if (key.startsWith("TR_") or key.startsWith("GUN_")) and key notin known: + result.add key + result.sort() + +type + DefineMapping* = object + ## A `{.intdefine.}`/`{.strdefine.}`/`{.booldefine.}` symbol and the runtime + ## env var (if any) that actually controls the same knob. `suggestedEnv` is + ## the env name a user would naturally guess, used only for the + ## "does not exist" text when `runtimeEnv` is empty. + defineName*: string + runtimeEnv*: string + suggestedEnv*: string + +## Derived by grepping the tree for intdefine/strdefine/booldefine (see the +## guard test, which re-greps and fails if this table drifts): +## tm_horizon.nim:101,102,103,106,108,139 — TMH_* ; only NSTATES has an env +## tm_pattern.nim:50,53,55,57,59,62,65,68,69,85,87,92,94,96 — TM_* +## tsetlin.nim:29,116,118,119,120 — TM_* +## docs/env_reference.md documents the same table. +const CompileTimeDefineMap*: seq[DefineMapping] = @[ + # tm_horizon.nim — the ONLY define with a runtime twin (`TR_TMHORIZON_NSTATES`) + DefineMapping(defineName: "TMH_NSTATES", runtimeEnv: "TR_TMHORIZON_NSTATES"), + DefineMapping(defineName: "TMH_NCLAUSES", suggestedEnv: "TR_TMHORIZON_NCLAUSES"), + DefineMapping(defineName: "TMH_S_DEF", suggestedEnv: "TR_TMHORIZON_S_DEF"), + DefineMapping(defineName: "TMH_MIN_OBS", suggestedEnv: "TR_TMHORIZON_MIN_OBS"), + DefineMapping(defineName: "TMH_STALE_MAX", suggestedEnv: "TR_TMHORIZON_STALE_MAX"), + # tm_pattern.nim — no env form at all; do not invent a plausible one + DefineMapping(defineName: "TM_NCLAUSES"), + DefineMapping(defineName: "TM_NSTATES"), + DefineMapping(defineName: "TM_MIN_OBS"), + DefineMapping(defineName: "TM_CLASSES"), + DefineMapping(defineName: "TM_CONF_MARGIN_DEF"), + DefineMapping(defineName: "TM_SHRINK_DEF"), + DefineMapping(defineName: "TM_GF_MODE"), + DefineMapping(defineName: "TM_SOFT_BETA_DEF"), + DefineMapping(defineName: "TM_RADIAL_RANGE_DEF"), + DefineMapping(defineName: "TM_RAD_MARGIN_DEF"), + DefineMapping(defineName: "TM_REV_TURN_DEG_DEF"), + DefineMapping(defineName: "TM_REV_MARGIN_DEF"), + DefineMapping(defineName: "TM_REV_GAIN_DEF"), + # tsetlin.nim — no env form at all + DefineMapping(defineName: "TM_N_CLAUSES"), + DefineMapping(defineName: "TM_N_STATES"), + DefineMapping(defineName: "TM_T_DEF"), + DefineMapping(defineName: "TM_WINDOW_SIZE"), + # TM_S_DEF is shared by tm_pattern.nim AND tsetlin.nim (one -d sets both) + DefineMapping(defineName: "TM_S_DEF"), +] + +proc isCompileTimeDefine*(name: string): bool = + ## True when `name` is one of our -d: symbols (never a runtime env var). + for m in CompileTimeDefineMap: + if m.defineName == name: return true + +proc setCompileTimeDefines*(pairs: openArray[(string, string)]): seq[string] = + ## Pure: the -d: symbols present as env vars, sorted. + for (key, _) in pairs: + if isCompileTimeDefine(key): result.add key + result.sort() + +proc defineWarningMessage*(defineName, value: string): string = + ## The exact warning text for one -d: symbol set as an env var. "" when the + ## name is not a known define. + for m in CompileTimeDefineMap: + if m.defineName != defineName: continue + let head = "[env] WARNING: " & defineName & "=" & value & + " is a COMPILE-TIME define (-d:" & defineName & "=" & value & + ") and is NOT read at runtime; " + if m.runtimeEnv.len > 0: + return head & "the runtime equivalent is " & m.runtimeEnv & "=" & value & "." + if m.suggestedEnv.len > 0: + return head & m.suggestedEnv & + " does not exist - this knob is compile-time only." + return head & "no runtime env equivalent exists - this knob is " & + "compile-time only." + return "" + +proc printMistakeWarnings*() = + ## The two warning paths, boot-only. Emits nothing when the env is clean, so + ## a correct run keeps the documented A/B/build shape. + var pairs: seq[(string, string)] + for key, val in envPairs(): + pairs.add (key, val) + + let unknown = unknownEnvNames(pairs) + let defines = setCompileTimeDefines(pairs) + if unknown.len == 0 and defines.len == 0: return + + echo "[env] --- warnings: misnamed variables ---" + for key in unknown: + echo "[env] WARNING: ", key, + " is not a recognized TR_*/GUN_* variable; it is IGNORED. " & + "Check the spelling (see docs/env_reference.md)." + for key in defines: + echo defineWarningMessage(key, getEnv(key, "")) + # ── the entry point ────────────────────────────────────────────────────────── var envReportDone = false @@ -329,6 +489,7 @@ proc printEnvReport*(ctx: EnvReportContext) = echo "[env] === ENVIRONMENT (boot report) ===" printRawEnvironment() + printMistakeWarnings() printEffectiveValues(ctx) printBuildIdentity() echo "[env] === END ENVIRONMENT ===" diff --git a/common_libs/tests/test_env_report.nim b/common_libs/tests/test_env_report.nim new file mode 100644 index 0000000..4a12426 --- /dev/null +++ b/common_libs/tests/test_env_report.nim @@ -0,0 +1,161 @@ +## Guard test for the boot-time env report's Part 2 (the warnings). +## +## NO battle, NO Java, NO server. Run with: +## nim c -r common_libs/tests/test_env_report.nim +## +## It pins the two tables that must not drift: +## 1. the KNOWN TR_*/GUN_* name set covers every env read in the tree, so a +## new knob cannot silently start producing a false "unknown variable" +## warning (and, conversely, the set cannot rot); +## 2. the compile-time define map covers every `{.intdefine.}`/`{.strdefine.}` +## /`{.booldefine.}` symbol in the tree, and classifies the ONE real env +## twin (`TMH_NSTATES` -> `TR_TMHORIZON_NSTATES`) correctly. + +import std/[os, strutils, sets, sequtils] +import gun_harness/selector +import "../../ModularBot_garage/src/env_report" + +var failures = 0 +proc check(name: string, ok: bool) = + if ok: echo "PASS: ", name + else: echo "FAIL: ", name; inc failures + +const repoRoot = currentSourcePath().parentDir.parentDir.parentDir + +# ── the source surface the scan walks (the bot's real modules) ─────────────── + +proc scannedFiles(): seq[string] = + ## .nim files under common_libs (excluding common_libs/tests, which are + ## probes/measurement scripts, not bot code) and ModularBot_garage/src. + ## env_report.nim is excluded: it DEFINES the known set, so scanning it is + ## circular (it also names "suggested but nonexistent" env vars on purpose). + for dir in [repoRoot / "common_libs", repoRoot / "ModularBot_garage" / "src"]: + for path in walkDirRec(dir): + if not path.endsWith(".nim"): continue + if "/tests/" in path: continue + if path.endsWith("/env_report.nim"): continue + result.add path + +proc literalsOn(line: string): seq[string] = + ## Double-quoted string literals on one line (env names have no escapes). + var i = 0 + while i < line.len: + if line[i] == '"': + var j = i + 1 + while j < line.len and line[j] != '"': inc j + if j >= line.len: break + result.add line[i + 1 ..< j] + i = j + 1 + else: + inc i + +proc isKnownOrPrefix(name: string, known: HashSet[string]): bool = + ## A literal counts as covered when it is a known name OR a prefix of one + ## (`TR_RACK_` is the rack prefix; `TR_`/`GUN_` are the filter prefixes). + if name in known: return true + for k in known: + if k.startsWith(name): return true + +proc defineSymbolsOn(line: string): seq[string] = + ## ` TMH_NCLAUSES* {.intdefine.} = 40` -> "TMH_NCLAUSES" + for pragma in ["{.intdefine.}", "{.strdefine.}", "{.booldefine.}"]: + if pragma in line: + var sym = line.split('{')[0].strip() + if sym.endsWith("*"): sym.setLen(sym.len - 1) + let parts = sym.splitWhitespace() + if parts.len > 0: result.add parts[^1] + +# ── 1. the known-name set ──────────────────────────────────────────────────── + +proc testKnownNames() = + let known = knownEnvNames() + let s = known.toHashSet() + check "known set has no duplicates", known.len == s.len + check "known set only holds TR_*/GUN_* names", + known.allIt(it.startsWith("TR_") or it.startsWith("GUN_")) + check "known set covers all 16 rack names", + RackGunNames.allIt((RackEnvPrefix & it) in s) + for name in ["TR_MOVEMENT", "TR_POWER_ENERGY_MIN", "TR_TMHORIZON_NSTATES", + "GUN_VBULLET_METRIC", "TR_ENV_REPORT", "GUN_SELECTOR_SEED", + "TR_RACK_TMHORIZON", "TR_TMHORIZON_NSTATES"]: + check "known set contains " & name, name in s + +# ── 2. the pure warning helpers ────────────────────────────────────────────── + +proc testUnknownEnvNames() = + let unknown = unknownEnvNames(@[ + ("TR_POWER_LOG", "1"), ("GUN_STATS_PATH", "/tmp/x"), + ("TR_TMHORIZON_NSTATE", "5"), ("PATH", "/bin"), + ("TMH_NSTATES", "2"), ("JAVA_HOME", "/jdk")]) + check "unknownEnvNames finds the misspelled TR_* only", + unknown == @["TR_TMHORIZON_NSTATE"] + check "unknownEnvNames ignores non TR_*/GUN_* names", + unknownEnvNames(@[("PATH", "/bin")]) == @[] + +proc testDefineMap() = + check "isCompileTimeDefine(TMH_NSTATES)", isCompileTimeDefine("TMH_NSTATES") + check "isCompileTimeDefine(TM_S_DEF)", isCompileTimeDefine("TM_S_DEF") + check "isCompileTimeDefine(TR_TMHORIZON_NSTATES) is false", + not isCompileTimeDefine("TR_TMHORIZON_NSTATES") + check "defineWarningMessage is empty for a non-define", + defineWarningMessage("TR_MOVEMENT", "x") == "" + + let m1 = defineWarningMessage("TMH_NSTATES", "2") + check "TMH_NSTATES warns it is compile-time and names TR_TMHORIZON_NSTATES", + "COMPILE-TIME define (-d:TMH_NSTATES=2)" in m1 and + "TR_TMHORIZON_NSTATES=2" in m1 + let m2 = defineWarningMessage("TMH_NCLAUSES", "40") + check "TMH_NCLAUSES says TR_TMHORIZON_NCLAUSES does not exist", + "TR_TMHORIZON_NCLAUSES does not exist" in m2 + let m3 = defineWarningMessage("TM_S_DEF", "1.5") + check "TM_S_DEF says this knob is compile-time only", + "compile-time only" in m3 + + let defs = setCompileTimeDefines(@[ + ("TMH_NSTATES", "2"), ("TR_MOVEMENT", "tfil"), ("TM_S_DEF", "1.5")]) + check "setCompileTimeDefines finds both -d symbols", + defs == @["TMH_NSTATES", "TM_S_DEF"] + +# ── 3. the tree scans (the anti-drift guard) ───────────────────────────────── + +proc testTreeScan() = + let known = knownEnvNames().toHashSet() + var literals = 0 + var missing: seq[string] + for path in scannedFiles(): + for line in lines(path): + for lit in literalsOn(line): + if lit.startsWith("TR_") or lit.startsWith("GUN_"): + inc literals + if not isKnownOrPrefix(lit, known): + missing.add path.extractFilename & ":" & lit + check "tree scan is alive (>= 50 TR_*/GUN_* literals)", literals >= 50 + check "every TR_*/GUN_* literal in the tree is in the known set: " & + $missing, missing.len == 0 + +proc testDefineScan() = + let mapped = CompileTimeDefineMap.mapIt(it.defineName).toHashSet() + var found: seq[string] + for path in scannedFiles(): + for line in lines(path): + for sym in defineSymbolsOn(line): + found.add sym + var missing: seq[string] + for sym in found: + if sym notin mapped: missing.add sym + check "tree define scan is alive (>= 20 symbols)", found.len >= 20 + check "every intdefine/strdefine/booldefine is in the map: " & + $missing, missing.len == 0 + +# ── driver ─────────────────────────────────────────────────────────────────── + +testKnownNames() +testUnknownEnvNames() +testDefineMap() +testTreeScan() +testDefineScan() + +if failures > 0: + echo "\n", failures, " check(s) FAILED" + quit(1) +echo "\nAll env-report checks passed."