Skip to content

docs(analysis): publish the validation, benchmark and known-limitations evidence - #207

Merged
bioedca merged 10 commits into
mainfrom
docs/m9-validation-evidence
Jul 21, 2026
Merged

docs(analysis): publish the validation, benchmark and known-limitations evidence#207
bioedca merged 10 commits into
mainfrom
docs/m9-validation-evidence

Conversation

@bioedca

@bioedca bioedca commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Closes: #158

What

Adds docs/validation.md — one section per NFR-VALID oracle (a)–(g), the kinSoftChallenge external benchmark stated only with its qualifiers, and a scope/known-limitations section. Also fixes the frozen parity artifact's provenance fields and adds a one-line opener to docs/analysis/parity-gallery.md saying it documents plot/feature parity, not accuracy — the mistake a reader was most likely to make.

Every oracle names its test tier. Several are gated to the large or sidecar tiers and do not run on the required 3-OS matrix. A validation page that implied continuous enforcement would overstate the evidence, which here is the worst failure mode available.

Two findings from the adversarial audit worth your attention

Both are about the project, not just the prose.

1. The tamper-evidence claim was backwards — and CI does not catch a loosened tolerance

The draft said a "loosened or tampered artifact fails [the self-coverage tests], on every pull request." It does not. test_frozen_artifact_covers_its_own_measured_evidence and its per-method sibling assert recorded values lie inside the bounds (_assert_spread_within, tests/test_parity.py:312-320), so loosening a bound keeps every recorded value inside it and CI stays green. Only tightening below the recorded evidence fails.

The auditor falsified it by replaying the test's own logic against the committed JSON:

as committed:                                          PASS
$.tolerance wildly LOOSENED (viterbi 0.10, delta 9.99): PASS   <-- not caught
$.tolerance TIGHTENED (state_mean_abs_delta_max 1e-12): FAIL
ebhmm block with loosened per-method tolerance:         PASS   <-- not caught

Nothing pins $.tolerance to its committed values — test_load_frozen_tolerance_returns_the_four_bounds only checks the four keys exist and are finite, and the one exact-value assertion guards $.provisional, which is not the row load_frozen_tolerance returns.

The page now states what is actually enforced: the freeze is protected by review and a deliberate re-freeze ADR, not by a test. Worth deciding separately whether you want a test that pins the bounds — I did not add one here, since it changes the freeze contract and belongs in its own PR.

2. The default tolerance was not measured on the build it is applied to

$.method — the source of the bounds applied to consensus VB-HMM and per-trace vbFRET — records tmaven_commit 71cfa1a… and sidecar_python_version 3.9.23. The project pins tMAVEN 10f4230… (NOTICE, scripts/setup_sidecar.py, sidecar.yml) and the shipped sidecar lock resolves Python 3.12.13. So the live sidecar / parity fits are asserted against a tolerance measured on an earlier upstream commit (13 commits behind, with tmaven/controllers/modeler/ changed in between) and a different interpreter. The measured_by_method.ebhmm block has no such gap — it was measured on the pinned build.

Stated plainly on the page and in the artifact's own build_provenance, together with the mitigation: the freeze only confirmed the provisional §11.2 defaults rather than widening them (tolerance == provisional), and the measured worst cases sit far inside the bounds (8.35e-09 / 1.00e-09 against ceilings of 0.02 / 0.01), so no bound was derived from the older build's numbers.

Artifact and script changes

schema/parity_tolerance.json had "sidecar_python": "C:/ProgramData/miniconda3/envs/tmaven/python.exe" — a committed absolute workstation path that named a machine and identified no build. Both method blocks now carry sidecar_python_version + tmaven_commit + a build_provenance string that is explicit about how each was obtained (the ebFRET values re-derived from the measuring workflow run's own log; the M0.5 values reconstructed on the ratifying workstation, with that caveat stated).

scripts/measure_parity.py now probes both fields at measure time, so a re-measure cannot reintroduce a local path.

No tolerance value or measured result changed.

Checks run locally

  • pytest tests/test_parity.py — 24 passed
  • pytest -m "not large and not sidecar and not deep"2139 passed, 8 skipped, 1 failed
    • the one failure is test_ml_deps_importable.py::test_ranker_dependencies_importableModuleNotFoundError: No module named 'sklearn', a gap in my local venv, untouched by this diff; CI's locked env has it
  • mkdocs build --strict — clean, page in nav, no link to docs/PRD.md
  • ruff check / format --check (0.15.19) — clean
  • reuse lint — compliant
  • grep -rn 'ProgramData\|miniconda3' schema/ — no matches

Self-review checklist

  • Tests green on the base matrix (bar the local-env failure above)
  • Schema freeze respected — no tolerance or measured value changed
  • No dependency change
  • Docs updated, docs-build green
  • No large data, no secrets — and one committed local path removed
  • Conventional-Commit PR title

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 21d57973-4428-478d-895a-a73105e88340

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@bioedca
bioedca force-pushed the docs/m9-validation-evidence branch from 2ebd1e3 to 879280d Compare July 21, 2026 00:10
@bioedca

bioedca commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 879280dcd3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…ns evidence

Someone deciding whether they can publish results produced with Tether had no
page to look at. The record is real and quantitative — seven acceptance oracles,
a frozen tolerance artifact with measured spreads, an external blind benchmark —
but it lived in the unpublished PRD, in developer ADRs, and in JSON under
`schema/`. The one page on the site with "parity" in the title documents *plot
inventory* parity with tMAVEN, not accuracy, and was the first thing a reader
would mistake for validation; it now says so in its opening line.

Adds `docs/validation.md` (one section per NFR-VALID oracle, the kinSoft external
benchmark with its qualifiers, and a scope/known-limitations section), and
replaces the absolute interpreter paths in the frozen parity artifact with fields
that actually identify the build compared against.

The page states, per oracle, which test tier enforces it. Several are gated to
the `large` or `sidecar` tiers and do not run on the required 3-OS matrix; a page
that implied otherwise would overstate the evidence, which on a validation page
is the worst failure mode available.

Four adversarial audits ran against the draft, and two of their findings are
substantive enough to call out:

- The tamper-evidence claim was backwards. The draft said a "loosened or tampered
  artifact fails [the self-coverage tests], on every pull request". Both tests
  assert recorded values lie *inside* the bounds, so loosening a bound keeps them
  inside and CI stays green; only tightening below the recorded evidence fails.
  Falsified by replaying `_assert_spread_within` against a wildly loosened
  tolerance — it passed. The page now says the freeze is protected by review and
  a deliberate re-freeze ADR, not by a test.

- The default tolerance was not measured on the build it is applied to. The M0.5
  vbconhmm block — the source of the bounds used for consensus VB-HMM and
  per-trace vbFRET — was measured against tMAVEN `71cfa1a` on CPython 3.9.23,
  while the project pins `10f4230` and the shipped sidecar lock resolves Python
  3.12.13. The ebFRET block has no such gap. That is now stated on the page and
  in the artifact's own `build_provenance`, together with the mitigation: the
  freeze only confirmed the provisional §11.2 defaults rather than widening them,
  so no bound was derived from the older build's numbers.

`scripts/measure_parity.py` now probes the interpreter version and the tMAVEN
commit at measure time, so a future re-measure cannot reintroduce an absolute
local path. No tolerance value or measured result changed.

Closes: #158
@bioedca
bioedca force-pushed the docs/m9-validation-evidence branch from 879280d to ce22df8 Compare July 21, 2026 02:28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce22df8d29

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/validation.md Outdated
The review found that the page claimed the self-coverage tests fail "on evidence
edited out from under a bound". They did not: `_assert_spread_within` iterated
whatever `values` remained, with no check on the fixture set, the run count, or
non-empty lists. Deleting a fixture outright passed.

Took the enforcement branch rather than weakening the sentence — this is a
scientific-validity page, and the stronger property is the one it should have
been describing.

`_assert_spread_within` now pins the evidence as well as bounding it: the
measured fixture set and each fixture's comparison count are frozen constants
(19/20 for the anchored spreads, 20 otherwise); the pinned count is cross-checked
against that block's own `n_runs_per_fixture`; all four metrics must be present
with the right direction; and each summary's n/min/max/mean/worst is recomputed
from its own values with the production `SpreadSummary` and compared exactly
(mean via approx, to stay off the cross-platform FP-tie rake).

Tamper-tested all four ways, artifact restored byte-identical after each:

    delete fixture smd_281mol            -> measured fixture set changed
    truncate smd_4mol values to 1        -> recorded n disagrees with the frozen evidence
    drop the worst run, recompute every  -> 19 comparisons, frozen evidence has 20
      summary statistic cleanly
    empty the ebhmm evidence             -> measured fixture set changed

The loosening caveat is byte-unchanged and still correct: a *widened* bound is
still not caught by any test, and remains held by review plus the re-freeze rule.
The page's summary bullet — "only tightening below the evidence, or removing the
evidence, is caught" — was aspirational before this commit and is now true.

No tolerance, provisional or measured value changed; `schema/parity_tolerance.json`
and `scripts/measure_parity.py` are untouched.

Consequence worth knowing: a future re-measure that changes the fixture set or
run count now fails until the frozen constants are updated. That is the intended
re-freeze tripwire, not a maintenance bug.
@bioedca

bioedca commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Valid, and a good catch — the page was describing a property the tests did not have.

Reproduced it first, by tampering with the committed artifact and running the two self-coverage tests (artifact restored byte-identical; git diff --stat schema/parity_tolerance.json empty):

tampered: deleted fixture smd_281mol; truncated smd_4mol values to 1; emptied all ebhmm value lists
$ pytest tests/test_parity.py -q -k "covers_its_own or cover_their_own"
2 passed, 22 deselected

_assert_spread_within iterated whatever values remained, with no check on the fixture set, the run count, or non-empty lists — so deleting evidence sailed through.

I took the enforcement branch rather than weakening the sentence. This is a scientific-validity page; the stronger property is the one it should have been describing, and the claim is cheap to make true.

_assert_spread_within now pins the evidence as well as bounding it: the measured fixture set and each fixture's comparison count are frozen constants (19 of 20 for the anchored spreads — one run is spent on the anchor — and 20 otherwise); the pinned count is cross-checked against that block's own n_runs_per_fixture; all four metrics must be present with the direction matching the floor/ceiling map; and each summary's n/min/max/mean/worst is recomputed from its own values using the production SpreadSummary and compared exactly (mean via approx(rel=1e-12), to stay off the cross-platform FP-tie rake this repo has been bitten by before).

Tamper matrix against the new tests — all four now fail:

tamper result
delete fixture smd_281mol measured fixture set changed
truncate smd_4mol values to 1 recorded n disagrees with the frozen evidence
drop the worst run and fully recompute every summary statistic 19 comparisons, frozen evidence has 20
empty the ebhmm evidence measured fixture set changed

The third row is the one that matters — a cleanly-removed run with all statistics recomputed is exactly the edit the old check could not see.

The loosening caveat is byte-unchanged, and still correct: a widened bound is still caught by nothing, and remains held by review plus the re-freeze rule ($.freeze_policy, PRD §11.2, ADR-0009). What changed is that the page's summary bullet — "only tightening below the evidence, or removing the evidence, is caught" — was aspirational before and is now true.

No tolerance, provisional or measured value changed; schema/parity_tolerance.json and scripts/measure_parity.py are untouched.

One consequence to flag: a future re-measure that changes the fixture set or run count will now fail until _EXPECTED_COMPARISONS is updated. That is the intended re-freeze tripwire, not a maintenance burden.

pytest tests/test_parity.py 24 passed · mkdocs build --strict clean · grep -rn 'ProgramData\|miniconda3' schema/ empty · ruff clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e1abb9a7d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/validation.md Outdated
Comment thread tests/test_parity.py Outdated
Comment thread tests/test_parity.py Outdated
bioedca added 2 commits July 21, 2026 01:13
…freeze claims

Three review fixes, all verified against the source they describe.

- docs/validation.md: the default `$.tolerance` row is vs-reference-anchored on
  `model_281mol.hdf5`, but `sidecar-measure.yml` hardcodes `--cross-seed` in its
  measurement argv, which forces `reference=None` for every fixture
  (`scripts/measure_parity.py:206`). So that workflow cannot reproduce the
  default freeze. Name the faithful invocation instead — the script's own
  docstring recipe, with no `--cross-seed`, run in the pinned sidecar build.

- tests/test_parity.py: `_assert_spread_within` allowed a two-wide band
  (`n_runs - 1 <= expected_n <= n_runs`) for an invariant that is exact in
  `parity.measure_spread`. The recorded `reference` field says which mode a
  spread used, so assert the exact count per mode; the old band accepted an
  impossible pairing (19 anchored runs declared against 19 comparisons). All
  three committed blocks already satisfy the exact form — no artifact change.

- tests/test_parity.py: the `$.provisional` assertion compares the artifact to
  the imported `PROVISIONAL` constant, so it catches drift between the two but
  not a PR that moves both in lockstep. Say that, rather than claiming an
  altered `$.provisional` fails.
…ment

Round-1 fixed the `$.provisional` claim in the test docstring but left the
identical, now-contradicted claim live on the published page. Fix the page too,
and correct two more claims about the re-measure recipe.

- docs/validation.md: the `$.provisional` assertion is `data["provisional"] ==
  PROVISIONAL` — a drift check, not a pin to the §11.2 literals. Verified by
  lockstep-editing `state_count_min_fraction` 0.90 -> 0.70 in both
  `src/tether/idealize/parity.py` and `schema/parity_tolerance.json`: all 24
  tests in `tests/test_parity.py` still pass. Drop "pins ... to the PROVISIONAL
  constant" and "an altered `$.provisional`", and say what actually holds.

- docs/validation.md: the named faithful invocation writes to
  `schema/parity_tolerance.json` by default (`measure_parity.py --out`), and the
  dict it writes has no `tolerance_by_method` / `measured_by_method` /
  `build_provenance` key, so running it as printed destroys the ebFRET freeze
  and `test_per_method_tolerances_cover_their_own_measured_evidence` then fails.
  Say so, and say to use a scratch `--out`.

- docs/validation.md: "run in the pinned sidecar interpreter" misdescribes the
  recipe it cites. That recipe runs the script under the BASE interpreter with
  `TETHER_SIDECAR_PYTHON` pointing at the sidecar; only the fits execute there,
  spawned by `tether.idealize.driver`. Name the variable, not the interpreter.

- .github/workflows/sidecar.yml: the header still claimed the pure PR-facing
  `tests/test_parity.py` "catches a tampered/loosened tolerance" — the exact
  assertion this PR disproves and now headlines on the page. Replace it with the
  direction the test really protects, and point at the section.
…hold

Round-2 asserted flatly that the pure `tests/test_parity.py` suite catches no
loosened bound. It catches one: `test_load_frozen_tolerance_selects_per_method`
asserts ebFRET's state-count floor is strictly below the default one, so a
loosened default floor trips it once it reaches ebFRET's frozen 0.5249.

Verified by running `tests/test_parity.py` (py3.13, pytest+numpy+h5py+scipy)
against edited copies of `schema/parity_tolerance.json`: baseline 24 passed;
`$.tolerance.state_count_min_fraction` 0.9 -> 0.40 and -> 0.5249 each give
1 failed (that test), 0.9 -> 0.60 still gives 24 passed. Loosening the other
three default bounds, and all four ebFRET bounds, was checked one at a time:
24 passed every time, so the blindness claim is right everywhere else.

- .github/workflows/sidecar.yml, docs/validation.md: replace the unqualified
  "does not catch a loosened bound" with the threshold that actually holds.
  The neighbouring sentence about the two evidence tests is untouched — it is
  scoped to "they" and is correct as written.

- docs/validation.md: under the printed re-measure recipe, the fits are not the
  only thing running in the sidecar. `scripts/measure_parity.py:254` calls
  `probe_sidecar_build(os.environ.get("TETHER_SIDECAR_PYTHON"))`, which itself
  spawns `[sidecar_python, "-c", _BUILD_PROBE]` (line 135) for the CPython /
  tMAVEN-commit provenance. Say fits plus probe, and say which spawns which.

- docs/validation.md: simulating the overwrite (strip `tolerance_by_method`,
  `measured_by_method`, `method.build_provenance` to match the dict the script
  writes) fails two tests, not one — `test_load_frozen_tolerance_selects_per_method`
  goes down with the per-method one. Name both, and note that the hand-merge
  remedy must also restore `$.method.build_provenance`, which the script never
  writes and which this page relies on above.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12791b1663

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/test_parity.py Outdated
bioedca added 4 commits July 21, 2026 02:10
… lists

Round 3 replaced two over-broad absolutes with two more. Both are now stated
as rules with their boundary, each verified by execution.

sidecar.yml said "the one loosening it does catch is narrow ... every other
loosening is held by review". False: deleting a bound key is the limiting
loosening and fails at least two tests (checked for all eight bounds), and an
infinite ceiling in $.tolerance fails the finiteness assertion in
test_load_frozen_tolerance_returns_the_four_bounds. What tests/test_parity.py
actually holds is the SHAPE of a tolerance block - exactly four bound keys per
block, finite values in $.tolerance only (an infinite per-method ceiling
passes), plus the ebFRET-below-default state-count ordering with its 0.5249
threshold. Only widening a bound to some other finite value is uncaught, which
is what both files now say. docs/validation.md's parallel sentence carried the
same absolute and gets the same rule, as a list of the three invariants.

validation.md's re-measure remedy was enumerative and still incomplete: it
named the three keys the script drops but not $.coverage, which the script
rewrites from pre-ADR-0043 literals - measured_methods becomes ["vbconhmm"]
rather than the committed "vbconhmm (vb Consensus HMM)" this same page quotes
verbatim, applied_to gains "ebFRET (M6)", and note reverts to text asserting
one shared tolerance for every method. Replaced with the rule: the script
assembles one literal dict of exactly ten top-level keys and dumps it over the
target, that dict is the authority, and everything the fresh dict does not
reproduce must be carried back by hand. The three current instances are kept
as examples, explicitly scoped to today's artifact.

Verified: pytest tests/test_parity.py 24 passed; mkdocs build --strict clean;
PyYAML parses sidecar.yml. The mutated schema/parity_tolerance.json was
restored (md5 matches HEAD) and git status shows only these two files.
`_assert_spread_within` inferred the cross-seed-vs-reference mode from the
artifact under test (`fixture["reference"] == _ANCHOR_REFERENCE`), so a
same-mode identity swap satisfied every assertion: re-pointing the 281-mol
block at a different reference model, or at a different SMD path, passed
uncaught. The 281-mol anchor is load-bearing elsewhere -- the producer
(`scripts/measure_parity.py`), the live sidecar fit and `docs/validation.md`
all name it -- so a hand-edit of that one JSON string left the PR-facing test
certifying evidence whose stated provenance no longer matched.

Replace the count-only maps with `_EXPECTED_EVIDENCE`, pinning each block's
SMD path, reference anchor, state count and comparison count, and drive the
anchored/not mode off the pin rather than off the artifact. Values mirror the
producer-side `_FIXTURES` dict.

The pin covers the strings the artifact records, not the files they name; the
docstring and `docs/validation.md` say so rather than implying the evidence
was re-verified against the model.
Three prose corrections in the parity evidence-pin documentation; no
assertion or pinned value changes.

- tests/test_parity.py: drop the trailing "File identity is Git-LFS plus
  the live sidecar fit" sentence from the `_assert_spread_within`
  docstring. Its subject was generic ("File identity") while the check it
  describes pins three paths, one of which -- tests/fixtures/smd_4mol.hdf5
  -- is not Git-LFS: .gitattributes tracks only tests/fixtures/large/**
  and benchmarks/large/**, and `git check-attr filter` on the small
  fixture reports "unspecified". The preceding sentence already states
  what the check does and does not attest, so the clause is removed
  rather than re-qualified.
- docs/validation.md: attribute the ebFRET pin to
  `_EXPECTED_EVIDENCE_BY_METHOD`, which is where it actually lives; the
  sentence previously credited all three per-block pins to
  `_EXPECTED_EVIDENCE`.
- docs/validation.md: split the run-on after the em-dash aside so
  "every fixture must carry all four metrics" starts its own sentence.

Validation: `mkdocs build --strict` on the pinned docs toolchain builds
with no warnings; tests/test_parity.py 24 passed.
@bioedca
bioedca enabled auto-merge (squash) July 21, 2026 08:03
@bioedca
bioedca merged commit 8c6fb11 into main Jul 21, 2026
15 checks passed
@bioedca
bioedca deleted the docs/m9-validation-evidence branch July 21, 2026 08:20
bioedca added a commit that referenced this pull request Jul 21, 2026
Both targets only reached main during this stack: docs/cli.md with #198
and docs/validation.md with #207, so neither link could be written when
the page landed.

- cli.md now points at the parameters page for everything the command
  line does not expose, which was deliverable 3 of #174.
- the tolerances-and-gates section points at the validation page for the
  measured gates, and the parity row names it alongside the ADRs. Still
  no restated numbers, per the same deliverable.
bioedca added a commit that referenced this pull request Jul 21, 2026
* docs(reference): re-home the user-facing analysis parameters

Every tunable a scientist can set lived in PRD §11.2, which the site does not
publish. Adds `docs/reference/parameters.md`: grouped by pipeline stage, in
pipeline order, each parameter with what it controls, its default read out of the
code, and what actually changes if you move it.

`tests/test_docs_parameter_defaults.py` pins the page's stated defaults to the
live constants, so a code change that moves a default fails until the page
follows.

Four adversarial audits ran against the draft, each importing the modules and
printing values rather than reading them off. They found three claims that would
have actively misled someone tuning a run:

- `--coloc-distance` was described as recruiting or discarding acceptor pairs.
  It does neither. Tether is donor-anchored: every in-frame donor spot becomes a
  molecule and its acceptor is read at the mapped position whether or not an
  acceptor was independently detected there. The gate sets only an informational
  `acceptor_detected` flag, which `grep` shows no downstream stage consumes. The
  draft described the Deep-LASI `findColoc` behaviour Tether deliberately
  rejected, so a reader would have widened the gate to "recover low-FRET
  molecules" and changed nothing.
- `vbconhmm` was expanded as "conjugate-prior HMM". It is **consensus** — one
  global HMM fitted across all the project's molecules. The invented expansion is
  also empty (every VB HMM uses conjugate priors) and hid the fact that matters:
  the default idealizer is a population fit, not per-trace.
- The ebFRET contrast was drawn on pooling, but the default is itself a global
  fit. What differs is ebFRET's per-trace state selection under a learned shared
  prior — which is why it is more seed-variable and carries its own parity
  tolerance.

And one real gap in the product, now stated rather than papered over: `tether
batch` exposes **none** of the nine extraction parameters. It always runs
`ExtractOptions()` defaults, overridable only by `--tmap`/`--tdat`. Presenting
those rows as settable "on the `tether extract` / `tether batch` command lines"
was a dead end for every batch user.

Closes: #174

* docs(reference): close the parameter-reference review findings

All four valid.

- The "batch exposes none of the extraction parameters" claim needed a carve-out.
  It holds for the nine flags, but `--tdat` replaces `detection_mode` and
  `detection_threshold` on every movie before the profile is written, so those
  two rows do not describe such a run. `--tmap` replaces no field but makes
  `donor_side` inert. Both stated, with `tdat_source` named as the record.
- Added the `include_stale` row next to its sibling `include_rejected`. The four
  model-backed population functions take it; a molecule goes stale when its
  input-provenance hash diverges after re-extraction or an alpha/gamma
  recalibration, and the fix is to re-idealize rather than to set the flag.
- Added two omitted defaults: `var_floor` (a real science tunable — the clamp
  feeds the next E-step, so it moves state assignment, not just a reported
  sigma) and `states_low`/`states_high`, filed under rendering defaults because
  the constant's own comment says it is not a PRD tunable. Did not repeat the
  reviewer's "silently drops molecules" framing: `StateNumberCounts` carries
  `n_in_range`/`n_out_of_range`, so a clipped axis is accounted for.
- The guard registry had drifted behind the page: 28 printed constants were
  unpinned, including every deep-architecture and plot-export default. Registry
  is now 97 entries, and a new completeness test makes it impossible for the
  registry to lag the page again.

The registry fix also required strengthening the comparison. Two constants could
not be pinned as strings (`1e-06` vs the page's `1e-6`; the repr of a tuple of
str). Rather than uglify the prose, the check now `literal_eval`s the page cell
and compares by value *and* exact type — strictly stronger than the substring
test it replaces, since `1` no longer satisfies `True` and `0` no longer
satisfies `0.0`.

Verified the guard bites: flipping `DEFAULT_DEVICE` to `"cuda"` in the source
fails with the page/code mismatch named, and dropping an entry from the registry
fails the completeness test. Mutation reverted.

* docs(reference): correct the corrected-layer, staleness and missing-parameter claims

Five verified fixes to the analysis parameter reference:

- intensity_quantity: 'corrected' names a background-subtracted layer, not a
  leakage/gamma-corrected one; no population function reads /molecules alpha or
  gamma, so neither setting yields an absolute E.
- include_stale: the post-synchronised A2 heatmap is model-derived but takes no
  include_stale and applies no staleness filter; document the gap and the
  live_idealization_keys workaround.
- Add a rendering-defaults row for the cross-correlation lag axis (max_lag,
  normalize) and correct the 'no tMAVEN counterpart' framing above it.
- Add a deep-classifier row for the 'channels' dataset argument, which sets the
  input width of the first convolution.

* docs(reference): scope the tMAVEN, staleness and layer claims to what the source says

Round-2 corrections to five claims the round-1 commit introduced, each
re-verified against the source rather than restated:

- The rendering-defaults preamble said only the cross-correlation has a tMAVEN
  counterpart "at all". tMAVEN does carry figure width/height as plot
  preferences (analysis_plots/base.py) and export format/DPI as matplotlib
  rcParams (interface/ui_analysisplots.py), and it ships k-means
  (modeler/kmeans.py). Narrow the claim to what holds: none of the five is a
  port of a tMAVEN display default, and the cross-correlation is the only one
  targeting the same quantity as a named tMAVEN routine.
- That paragraph also called tMAVEN's selection.py heuristic "raw", which on
  this page is a defined enum value for the pre-background-subtraction layer.
  tMAVEN runs it on its own corrected data; the word meant *unnormalized*, as
  crosscorr.py's own docstring says. Say unnormalized and disambiguate.
- The A2 heatmap's guard compares the idealized extent against
  donor_all.shape[1], the store-wide padded width of /traces, which is
  zero-padded to the experiment-max frame count. It therefore does not catch a
  re-extraction that shortens a single molecule; only one that narrows the
  shared width. State that instead of "catches a shortening re-extraction".
- There is a second unfiltered model-derived view:
  population_model_gaussian_overlay takes no include_stale, no molecule_keys
  and performs no freshness check. Name both exceptions, not "the exception".
- "every population function computes apparent E from the layer it is handed"
  was over-broad: only histogram/cloud/crosscorr/anticorrelation accept
  intensity_quantity. population_dwell_times, population_state_number,
  population_transition_density and population_transition_prob_histogram have
  no such argument, so the row's "Set via" cell misdirected. Scope both cells.

Also: name the first Conv1d's in_channels as a channel count, not a "width".

Guard test, closing two holes the same commit exposed:

- Register DEFAULT_DEEP_CHANNELS. The round-1 commit added the channels row
  without a registry entry, so
  test_every_constant_the_page_prints_a_value_for_is_registered was failing.
- Add function_default(), an ast reader for signature defaults, and register
  the ~20 printed defaults that are keyword defaults rather than module
  constants: the per-mode detector thresholds and separations, the batch
  over-gate policy, intensity_quantity on all three of its rows, the
  correction overrides, nstates/nrestarts, defer_if_unavailable, the
  population booleans, the dwell model, states_high, max_lag and normalize.
- Add a row-level pass so a drifted default cannot pass by colliding with a
  sibling's value in the same multi-value Default cell. Mutation-checked:
  flipping DEFAULT_CONV_CHANNELS 32 -> 5 passes the old any()-based check and
  fails the new one.

Validated: mkdocs build --strict clean; 126 passed in
tests/test_docs_parameter_defaults.py; ruff check + format clean.

* docs(reference): correct the rcParams citation and the two extent claims

Round-3 corrections, each re-verified against the source tree rather than
restated:

- The round-2 commit cited tmaven/interface/ui_analysisplots.py for tMAVEN's
  export format and DPI. That file sets neither: it only reads the screen DPI
  (line 29) and applies it to the on-screen figure (line 43). The only
  matplotlib.rcParams in the tMAVEN tree are in trace_plot/multi_plot.py
  (savefig.format at line 36, savefig.dpi at line 41). Cite that file, and say
  the trace-plot canvas is what sets them.
- The same commit corrected the rendering-defaults preamble but left the
  walked-back claim standing in the "Science tunables vs rendering defaults"
  bullet, which still said the plot-export settings have "no tMAVEN
  counterpart" — contradicting the corrected paragraph on the same page. Scope
  it the same way: the cloud and the anticorrelation finder have no
  counterpart; the export settings are not a port of a tMAVEN *display
  default*, but tMAVEN does carry a figure size (analysis_plots/base.py) and a
  savefig format/DPI of its own.
- windowed_state_and_channels' docstring still carried the near-duplicate of
  the claim the page had just corrected ("its current trace width (a
  re-extraction shortened it)"). trace_len is donor_all.shape[1], the
  store-wide zero-padded /traces width, so a re-extraction that shortens one
  molecule does not trip the guard. Say what the guard actually compares, and
  name the case it misses. Same fix to the inline comment on the guard.
- "it skips a molecule only when a re-extraction has narrowed that shared
  width" was over-broad: write_idealization_model is a public writer that
  accepts arbitrary state_paths, so an extent past the shared width can arise
  without a re-extraction. State the condition the code tests instead of a
  cause for it; the following sentence already covers the re-extraction case.

Not fixable here: the round-2 commit message listed defer_if_unavailable among
the defaults registered by the new function_default(); it is registered via
dataclass_field('tether.idealize.supervisor', 'SidecarSupervision',
'defer_if_unavailable') at tests/test_docs_parameter_defaults.py:272-276. The
test registry itself is correct — only the prose of an already-landed commit
message is wrong, and correcting it would mean rewriting history.

Validated: mkdocs build --strict clean (anchors: warn + --strict, so the new
#rendering-defaults link is checked); 126 passed in
tests/test_docs_parameter_defaults.py, 54 passed across
test_docs_export_columns.py, test_property_docstrings.py and
test_analysis_histogram_postsync.py; ruff check + format clean on _store.py.

* docs(reference): close the parameter-page gaps found in the new review round

Correctness fixes:

- the intro claimed every extraction parameter is a `tether extract` flag while
  seven rows below said the opposite; scope the claim to the ten CLI-exposed
  `ExtractOptions` fields and name the seven Python-only ones.
- the `intensity_quantity` analysis row claimed `compute_leakage_alpha` /
  `compute_gamma` under its own "Recorded: no"; both stamp the value into
  `/settings/leakage` and `/settings/gamma`, so split them into their own
  Correction-factors row and leave the analysis row answering only for
  `tether.analysis`.
- the analysis-window row claimed corrections read the window; they do not —
  leakage, gamma and correct all slice by `frame_range` and `bleach_frames`.
- the methods section claimed extraction provenance is "fully recoverable"; the
  group is write-once and only five aperture parameters are re-checked, so scope
  it to the one-movie project every CLI/GUI path produces and to the native
  `ExtractOptions` path.

Completeness rows added, each default read out of source and pinned in the guard
registry: the alpha-shape support boundary; `fit_gaussian_hmm(nstates=…)`; the
transition-sync `from_state` / `to_state` / `single_dwell` selection; the
`density` switch; `build_deep_dataset(intensity_quantity=…)`;
`compute_features(intensity_quantity=…, include_rejected=…)`; the elbow's
`k_min` / `in_grid_only`; the deep training `seed`; the return-leg identity
match gate; the cloud's `time_range` / `kde`; and `train_val_split(stratify=…)`.
The fine-tuning row now names `batch_size`, and the rendering-defaults preamble
counts and constant-vs-keyword enumeration follow the new rows.

Guard suite 126 -> 144 passing; `mkdocs build --strict` clean.

* docs(reference): scope the extraction-recovery claim to the CLI paths

The methods-section bullet claimed extraction settings are "fully
recoverable for the one-movie project every CLI and GUI path produces".
Two of the Deep-LASI import legs falsify that: `import_analysis_only_project`
is a deliberately movie-less writer that never calls `write_extraction`, so
there is no `/settings/extraction` group to read at all, and
`reconstruct_project` writes the group with hardcoded aperture constants
that record the provenance of pre-integrated legacy traces rather than an
integration it performed. Scope the claim to `tether extract` / `tether
batch` and state the two import paths separately.

Four smaller corrections on the same page:

- The intro said the seven flagless `ExtractOptions` fields are reachable
  "only by constructing `ExtractOptions(...)` in Python", but four of their
  own rows also offer `integrate_traces(...)`. Say "only from Python".
- The transition-sync row named `from_state`/`to_state` as tMAVEN's
  `sync_hmmstate_1`/`sync_hmmstate_2` without disclosing that tMAVEN's own
  defaults are `0`/`-1` (transitions leaving state 0), while the table
  preamble primes the reader to read the row as a port.
- The elbow row listed `in_grid_only` second in the Parameter cell but last
  in the Default and Units cells; a positional read mapped it to `8`.
- The page intro claimed to answer its three questions "for every number
  Tether applies to your data". It does not cover every constant-less
  keyword default; say "for each parameter it tables" instead.

Guard suite green (144 passed) and `mkdocs build --strict` clean.

* docs(reference): correct three over-broad provenance claims

Three cells/callouts on the analysis-parameter page overstated what a
.tether project records.

- The photobleach callout claimed the priors are "the one gap". They are
  not: compute_photobleach writes no attributes at all, so the
  intensity_quantity trace layer the detection ran on is equally
  unrecoverable — it travels only in the transient PhotobleachSummary.
  The callout and the methods-section bullet now name both.
- The w0 row's "Recorded in .tether" cell said the resulting per-label
  weights are written to /labels, but it lists two surfaces and only one
  of them writes. train_ranker is read-only (it derives the decayed
  weights in memory via _prepare); only recompute_label_weights rewrites
  /labels/table. The cell now separates the two.

No rows added and no registry entries added; tests/test_docs_parameter_defaults.py
(144 passed) and mkdocs build --strict are both green.

* docs(reference): wire the two cross-links issue #174 asked for

Both targets only reached main during this stack: docs/cli.md with #198
and docs/validation.md with #207, so neither link could be written when
the page landed.

- cli.md now points at the parameters page for everything the command
  line does not expose, which was deliverable 3 of #174.
- the tolerances-and-gates section points at the validation page for the
  measured gates, and the parity row names it alongside the ADRs. Still
  no restated numbers, per the same deliverable.

* docs(reference): scope two over-broad claims to the surfaces that hold them

The ranker hyper-parameter rows said the values were "recorded in the
portable model artifact's manifest.json", but neither surface those rows
name writes one: train_quality_ranker returns an in-memory QualityRanker
(src/tether/ml/gbranker.py:239) and ranker_prequential_uplift is read-only
and persists no model (src/tether/project/prequential.py:118). The manifest
is written only by save_model on the train_portable_model /
warm_start_retrain path (src/tether/ml/persistence.py:212, 349), so the cell
made a non-default hyperparams look recoverable when it is not. Narrowed the
cell and the matching methods-section bullet.

The deep device row printed a cpu default while describing "where training
runs", but fine_tune and predict_proba take device=None and resolve it from
the model they are handed (src/tether/ml/deep/model.py:263, 335), so a
classifier trained on CUDA fine-tunes and scores on CUDA. Scoped the cpu
default to train_classifier and stated the inheritance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(analysis): publish the validation, benchmark and known-limitations evidence

1 participant