Merge pull request #361 from OpenFUSIONToolkit/feature/harness-env-integrity

REGRESSION - IMPROVEMENT - Pin the package set and fingerprint run environments

Author: matt-pharr

docs/development/regression-harness.md

@@ -96,3 +96,48 @@ regress --cases solovev_n1 --ref-range develop~10..develop
 - `--force` — re-run even if cached
 - `--verbose` — print GPEC subprocess output
 - `--no-instantiate` — skip `Pkg.instantiate()` (faster if deps are already resolved)
+- `--no-pin-manifest` — let each ref resolve its own package set (see below)
+- `--allow-env-mismatch` — reuse cached results produced in a different environment
+- `--fail-on-change` — exit non-zero when any tracked quantity changed
+
+## Making source code the only variable
+
+`Manifest.toml` is untracked, so a worktree checked out at an old commit used to resolve whatever
+package versions were newest at run time. Machine-epsilon differences in library math then get
+amplified by the adaptive ODE step controller and by ill-conditioned near-resonant diagnostics
+into double-digit-percent "regressions" that no source change caused.
+
+Two mechanisms prevent that:
+
+**The working tree's Manifest is pinned into every worktree** before `Pkg.instantiate()`, so all
+refs in a comparison run against one package set. `--no-pin-manifest` opts out (and says so
+loudly). If a commit declares a direct dependency the pinned Manifest lacks, `Pkg.instantiate()`
+refuses to run: that ref is recorded as a failed run whose error suggests `--no-pin-manifest` to
+let it resolve its own package set.
+
+**Every run records the environment that produced it** — Julia version, host, resolved Manifest
+hash, Julia and BLAS thread counts. The cache still holds a single result per
+`(commit, case)`, so a re-run replaces the stored one rather than keeping a result per
+environment; what the fingerprint adds is that a cached result whose environment differs from
+the current one is re-run instead of silently reused. `--allow-env-mismatch` skips that check
+and reuses whatever is cached, whatever produced it. Every report prints the environment of
+each ref:
+
+```
+Ref 1: develop  @ a0cad260 (2026-08-12)
+       env: julia 1.11.6, arm64-apple-darwin24.0.0, manifest 7e5c34ad (pinned), 1 thread/8 BLAS
+```
+
+When two compared runs did not share an environment, the report says so before the table rather
+than leaving you to infer it from the numbers.
+
+Results cached before environment fingerprinting existed carry no environment and are therefore
+re-run once — those are exactly the entries whose provenance cannot be established.
+
+Thread counts are recorded but **not** forced: the harness does not silently change how your runs
+execute. If the two refs in a comparison ran under different thread counts, the report flags it.
+
+## Exit status
+
+- `0` — every run completed (and, with `--fail-on-change`, nothing changed)
+- `1` — a run failed, or a quantity changed under `--fail-on-change`

docs/src/developer_notes.md

@@ -220,3 +220,7 @@ regress --cases solovev_n1 --ref-range develop~10..develop
 - `--force` — re-run even if cached
 - `--verbose` — print GPEC subprocess output
 - `--no-instantiate` — skip `Pkg.instantiate()` (faster if deps are already resolved)
+
+The full flag list, the environment-pinning behaviour that keeps source code the only variable in
+a comparison, and the exit-status contract are documented in
+[`docs/development/regression-harness.md`](https://github.com/OpenFUSIONToolkit/GPEC/blob/develop/docs/development/regression-harness.md).

regression-harness/regress.jl

@@ -8,6 +8,7 @@ const DEFAULT_DB_PATH = joinpath(HARNESS_DIR, ".regress_cache.sqlite")
 const CASES_DIR = joinpath(HARNESS_DIR, "cases")
 
 include("src/types.jl")
+include("src/env.jl")
 include("src/config.jl")
 include("src/database.jl")
 include("src/utils.jl")
@@ -26,6 +27,9 @@ function parse_args(args)
     db_path = nothing
     verbose = false
     no_instantiate = false
+    no_pin_manifest = false
+    allow_env_mismatch = false
+    fail_on_change = false
     help = false
 
     i = 1
@@ -46,6 +50,15 @@ function parse_args(args)
         elseif arg == "--no-instantiate"
             no_instantiate = true
             i += 1
+        elseif arg == "--no-pin-manifest"
+            no_pin_manifest = true
+            i += 1
+        elseif arg == "--allow-env-mismatch"
+            allow_env_mismatch = true
+            i += 1
+        elseif arg == "--fail-on-change"
+            fail_on_change = true
+            i += 1
         elseif arg == "--cases" && i < length(args)
             cases = split(args[i+1], ",") |> collect .|> strip
             i += 2
@@ -69,7 +82,8 @@ function parse_args(args)
         end
     end
 
-    return CLIOptions(cases, refs, ref_range, force, list_cases, show_qty, show_case, db_path, verbose, no_instantiate, help)
+    return CLIOptions(cases, refs, ref_range, force, list_cases, show_qty, show_case, db_path, verbose,
+        no_instantiate, no_pin_manifest, allow_env_mismatch, fail_on_change, help)
 end
 
 const HELP_TEXT = """
@@ -93,8 +107,19 @@ Options:
     --db path              Override database path
     --verbose              Print subprocess output
     --no-instantiate       Skip Pkg.instantiate() in subprocess
+    --no-pin-manifest      Let each ref resolve its own package set (default: pin the working
+                           tree's Manifest.toml into every worktree so source code is the only
+                           variable in a comparison)
+    --allow-env-mismatch   Reuse cached results produced in a different environment instead of
+                           re-running them
+    --fail-on-change       Exit non-zero if any tracked quantity changed (for CI use; a failed
+                           run always exits non-zero regardless)
     --help                 Print this help message
 
+Exit status:
+    0  all runs completed (and, with --fail-on-change, nothing changed)
+    1  a run failed, or a quantity changed under --fail-on-change
+
 Examples:
     # Compare two refs
     julia --project=regression-harness regression-harness/regress.jl \\
@@ -170,6 +195,25 @@ function main(args=ARGS)
             error("No commits resolved from the given refs")
         end
 
+        warn_stale_refs(resolved_refs, REPO_ROOT)
+
+        # Pin every ref to the working tree's package set unless asked not to, so that a
+        # comparison varies source code alone.
+        manifest_path = joinpath(REPO_ROOT, "Manifest.toml")
+        pin_manifest = if opts.no_pin_manifest
+            @warn "Manifest pinning disabled — refs may resolve different package sets, and differences below may not be caused by source changes"
+            nothing
+        elseif !isfile(manifest_path)
+            @warn "No Manifest.toml in the working tree; cannot pin the package set. Run Pkg.instantiate() first."
+            nothing
+        else
+            manifest_path
+        end
+        expected_key = opts.allow_env_mismatch ? nothing : expected_env_key(pin_manifest)
+
+        n_failed = 0
+        n_changed = 0
+
         # Run each case at each commit
         for case_spec in case_specs
             println("\n", "="^64)
@@ -179,18 +223,28 @@ function main(args=ARGS)
             for ref in resolved_refs
                 run_commit(db, ref.commit_hash, ref.name, case_spec, REPO_ROOT;
                            force=opts.force, verbose=opts.verbose,
-                           no_instantiate=opts.no_instantiate)
+                           no_instantiate=opts.no_instantiate,
+                           pin_manifest=pin_manifest, expected_key=expected_key)
             end
 
             # Report
-            if length(resolved_refs) == 1
-                report_multi_ref(db, case_spec, resolved_refs)
-            elseif length(resolved_refs) == 2
+            summary = if length(resolved_refs) == 2
                 report_two_ref_comparison(db, case_spec,
                     resolved_refs[1], resolved_refs[2])
             else
                 report_multi_ref(db, case_spec, resolved_refs)
             end
+            n_failed += summary.n_failed
+            n_changed += summary.n_changed
+        end
+
+        if n_failed > 0
+            @error "$n_failed run(s) failed — see the reports above"
+            exit(1)
+        end
+        if opts.fail_on_change && n_changed > 0
+            @error "$n_changed quantity/quantities changed (--fail-on-change)"
+            exit(1)
         end
     finally
         close_database(db)

regression-harness/src/database.jl

@@ -14,6 +14,13 @@ CREATE TABLE IF NOT EXISTS runs (
     runtime_s   REAL,
     success     INTEGER NOT NULL DEFAULT 1,
     error_msg   TEXT,
+    env_key       TEXT,
+    julia_version TEXT,
+    os_arch       TEXT,
+    manifest_sha  TEXT,
+    nthreads      INTEGER,
+    blas_threads  INTEGER,
+    pinned        INTEGER,
     UNIQUE(commit_hash, case_name)
 );
 
@@ -35,6 +42,14 @@ CREATE INDEX IF NOT EXISTS idx_runs_case ON runs(case_name);
 CREATE INDEX IF NOT EXISTS idx_quantities_run ON quantities(run_id);
 """
 
+"""
+Value of a possibly-absent SQLite column, falling back to `default`.
+
+SQLite.jl returns `missing` for NULL while Julia's `something` only skips `nothing`, so columns
+added by a later schema migration (NULL on every pre-existing row) need both cases handled.
+"""
+_column(x, default) = (x === nothing || x === missing) ? default : x
+
 """Materialize SQLite query results as a Vector of NamedTuples."""
 function query_rows(db::SQLite.DB, sql::String, params=())
     result = DBInterface.execute(db, sql, params)
@@ -44,6 +59,29 @@ function query_rows(db::SQLite.DB, sql::String, params=())
     return [NamedTuple{keys(ct)}(Tuple(col[i] for col in values(ct))) for i in 1:nrows]
 end
 
+"""
+Environment columns added to `runs` after the original schema shipped. Databases created before
+fingerprinting keep their rows, with NULL in these columns — such rows never match a computed
+`env_key`, so they are re-run rather than silently trusted.
+"""
+const ENV_COLUMNS = [
+    ("env_key", "TEXT"), ("julia_version", "TEXT"), ("os_arch", "TEXT"),
+    ("manifest_sha", "TEXT"), ("nthreads", "INTEGER"), ("blas_threads", "INTEGER"),
+    ("pinned", "INTEGER")
+]
+
+"""Add any `runs` columns missing from a database created by an earlier harness version."""
+function migrate_schema!(db::SQLite.DB)
+    existing = Set(String[])
+    for row in query_rows(db, "PRAGMA table_info(runs)")
+        push!(existing, String(something(row.name, "")))
+    end
+    for (col, sqltype) in ENV_COLUMNS
+        col in existing && continue
+        DBInterface.execute(db, "ALTER TABLE runs ADD COLUMN $col $sqltype")
+    end
+end
+
 function open_database(path::String)::SQLite.DB
     db = SQLite.DB(path)
     DBInterface.execute(db, "PRAGMA journal_mode=WAL")
@@ -53,19 +91,47 @@ function open_database(path::String)::SQLite.DB
         isempty(s) && continue
         DBInterface.execute(db, s)
     end
+    migrate_schema!(db)
     return db
 end
 
 function close_database(db::SQLite.DB)
     SQLite.close(db)
 end
 
-function is_cached(db::SQLite.DB, commit_hash::String, case_name::String)::Bool
-    rows = query_rows(db, "SELECT id FROM runs WHERE commit_hash = ? AND case_name = ? AND success = 1",
-                      (commit_hash, case_name))
+"""
+Is there a usable cached result for this (commit, case)?
+
+With `expected_key` supplied, a cached run only counts when it was produced in the same
+environment. Rows predating fingerprinting hold NULL and therefore never match — the cache
+entries most likely to be misleading are exactly the ones that get re-run.
+"""
+function is_cached(db::SQLite.DB, commit_hash::String, case_name::String;
+                   expected_key::Union{String,Nothing}=nothing)::Bool
+    if expected_key === nothing
+        rows = query_rows(db, "SELECT id FROM runs WHERE commit_hash = ? AND case_name = ? AND success = 1",
+                          (commit_hash, case_name))
+        return !isempty(rows)
+    end
+    rows = query_rows(db,
+        "SELECT id FROM runs WHERE commit_hash = ? AND case_name = ? AND success = 1 AND env_key = ?",
+        (commit_hash, case_name, expected_key))
     return !isempty(rows)
 end
 
+"""
+Environment key stored for a cached run, or `nothing` when the run is absent or predates
+fingerprinting. Used to explain *why* a cached result was rejected.
+"""
+function cached_env_key(db::SQLite.DB, commit_hash::String, case_name::String)::Union{String,Nothing}
+    rows = query_rows(db, "SELECT env_key FROM runs WHERE commit_hash = ? AND case_name = ?",
+                      (commit_hash, case_name))
+    isempty(rows) && return nothing
+    key = _column(first(rows).env_key, nothing)
+    key === nothing && return nothing
+    return String(key)
+end
+
 function delete_cached(db::SQLite.DB, commit_hash::String, case_name::String)
     # ON DELETE CASCADE handles quantities cleanup automatically
     DBInterface.execute(db, "DELETE FROM runs WHERE commit_hash = ? AND case_name = ?",
@@ -76,18 +142,23 @@ function store_run(db::SQLite.DB, commit_hash::AbstractString, commit_short::Abs
                    commit_date::AbstractString, commit_msg::AbstractString,
                    case_name::AbstractString, runtime_s::Float64,
                    extracted::Vector{ExtractedQuantity};
-                   success::Bool=true, error_msg::AbstractString="")
+                   success::Bool=true, error_msg::AbstractString="",
+                   fingerprint::EnvFingerprint=UNKNOWN_ENV)
     ran_at = Dates.format(Dates.now(), "yyyy-mm-ddTHH:MM:SS")
 
     SQLite.transaction(db) do
         delete_cached(db, String(commit_hash), String(case_name))
 
         DBInterface.execute(db,
             """INSERT INTO runs
-               (commit_hash, commit_short, commit_date, commit_msg, case_name, ran_at, runtime_s, success, error_msg)
-               VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)""",
+               (commit_hash, commit_short, commit_date, commit_msg, case_name, ran_at, runtime_s, success, error_msg,
+                env_key, julia_version, os_arch, manifest_sha, nthreads, blas_threads, pinned)
+               VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
             (String(commit_hash), String(commit_short), String(commit_date), String(commit_msg),
-             String(case_name), ran_at, runtime_s, success ? 1 : 0, String(error_msg)))
+             String(case_name), ran_at, runtime_s, success ? 1 : 0, String(error_msg),
+             env_key(fingerprint), fingerprint.julia_version, fingerprint.os_arch,
+             fingerprint.manifest_sha, fingerprint.nthreads, fingerprint.blas_threads,
+             fingerprint.pinned ? 1 : 0))
 
         run_id = SQLite.last_insert_rowid(db)
 
@@ -147,18 +218,28 @@ Get run info for a (commit, case) pair. Returns NamedTuple or nothing.
 """
 function get_run_info(db::SQLite.DB, commit_hash::String, case_name::String)
     rows = query_rows(db,
-        """SELECT commit_short, commit_date, commit_msg, runtime_s, success, error_msg
+        """SELECT commit_short, commit_date, commit_msg, runtime_s, success, error_msg,
+                  julia_version, os_arch, manifest_sha, nthreads, blas_threads, pinned
            FROM runs WHERE commit_hash = ? AND case_name = ?""",
         (commit_hash, case_name))
     isempty(rows) && return nothing
     row = first(rows)
+    fingerprint = EnvFingerprint(
+        String(_column(row.julia_version, "")),
+        String(_column(row.os_arch, "")),
+        String(_column(row.manifest_sha, "")),
+        Int(_column(row.nthreads, -1)),
+        Int(_column(row.blas_threads, -1)),
+        _column(row.pinned, 0) == 1
+    )
     return (
         commit_short = something(row.commit_short, ""),
         commit_date = something(row.commit_date, ""),
         commit_msg = something(row.commit_msg, ""),
         runtime_s = something(row.runtime_s, 0.0),
         success = coalesce(row.success, 0) == 1,
         error_msg = something(row.error_msg, ""),
+        fingerprint = fingerprint,
     )
 end