docs(plans): make the review-v2 remediation plan executable

[pdlourenco]

Summary

Refines docs/plans/2026-10-04-review-v2-remediation-plan.md (landed in #205) so it can be executed without stalling. The analysis document is untouched (docs/analyses/README.md rule 3).

• §0.1 Must-do set: A1–A6, A8, A10, D1, D2, B1 and F1 item 1. That is the check repairs plus the critical defects. It needs seven decision items: A1, A2, A3, A6, A10, D2 and E1b Cost.
  • The other silent wrong answers form a named first backlog tier: F3, F12, U2, F8/F9, K1–K4 and the SP8 Welch σ_G understatement.
  • A7 is outside the set. Until it lands, nothing catches a vector both ports match wrongly, and the plan says so.
• §0.2 Decision batch: every [decision] in one table, each with a recommended option and a one-line reason.
  • Unmarked rows restate the item text. (new) rows recommend where the plan had none. (revised) rows changed an existing item recommendation after review: K3, D4, D7, A9.
  • These are recommendations, not decisions.
  • A decision that changes an accepted ADR becomes a new superseding ADR, not an edit.
• Issue granularity: one issue per PR group (24 issues, not 43). Must-do rows get a must-do label and a milestone.
• E1 split:
  • E1a is editorial. It starts once its two small §0.2 choices are made (history location; keep Appendix B, marked as unused), and it carries the analysis §6 factual corrections.
  • E1b holds the readings that pick a side: the spec version bump, ETFE WindowSize, SP1, SP3, SP8 and §14.2.
• A2 option (i) completed:
  • (i-a) The PR's MATLAB job pushes regenerated vectors to the same-repo PR branch, without [skip ci]. For fork PRs, (i-b): the maintainer commits the artifact.
  • The generator checks out the PR head SHA.
  • The provenance check is a PR-time test that each changed vector's SHA is an ancestor of the PR head.
  • A push to main regenerates and diffs without committing, so a stale base fails loudly.
  • (i) also removes the bot's commit path and its always-bypass on main.
• A6: the full-clone history is confirmed. The canonical drift comes from repeated R2025a runs, not from local R2024a.
• §10 is trimmed to the facts its per-item table relies on.

Review

Addresses the review of 2026-10-08: all 3 blockers, non-blockers 4–11 and the nits. Point-by-point reply in the PR conversation.

Test plan

Docs-only plan edit. docs/plans/ is outside the published site.

Claims re-verified first-hand:

• K3: ltv_disc on collinear states raises LinAlgError at the last block with no warning.
• SP8: Welch E|Ĝ−G|²/σ_G² comes out at 2.15 in a Monte-Carlo run.
• p_cov and complex_stft are in v0.2.0-python.
• §8.3.4 checks only Λ_{k−1}.
• The ADR "do not rewrite accepted text" rule.
• The PR job checks out the merge ref, and all three merge methods are enabled.

Pre-push review:

• Round 1 flagged 11 issues, fixed in c3946c9.

• Round 2 (review fixes) flagged 10 leftovers: the §9 graph still called D2 decision-free; §10.3 D2/A2/G2/D10/E cells; A2's "fails the PR" vs (i-a); E1b Cost recommendation; counts. All fixed in dcf8bf4.

• Round 3 (re-review fixes) flagged 8 leftovers, all fixed in c4b50fe. The main one: the non-canonical-engine check had no path to replace those bytes.

Addresses the re-review of 2026-10-08 as well; point-by-point reply in the PR conversation.

🤖 Generated with Claude Code

[pdlourenco]

Review of PR #206 — docs(plans): make the review-v2 remediation plan executable

Posted as a comment review because this session's GitHub identity is the PR author, so GitHub does not allow a "request changes" state. The verdict is: request changes (see §5).

Reviewed at PR head c3946c9 against base d8b2b64. Line numbers below are docs/plans/2026-10-04-review-v2-remediation-plan.md at the PR head, written plan:N. Disclosure: this session wrote the base plan in #205. The #206 diff was written in another session, and this review covers that diff and the plan as it stands after it.

Method: seven independent finder agents, each with its own remit: restated §0.2 rows, new §0.2 rows, decision coverage, A2 mechanics, must-do and sequencing, internal and cross-doc consistency, and the PR body. They were seeded with docs/REVIEW_CONTEXT.md. Findings were deduplicated, and each was checked by two refuters with different lenses. A completeness critic went last. Of 44 candidate findings, 43 survived verification, and every blocker below was re-checked first-hand. Mechanical checks ran (§5), so this is a verification-and-validation review.

1. Summary

The PR turns the plan into something that can be scheduled. It adds a cut line (§0.1), a single decision batch with a recommendation per decision (§0.2), one issue per PR group, an E1a/E1b split, a worked A2 option (i), a full-clone confirmation for A6, and a shorter §10. The direction is right, and the decision batch is the right instrument for CLAUDE.md §4.

It is not ready to merge as written, for three reasons. Several §0.2 rows ask the maintainer to decide on a premise that the spec or the tree contradicts. Two items labelled decision-free carry contract decisions and are scheduled for day 1. And under A2's recommended option (i), the step that produces canonical bytes is assigned to an author who cannot perform it from a cloud session. All of it is text-level; nothing changes the plan's direction.

2. What works well

• §0.2 is honest about provenance. Rows that restate an item are unmarked, new ones carry (new), and the batch says plainly that these are recommendations, not decisions.
• A2 (i) correctly spots two real flaws in the original text. Ancestry checks do not survive squash or rebase merges, and the default refs/pull/N/merge checkout would stamp a commit that never reaches main.
• The A2 motivation reproduced live on this PR's base. Merging #205 re-stamped all 32 vectors to f9744bd in d8b2b64, and every payload is identical apart from the provenance block.
• The A6 history caveat is resolved properly. The clone now has 606 main commits, and da5a625 does introduce the ltv_io 1e-2.
• Splitting E1 is right in principle: editorial work should not wait on decisions.

3. Issues to address before merge

Blockers

1. Five §0.2 rows rest on a premise that does not hold. The batch asks the maintainer to settle these in one pass on the stated reasons (CLAUDE.md §3–§4; principles 1–3).

   • K3, plan:95 (repeated at plan:159). The row says "keep §8.3.4 (warn and return); Python conforms | the spec already says it".
     • §8.3.4 checks only rcond(Λ_{k−1}) (spec/SPEC.md:993). Both ports check only the previous block (python/sid/_internal/ltv_cosmic_solve.py:107–121, matlab/sid/private/sidLTVcosmicSolve.m:66–77). The analysis's K3 row names exactly this as the structural defect: a singular last block is never diagnosed.
     • A probe confirms it. ltv_disc with X = [s, 2s], U = 0 and N = 50 raises LinAlgError at k = 49 with no warning, at rcond(Λ_48) = 3.1e-3. That is the item's own collinear-state fixture.
     • Suggested row: "amend §8.3.4 so the rcond check covers every block the recursion inverts, Λ_0 … Λ_{N−1}, the last included; keep warn-and-return (sid:singularLbd); both ports conform | as worded, §8.3.4 never checks the last block, which is where the collinear-state fixture is singular (analysis K3); no new error code".
     • The test should assert the warning in both ports. Assert a non-finite result only in Python and on MATLAB proper, because Octave's \ returns a finite minimum-norm result (analysis Appendix A).
   • D4, plan:88 (plan:153 says the same). The row says "NaN for both | what §2.7 already says".
     • §2.7 (SPEC.md:245–269) covers Φ̂_v only. §3.2 (:326) bounds γ̂² to [0, 1], and §3.3 (:342) handles a degenerate frequency as γ̂² < ε. Python sets the coherence to 0 there (degenerate.py:325).
     • A NaN γ̂² is therefore a change to §3.2/§3.3 that D4's spec column (plan:41) does not list. It would also break A7's coherence ∈ [0, 1] invariant (plan:120).
     • Suggested row: "Φ̂_v: NaN in every estimator (what §2.7 says). γ̂²: keep 0 (consistent with §3.2/§3.3); NaN would be a contract change to §3.2/§3.3 and would relax A7's invariant to finite entries". Align plan:153.
   • D7, plan:91 (plan:156). The row maps SidWarning.code "1:1 to the §10 identifiers", but §10 (SPEC.md:1872–1923) names no sid: identifier.
     • The spec names five, across §2.3, §6, §8.12.4 and §8.14. MATLAB emits 14. A 1:1 mapping would in practice copy MATLAB's set into Python (principle 2), and the D7 spec column says only "python guide" (plan:44).
     • Suggested: "spec first: add a warning-identifier table to §10 (each diagnostic and its condition); then Python SidWarning with code mapped 1:1 to that table, and MATLAB checked against it". Set the D7 spec column to "§10 identifier table; python guide" and the ports to "both".
   • SP8 Welch σ_G, plan:102 (new). The row codifies the current formula because "both ports already agree".
     • §3.3 (SPEC.md:330–345) defines σ_G as the complex-total std E|Ĝ − G|² and names the Welch path. Analysis F18 records that the implemented |Ĝ|√((1−γ²)/(γ²ν)) (freq_map.py:186, sidFreqMap.m:522–543) is √2 below that form.
     • Two independent Monte-Carlo runs in this review gave E|Ĝ − G|² / σ_G² ≈ 2.0–2.15, where 1 is expected. Agreement between ports is a consequence, not a reason (CLAUDE.md §3).
     • Suggested: split SP8 into its own row. "(new) make Welch σ_G the §3.3 complex-total std (√2 × current), state it in §6.5, regenerate reference_freqmap_welch; alternative: declare it magnitude-only in §6.5 and exempt Welch from §3.3". Give SP1, SP3, the MIMO NaN, the DC bin and §14.2 a reason each.
   • p_cov / complex_stft rename, plan:99 (new). "Nothing published breaks" is false.
     • The v0.1.0-python and v0.2.0-python releases ship both names (python/sid/_results.py:82,186,298 at the tag). python/RELEASE_NOTES.md:232 and python/README.md:127 document them, and spec/EXAMPLES.md:1066,1087 names complex_stft.
     • Suggested Why: "restores §9's purely syntactic mapping; no PyPI user yet, but the GitHub releases and docs expose the names, so ship as a documented breaking change (release-notes 'Breaking' entry, optionally a one-release alias), EXAMPLES.md first".

2. Two items are labelled decision-free but carry contract decisions (CLAUDE.md §4).

   • E1a, plan:166 (row plan:49). It is called "editorial, no decision … can start on day one", but it does two things that are choices.
     • It writes SegmentLength/ConsistencyThreshold/CoherenceThreshold into §8.4.3. Today §8.4.3 lists only LambdaGrid and Algorithm (SPEC.md:1066–1074). The PR's own K7–K16 row (plan:105) and D10 (plan:159) list those extras as an open "specify or remove" decision, so day-1 E1a would pre-empt it.
     • It bumps SPEC.md "to a version that reflects 0.2.0" and adopts a versioning rule. SPEC.md is already at 1.0.0 (SPEC.md:3), so the direction of the bump and the policy are a choice.
     • Suggested: keep only "add Uncertainty/NoiseCov/CovarianceMode to §8.2 and Method to §8.4.3". Move the tuning extras under the K7–K16 decision ("added to §8.4.3 only if 'specify' is accepted"). Move the version bump and versioning rule to E1b, or give them a §0.2 row.
   • D2, plan:68, :72, :204 (item text plan:151). It is called "no decision needed", but it adds two §14.5 error conditions and a typed error.
     • The conditions are N_data ≤ DataLength and 1 ≤ MaxLag ≤ N − 1. The typed error is for u = [] on state-space models, and it reverses §14.5's documented default [] (SPEC.md:2126).
     • Real alternatives exist. For example, [] could mean zero input, or be allowed when q = 0: residual accepts zeros(N, 0) on a q = 0 model today.
     • The label predates this PR, but the PR promotes D2 into the day-1 must-do set.
     • Suggested: "D1 — no decision; D2 — needs the §14.5 additions accepted". Add a §0.2 D2 row marked (new): typed errors for N_data > DataLength and for MaxLag out of range; u = [] an error only when q > 0. Update plan:72 to five decisions and take D2 off the day-1 list.

3. A2 option (i) assigns the canonical bytes to someone who cannot produce them (plan:115, plan:233, plan:280).

   • Under (i), "the PR author commits that artifact's files". Most PRs here are written by cloud sessions.
   • From this container, the artifact download redirects to productionresultssa*.blob.core.windows.net, and the egress proxy denies it (CONNECT 403; tested with both the API and curl).
   • The plan also disagrees with itself: §10.2 (plan:233) says the PR author commits, while §10.4 (plan:280) lists the same step as maintainer-only. Every ci-main cell in §10.3 depends on it.
   • Suggested: a (new) sub-decision in A2 and §0.2 on who commits under (i).
     • (i-a) The PR's MATLAB job pushes the regenerated vectors to a same-repo PR branch when the numeric diff reports a semantic change. It must not use [skip ci], so the validators run on that commit. Fork PRs fall back to (i-b).
     • (i-b) The maintainer downloads the artifact and commits it locally.
     • Recommend (i-a). That means the §0.2 row's "no auto-commit" should read "no commit on main". Align §10.2 and §10.4.

Non-blockers

4. The must-do set's criterion does not match its membership (plan:63, :67, :72, :199–206).

   • "The subset that stops wrong answers and false claims; everything else is coverage and hygiene" is not what the cut does. Outside the set are silent wrong answers and crashes on documented input: F12 (D6: MATLAB ETFE and Welch silently return NumTrajectories = 1), F3 (D3), U2 (D5), F8/F9 (D9) and K1/K2/K4 (D10). False user-facing claims are outside it too (F1 items 2–8).
   • Either reword: "the gate repairs plus the critical defects; it does not remove every silent wrong answer — F3, F12, U2, F8/F9 and K1–K4 form the first backlog tier, gated on their §0.2 decisions". Or pull D3, D6 and D10 K1–K4 in and raise the decision count. That choice is the maintainer's.
   • A10 is missing from the membership table, yet plan:72 counts its decision and plan:204 schedules it.
   • A7 is left out without comment. Say that until it lands, nothing catches a vector both ports match wrongly.
   • By A1's own text, requiring the MATLAB context blocks fork PRs until the token-mint step is guarded, and that guard sits in A9, outside the set. Move the guard into A1.
   • The day-1 lists conflict. plan:63 says everything outside the set comes after it, yet §9 starts E1a, E3 and all of F1 on day 1 (plan:199–200), and plan:204 puts F1 item 1 after A10. Suggested: "F1 items 2–8 day 1; item 1 after A10", with decision-free items allowed alongside the set.
   • Several groups mix day-1 rows with decision-gated rows (A-ci, B-relax, E-spec). Because "the last PR of a group closes it", no issue closes when the must-do set is done. Use a must-do label or milestone, or split groups at the cut line. Also, "about twenty" is 24 groups; 43 rows is correct.

5. Loose ends in A2 option (i) (plan:114–115, :241, :81).

   • The (a) bot-PR / (b) workflow_run + revert choice and "decouple the SHA" still read as unconditional, though they are moot under (i). State what (i) removes: the commit step, the App-token mint and the workflow_dispatch push path, contents: write, and the bot's always-bypass in main.json (and the live "Main" ruleset). Removing that bypass is what lets A1 + A2 meet the Phase-A exit criterion.
   • Stale base. With strict_required_status_checks_policy: false and the generator on the PR head, two individually green PRs can merge into stale vectors. Nothing regenerates on main, so the next, unrelated PR gets the blame. Add: on push to main, regenerate and run the numeric diff without committing, and fail loudly.
   • "Real commit" check. Any commit of any PR passes it, abandoned ones included. Local consumer runs have no refs/pull/*, and fetch-depth: 0 does not fetch pull refs. Suggested: a PR-time CI check that each changed vector's full 40-hex git_sha is an ancestor of the PR head; consumers reject only "unknown" or malformed SHAs; the generator stamps the full SHA.
   • Every PR merge to date has been a merge commit, so the merge-commit-only alternative costs nothing today. The setting probably lives in the untracked "Main" ruleset.

6. Other §0.2 premises to correct (non-blocking, since none of them changes the recommended option).

   • D5 (plan:89): lti_freq_io's DC rule is a linear extrapolation, Re(2Ĝ₁ − Ĝ₂) (§8.12.12), not a hold. Say that it differs rather than that it is consistent.
   • D6 (plan:90): the item names sid:dimMismatch only for frequency-domain 3-D input. For y 2-D with u 3-D, the shared validator already uses sid:trajMismatch/traj_mismatch for the mirror case (sidValidateData.m:255, validate_data.py:279). Use that code, or mark the choice (new).
   • A9 (plan:84): the failing 1e-15 tolerance is reference_internals' W_rel, not a test-side tolerance, and A4 forbids hardcoding one in a test. Write "add a W_atol to reference_internals in generate_reference.m (A5 needs one anyway), regenerated per A2".
   • E1b IO Cost (plan:101): neither port's residual or compare reads Cost, so there is no special-casing to stop. The rename breaks the released LTVIOResult.cost and MATLAB .Cost, the spec defines no (1×3) IO triple, and A6 stores the Cost history in reference_ltv_io first. Rewrite the Why, set the E1b ports cell to "spec + both ports + reference_ltv_io", and order the decision before A6.
   • Condition metric (plan:98): cond raises on non-finite input in numpy and Octave, where rcond returns 0. Treat a non-finite block as rcond = 0 (the degenerate.py:351 guard), so K3's warn-and-return cannot turn into an error.
   • B2 (plan:86): only "≥ 1" follows from the derivation (Bayesian ≽ sandwich). An upper bound "from the measured λ = 1e0 ratio plus MC margin" is a measurement, which is what B2 says the band must not pin. A cap near 1.4 at the gated λ = 1e2, where the ratio is 1.07, would also pass a ~30 % over-statement. Suggested: a per-λ upper bound from a stated requirement in §8.9 that rejects the S1 over-statement.
   • K7–K16 (plan:105): specifying the lti_freq_io trim "as implemented (both ports agree)" overrides §8.13.1's "average across trajectories" and makes a silent discard part of the contract. Specify it with a warning that names the discarded trajectories (or average all, as §8.13.1 says), and give each tuning extra its own reason.

7. "ADR amendments" contradicts the ADR process (plan:76; also the A1, A2 and A3 cells). docs/decisions/README.md §"Editing a merged ADR" forbids rewriting accepted ADR text; a changed decision is a new, superseding ADR. Suggested: "recorded as new ADRs; a decision that changes an accepted ADR (A1 vs ADR-0005; A2, A3 vs ADR-0002; K1 vs ADR-0004) is a superseding ADR, not an edit".

8. G2 decisions are incomplete (plan:58, :106, :181, :280). #204 and #203 carry the same "Options (recommend, don't decide)" framing as #202. G2's text settles #204 ("choose option 1") with no marker, and #204 may be moot under A2 (i). Mark both and add §0.2 rows; note the A2 coupling on #204.

9. E1a/E1b enumerations (plan:49, :166, :167).

   • The E1a row omits four things §5 E1a does: 'welch' out of the Method enumeration, SP5 (now in neither row), L in §3.4/§3.5, and the ETFE WindowSize.
   • Several analysis §6 factual corrections are in neither list, and no other plan item covers them: §8.12.2's reported IO cost is 2N·f(C), not N·f(C); §8.4.2 is missing a ½; and uncertainty_derivation.md has wrong X'(k) dimensions, the ν N-factor missing, an overloaded H⁻¹, and a "Full = Default" row. Append them to E1a as factual corrections.

10. §10.3/§10.4 and inventory markers (plan:28, :47, :266, :267, :280).

    • The D9 maintainer cell is "—", though D9 is a [decision] everywhere else.
    • The D10 cell omits K7–K16 and includes D11's p_cov.
    • The inventory says K7–K17, while D10 and §0.2 say K7–K16. §10.4 lists the whole range as decisions, though only part of it is.
    • B2's spec-edit column is "—", though B2 now writes a §8.9 sentence.

11. A6 on local R2024a (plan:119).

    • A local R2024a run measures cross-release drift, not the canonical R2025a drift the tolerance must cover, and §10.1 flags R2024a as churny. The residuals against the committed bytes are measurable in a cloud session.
    • "The only items that need local MATLAB" contradicts §10.3 and §10.4: the A5 and D3–D6 rows have LOCAL cells, and interim bytes are listed there too.
    • Separately, da5a625 is "the only 'born at' row" at the boundary, not the only row: 8a1c436 (ltv_cosmic 1e-10 → 1e-6) also dates from 2026-04-08.

4. Follow-up suggestions (nits)

• plan:68: "(analysis F1, F2, U1, U5)" is ambiguous, because the analysis uses F1/F2 in both §4.1 and §5.1. Write "§5.1 F1, F2; §5.3 U1, U5".
• plan:85 (A10): tests.yml:62–65 records that snap channels are not version-pinnable. Name a pinnable source, such as a versioned container image or a snap revision.
• plan:87 (D3): the all-NaN result is not silent. It carries a misleading "some frequencies" warning.
• E1a still leaves two choices open: where the spec history goes, and whether to delete or mark Appendix B. Either recommend one each, or note them as small decisions.
• PR body: the (new) list omits the A2 provenance-check row, the must-do list omits A10, and "about 20 issues" is 24.

5. Verdict

Request changes. Blockers 1–3 change what a reader would do or decide. The non-blockers can land in the same push.

Mechanical checks that ran:

• CI is green on c3946c9: both lint jobs and the strict site build pass, and the engine jobs are correctly gated off for a docs-only change.
• Inventory count: 43 rows, 24 PR groups.
• Python probes: the K3 last-block LinAlgError; two Monte-Carlo runs of the Welch σ_G ratio; residual on a q = 0 model with zeros(N, 0).
• An artifact download attempted from a cloud container (denied by the proxy).
• git log -S on a full 606-commit clone.
• The release tags read with git show.
• Every spec line cited above.

Not verified: MATLAB-proper behaviour, and the merge-method settings of the live "Main" ruleset.

---

Generated by Claude Code

[pdlourenco]

Thanks for the review. Every point is addressed in dcf8bf4. Point by point:

Blockers

1. Five §0.2 rows had wrong premises. All re-verified first-hand and corrected; the item text now matches.
   • K3 (revised): amend §8.3.4 so the rcond check covers every block the recursion inverts, Λ_0 … Λ_{N−1} including the last. It stays warn-and-return. The test asserts the warning in both ports, and a non-finite result only in Python and MATLAB proper (Octave's \ caveat).
   • D4 (revised): Φ̂_v is NaN in every estimator, and γ̂² stays 0, consistent with §3.2/§3.3 and A7's invariant.
   • D7 (revised): add a §10 identifier table first, then Python SidWarning maps to it and MATLAB is checked against it. The inventory ports column is now "both".
   • SP8: now its own row, recommending the §3.3 complex-total std (√2 × the current formula), with the magnitude-only exemption as the alternative. My own Monte-Carlo run reproduces the ratio at 2.15. The other SP items each get their own row with a reason.
   • p_cov / complex_stft: the rename is now a documented breaking change, EXAMPLES.md first, with an optional one-release alias.
2. E1a and D2 were mislabelled as decision-free.
   • The §8.4.3 tuning extras are no longer in E1a. A new K7–K16 row adds them only if "specify" is accepted.
   • The version bump and versioning rule moved to E1b and have their own row (recommended: adopt the EXAMPLES §6.1 semver rule, bump to 2.0.0).
   • The history location and Appendix B are listed as small §0.2 choices, so E1a carries none.
   • D2 is now [decision] with a §0.2 row: u = [] is an error only when q > 0. It is off the day-1 list, and the must-do set now needs five decision items.
3. A2 assigned the canonical bytes to an author who can't produce them.
   • New sub-decision, recommended (i-a): the PR's MATLAB job pushes the regenerated vectors to the same-repo PR branch with the App token and without [skip ci]. Forks fall back to (i-b), the maintainer committing the artifact.
   • The row now reads "no commit on main". §10.2 and §10.4 are aligned with it.

Non-blockers

4. Must-do criterion vs membership. I took the reword option.
   • The set is "check repairs plus critical defects". F3, F12, U2, F8/F9, K1–K4 and SP8 are named as the first backlog tier, and pulling them in is noted as your alternative.
   • A10 is added to the table, and the A7 gap is stated.
   • The fork-token guard moved into A1.
   • Day-1 lists: F1 items 2–8 on day 1, item 1 after A10.
   • Groups that mix must-do and backlog rows use a must-do label and milestone. The group count is corrected to 24.
5. A2 loose ends.
   • (a)/(b) and "decouple the SHA" are now conditional on (ii)/(iii).
   • The text states what (i) removes, the bot's always-bypass included, which is what meets the Phase-A exit criterion.
   • Added a push-to-main regenerate-and-diff check, which fails loudly on a stale base.
   • The provenance check is now PR-time ancestry against the PR head. Consumers reject only "unknown" or malformed SHAs, and the generator stamps the full SHA. Merge-commit-only stays as the alternative.
6. Other row premises.
   • D5's reason now says it differs from lti_freq_io's rule.
   • D6 uses trajMismatch, marked (new).
   • A9 is a W_atol in the generator.
   • E1b Cost (revised): keep the name and state the overload in §8.12.9, decided before A6.
   • The condition metric treats a non-finite block as rcond = 0.
   • B2's upper bound comes from a stated per-λ §8.9 requirement that rejects the S1 overstatement.
   • K7–K16 is split into four rows. The lti_freq_io discard gets a warning naming the dropped trajectories; the ports move to §8.11.2's >.
7. ADR amendments. Every "amendment" is now a superseding ADR, including in §10.3.
8. G2. Added ci: de-duplicate the tests.yml "Detect MATLAB-relevant change" step #203 and ci: release automation is silently suppressed by the reference bot's [skip ci] #204 rows, with the ci: release automation is silently suppressed by the reference bot's [skip ci] #204 / A2 coupling noted.
9. E1a/E1b enumerations. The E1a row is complete. The §6 factual corrections (2N·f(C), the §8.4.2 ½, the four uncertainty_derivation.md fixes) are appended.
10. Markers. D9 and D10 cells fixed, K7–K17 vs K7–K16 reconciled, B2's spec-edit column filled in.
11. A6. The canonical drift comes from repeated R2025a runs via workflow_dispatch; local R2024a only measures cross-release drift. Removed the "only local items" over-claim, and fixed the "born at" row wording (8a1c436 is a loosening, not a birth).

Nits: all applied. That covers the analysis section references, the pinnable Octave source, D3's misleading warning, the E1a small choices, and the PR body.

A second fresh-context pre-push review of the fixes found 10 leftovers, all fixed before push. The main one: the §9 graph still called D2 decision-free.

🤖 Generated with Claude Code

[pdlourenco]

Re-review of PR #206 at dcf8bf4

Posted as a comment review because this session's GitHub identity is the PR author. The verdict is: request changes (see §5). The remaining blockers are small, and one of them is mechanical.

Scope: fix commit dcf8bf4, read against the previous review of c3946c9, and the plan as a whole at the new head. Line numbers are docs/plans/2026-10-04-review-v2-remediation-plan.md at dcf8bf4, written plan:N.

Method:

• Twelve verifiers each checked one prior item, or the nits, against the plan text rather than the reply.
• Three hunters looked for regressions in the fix commit: A2 mechanics, the new §0.2 rows, and internal consistency.
• Two refuters checked every finding, and a critic went last.
• Every blocker below was re-checked first-hand.

1. Summary

The fix commit is a real response. Every prior blocker is addressed in the plan text, and most non-blockers are too:

• K3, D4 and D7 are corrected.
• SP8 has its own row.
• The p_cov rename is framed as a documented breaking change.
• D2 is a decision.
• A2 has the (i-a)/(i-b) owner split, plus the stale-base and PR-time ancestry checks.
• ADR amendments became superseding ADRs.

Four things keep it from merging:

1. CI never ran on dcf8bf4.
2. Two of the new §0.2 rows recommend an action on a premise the spec contradicts.
3. E1a and A1 are still scheduled as decision-free while the plan itself gates them on decisions.
4. A handful of §10 cells were not carried along with the revisions.

2. Status of the previous review

Prior item	Status at dcf8bf4
1 — K3, D4, D7, SP8, p_cov premises	fixed; residuals in §3 items 5 and 7
2 — E1a / D2 not decision-free	D2 fixed. E1a partly fixed: still picks a side and is still day 1 (blocker 3)
3 — A2 owner of canonical bytes	fixed; the (i-b) fallback needs an artifact step (item 4)
4 — must-do criterion and membership	fixed; A1 is the remaining gap (blocker 3)
5 — A2 loose ends	fixed; new mechanics gaps in item 4
6 — other §0.2 premises	fixed; the new K7–K16 rows introduce one false premise (blocker 2)
7 — ADR amendments	fixed; A2 also supersedes part of ADR-0005 (blocker 3)
8 — G2	fixed (nit on #203)
9 — E1a/E1b enumerations	fixed (nits)
10 — markers	fixed; some §10 cells are stale (item 6)
11 — A6	fixed in the item text; the §10.3 A6 row is stale (item 6)
Nits	applied

3. Issues to address before merge

Blockers

1. CI did not run on dcf8bf4.

   • The commit message says the MATLAB job "pushes to the PR branch without [skip ci]". GitHub skips every push and pull_request workflow when the head commit's message contains [skip ci] anywhere.
   • No workflow run exists for dcf8bf4, and the combined status is pending with zero checks. The two required lint checks therefore never reported, which is why the PR shows as blocked.
   • Any follow-up commit whose message does not contain the marker fixes it, for example "…without the skip-ci marker".
   • This is the same trap as #204, and A2 (i-a) will meet it again: the plan could add that commit messages must never quote the marker.

2. Two new §0.2 rows recommend an action on a false premise (CLAUDE.md §3–§4).

   • K7–K16 tuning extras, plan:115. The row says "ports follow §8.11.2's > rather than ≥ … a conformance fix, not a spec change".
     • §8.11.2 contradicts itself within one sentence: "Select λ* = max{λ : S(λ) > 0.90} — the largest λ for which at least 90 % of grid points are consistent" (SPEC.md:1435).
     • Both ports implement the prose, ≥ (ltv_disc_tune.py:405). Changing the ports to > therefore picks one half of a self-contradictory sentence, and it is a behaviour change in both ports.
     • Suggested: "§8.11.2 is self-contradictory (formula >, prose 'at least 90 %'); (new) make the formula ≥, which matches the prose and both ports, so no code changes; alternative: > and change both ports".
   • SP3, plan:107. The row says "remove the §5.4 floor claim | no port has the floor".
     • §5.4's floor, M_default = clip(floor(N/10), 2, 30) (SPEC.md:518), is BTFDR's, and both BTFDR ports implement it (freq_btfdr.py:211, sidFreqBTFDR.m:109–112). Followed literally, the row deletes an implemented contract.
     • The false part of §5.4 is only its side claim that the floor "matches the short-data default of sidFreqBT". BT has no floor, so M = 1 errors.
     • Suggested: "(new) keep §5.4's BTFDR floor; for BT, either give §2.1's default the same max(·, 2) floor (both BT ports change) or delete §5.4's 'matches sidFreqBT' clause. Recommended: the floor, because a default that errors on data §10.1 accepts (N ∈ [10, 19]) is a defect".

3. E1a and A1 are still scheduled as decision-free while the plan gates them on decisions (CLAUDE.md §4; the residue of the previous blocker 2).

   • E1a, ETFE WindowSize (plan:49, :180): stating it is not editorial. §4 says ETFE is BT with M = N − 1 (SPEC.md:431), and §9 defines WindowSize as M, but both ports report N (sidFreqETFE.m:375, freq_etfe.py:513). Writing either value picks a side. Move it to E1b with a §0.2 row, for example "(new) state N − 1 and change both ports; alternative: state N and reword §4's equivalence".
   • E1a, day 1 vs its two §0.2 small choices. §5 (plan:180), the §9 graph (:213) and the §9 prose (:218) call E1a "day 1, no decision". §0.2 (:120) and §10.4 (:291, :294) gate it on two maintainer choices, one of which creates spec/CHANGELOG.md. Say "E1a starts after its two small §0.2 choices", or move those two edits out of E1a.
   • A1 (plan:17, :73, :218): the new preamble (plan:77) itself lists A1 as a decision that changes an accepted ADR (ADR-0005). Yet A1 has no [decision] marker and no §0.2 row, and it "can start immediately". Add a §0.2 row, for example "accept the six required contexts and the fork-token guard", and change plan:73 and :218 to "once its ADR is accepted".
   • Relatedly, A2 consequence (4) removes the bot's always-bypass, which ADR-0005's Decision records. So A2 supersedes part of ADR-0005 as well as ADR-0002 (plan:18, :77).

Non-blockers

4. A2 option (i) mechanics (plan:82, :83, :129, :255).

   • "Semantic change" is too narrow. It is defined only as a payload outside tolerance, but the A-group items mostly change tolerance blocks, keys or encodings: A3 sentinels, A5 _atol fields, A6 Iterations and Cost. Under (i-a) those never count as semantic, so the bot never pushes them. Define a semantic change as "payload outside tolerance, or any change to a tolerance entry, key set or encoding".
   • Loop guard. The bot's push touches testdata/, so the MATLAB job runs again on its own head. That terminates only if every stored tolerance covers R2025a's run-to-run drift. State a guard, for example "never push when the head commit is the bot's own".
   • (i-b) needs an artifact. tests.yml has no artifact-upload step today, and A2's work cell does not add one. Also say where the maintainer commits for a fork PR.
   • Full 40-hex SHA. All 32 committed vectors carry 7-hex SHAs ("f9744bd"), so a consumer that rejects "malformed" SHAs rejects every existing vector. Accept 7–40 hex, or regenerate everything in the A2 PR.
   • "contents: write on main" does not exist. The grant is workflow-level and applies to GITHUB_TOKEN only (tests.yml:18–19). Under (i-a) the push uses the App token, so the mint step stays and the grant can go.
   • The author's branch falls behind after a bot push. Note "pull before pushing; never force-push over a bot commit", since agent sessions often amend.
   • Local R2024a bytes that sit within tolerance of R2025a are never replaced, so they land on main as final. Provenance records no engine. Record the engine in provenance, or rule out locally generated bytes.
   • A6's "repeated R2025a runs via workflow_dispatch" (plan:133) is not safe today. A dispatch regenerates, commits and pushes to main under [skip ci], and on a non-main ref it rebases onto main and pushes through the bypass (tests.yml:176–186). Run it only after A2 removes the push path, or add a no-commit input. plan:237 has the same issue.
   • §10.4's maintainer list lacks the ruleset re-application that the bypass removal needs. A2 says it closes #204, while G2 implements #204's option 1, so the tracking issue has two owners.

5. Other row premises.

   • Welch MIMO σ_G (plan:109). "No derivation behind it" is false: §3.6 gives the MIMO diagonal form BT uses, and the Welch ν substitution is the same one the SISO row recommends. Both ports also emit §3.3's Inf sentinel at degenerate bins (freq_map.py:197–200, sidFreqMap.m:553–554). Suggested: "NaN except the §3.3 Inf sentinel at degenerate bins; extending §3.6 is out of scope for now".
   • SP8 has nothing to pin it (plan:108, :50). reference_freqmap_welch stores no ResponseStd (its keys are Coherence, Frequency, NoiseSpectrum, Response_*, Time). Regenerating it changes nothing, and the √2 fix would ship with no cross-port check. Add ResponseStd to the vector.
   • Spec version (plan:104). EXAMPLES.md §6.1's table covers only example-suite changes, so "MAJOR under that rule" is not something the rule says. Write a SPEC-specific semver table instead; under it, changed outputs of a conforming implementation are MAJOR.
   • IO Cost (plan:105). It is not (n_iter × 1) on every path: trust-region stages append the best iterate's J, and the rank(H) = n fast path returns length 1. Say "the per-iteration cost history (length varies by path)". "Decide before A6" also makes this E1b decision a prerequisite of the must-do set, which the count at plan:73 omits.
   • K3 test (plan:98, :173). It asserts a non-finite Python result, but neither §8.3.4 nor the amendment says what is returned. Either specify it in the amendment, for example a NaN-filled estimate, or assert only the warning.
   • Tuning row (plan:115). sid:noConsistentLambda is not in the spec and Python's warning has no identifier, so "already a named diagnostic" takes MATLAB as the reference (principle 2); route it through D7's table. The default is min(floor(N/4), 256).
   • DoF row (plan:113). §8.9.3's first fallback, ν = Σ|L(k)| − N·d, cannot be positive when it is reached. So the second fallback is always the one hit, and the typed error would fire on every fallback; say so.
   • SP1 (plan:106). Both ports' ETFE constant-input warning still says "sigma_G to Inf" while returning NaN, and §10.3 states σ_G = Inf for every estimator. Codify it and fix the warning text and §10.3 together.
   • D2 (plan:89). N_data is undefined, and state data has N + 1 rows. compare (§15.4) requires u and has no [] default, so name §15.4. Give the MaxLag error an identifier, and take MaxLag out of D10's K list.
   • D4 (plan:41). The normative degenerate-input values live in §10.2/§10.3, so add them to the spec column.
   • First backlog tier (plan:63). It omits F4 (D4): MATLAB ETFE's max(PhiV, 0) turns NaN into 0, which is silent and rated major.
   • K7–K17 typed errors are called "contract additions" in D10 (plan:173) but carry no [decision], unlike D2's.

6. §10 cells not carried along with the revisions.

   • D7's §10.3 row (plan:278) is still Python-only, with no spec table and no MATLAB side. §10.4 (:291) still lists D7 as cloud-only.
   • E1b now changes both ports' numerics and a MATLAB-generated vector (SP8), yet §10.3 (:283) and §10.4 (:291) call it text-only and entirely cloud-capable.
   • The A6 row (plan:259) still puts the MATLAB-vs-Octave residual under LOCAL and describes the bytes arriving "on merge".
   • The A10 cell (plan:263) still assumes a snap channel.
   • §10.4 (:294) lists the canonical regeneration as maintainer-only, though (i-a) automates it; only (i-b) is a maintainer action.

7. Other consistency.

   • B2's item text (plan:144) does not match its revised §0.2 row (:88): it has no "≥ 1" requirement and no per-λ bounds.
   • Labels and milestones attach to issues, not checklist rows (plan:13). "F1 item 1" is not a row, so labelling the F1 row marks all eight items.
   • E1a's "scope §1's 'all functions'" gives no list. The analysis list is partly wrong: detrend and residual do accept 3-D input.
   • The analysis §6 Verified by: over-claims are in neither E1a nor B6.
   • The markers do not all follow the preamble's rule. A9 changed but is unmarked, and E1b Cost is marked (revised) although the item listed options without recommending one.

4. Nits

• D7 (plan:94): the five named identifiers sit in §2.3, §5.2, §8.12.4, §8.13.1 and §8.14, not "§6".
• D4: ETFE has no coherence output, so the γ̂² half applies to BT, BTFDR and Welch only.
• E1a: H⁻¹ is overloaded in SPEC.md §8.9.2, not in uncertainty_derivation.md. Deleting output.md Appendix B leaves §4.1's reference to it dangling. The E1a row still names only output.md §7.
• plan:13: "never as an edit" is too strong, because superseding does edit the old ADR's Status line.
• #203 lists two options without recommending either, so "the issue's own recommendation" (plan:118) does not hold.
• K4 needs no decision (plan:63). D7, D9 and D10 are missing from the phase-order sentence (plan:162). plan:128 has an unmatched closing parenthesis. (§9) at plan:67 reads as the analysis's §9.
• D11: p_cov appears in no .md (only code), and the notebook that uses complex_stft needs updating too.

5. Verdict

Request changes.

• Blocker 1 needs any new commit without the marker.
• Blockers 2 and 3 are a few rows each.
• The non-blockers can land in the same push. Items 4 and 6 matter most, because they are what an implementer of A2 would trip over.

Mechanical checks that ran:

• The workflow-run list for the branch: none for dcf8bf4.
• The combined status: pending, 0 checks.
• The spec and code lines cited above, read at dcf8bf4.
• Python's >= at ltv_disc_tune.py:405.
• The BTFDR floor in both ports.
• The keys of reference_freqmap_welch.
• The provenance SHA lengths of all 32 vectors.
• tests.yml's dispatch and push path.

Not verified: MATLAB-proper behaviour, and the live "Main" ruleset.

---

Generated by Claude Code

[pdlourenco]

Re-review addressed in c4b50fe.

Blockers

1. CI did not run. Correct: the dcf8bf4 message quoted the marker. The c4b50fe message does not contain it (checked with git log -1 --format=%B | grep -ci "skip ci" → 0). A2 now also states that the bot's commit messages must never contain the marker, not even quoted.
2. False premises.
   • Tuning: §8.11.2 is self-contradictory. The recommendation is now to make the formula ≥, which matches the prose and both ports (ltv_disc_tune.py:405, sidLTVdiscTune.m:278), so no code changes. Alternative: >, changing both ports.
   • SP3: keep §5.4's implemented BTFDR floor. Recommended: give BT's default the same max(·, 2) floor, changing both BT ports. Alternative: delete the "matches sidFreqBT" clause.
3. E1a and A1 scheduled as decision-free.
   • The ETFE WindowSize moved to E1b, with a §0.2 row: state N − 1 per §4's equivalence and change both ports.
   • E1a now starts after its two small §0.2 choices. I also revised the Appendix B choice: keep it, marked as unused. output.md §4.1 cites it for the composite solve, so deleting it would leave a dangling reference.
   • A1 gets a [decision] and a §0.2 row, and starts once its ADR is accepted.
   • A2 now supersedes ADR-0002 and ADR-0005's bot bypass.
   • The must-do count is now seven decision items, including E1b's IO Cost, which A6 depends on.

Non-blockers

4. A2 (i) mechanics.
   • "Semantic change" now covers a payload outside tolerance or any change to a tolerance entry, key set or encoding.
   • Loop guard: the job never pushes when the head commit is the bot's own.
   • A new artifact-upload step for (i-b). The maintainer commits to the fork branch or a follow-up PR.
   • Consumers accept 7–40 hex. The generator stamps the full SHA and the engine version from now on.
   • Provenance must name the canonical engine. The bot also replaces non-canonical (local) bytes, and authors are told not to commit locally generated bytes.
   • The contents: write wording is corrected: it is a workflow-level GITHUB_TOKEN grant, so the mint step stays.
   • Branch-behind note: pull before pushing, never force-push over a bot commit.
   • The dispatch hazard is noted in A6 and §10.1.
   • Ruleset re-application is added to §10.4. G2 owns ci: release automation is silently suppressed by the reference bot's [skip ci] #204.
5. Row premises.
   • Welch MIMO: NaN except the Inf sentinel at degenerate bins; extending §3.6 is out of scope for this cycle.
   • SP8 adds ResponseStd to the Welch vector.
   • Spec version: a SPEC-specific semver table, so 2.0.0 follows from that table rather than from EXAMPLES §6.1.
   • IO Cost: the per-iteration history, whose length varies by path.
   • K3: the amendment specifies that NaN is set explicitly, and both ports assert the warning and the NaN.
   • The tuning fallback identifier goes through D7's table; the default is min(⌊N/4⌋, 256).
   • DoF: the typed error replaces the fallback path entirely, as you described.
   • SP1: §10.3's wording and both ETFE warning texts are fixed together.
   • D2: N_data is defined, with N + 1 state rows. compare keeps u required per §15.4. New sid:badMaxLag over [1, N_data − 1]. MaxLag is out of D10's list.
   • D4: the spec column adds §10.2/§10.3.
   • F4 is added to the first backlog tier.
   • The K7–K17 typed errors are [decision]s, in the inventory, §0.2 and §10.
6. §10 cells. D7, E1b, A6 and A10 are updated, and §10.4's canonical-regeneration line is fixed.
7. Consistency.
   • B2's item text now matches its row.
   • Issues are split at the cut line instead of labelling rows: 27 issues from 24 groups.
   • The §1 scope comes from a per-function check, noting that detrend and residual accept 3-D input.
   • B6 carries the §6 over-claims.
   • Markers fixed: A9 is (revised), and E1b Cost is (new).

Nits: all applied. That covers the D7 section list, D4's ETFE coverage, H⁻¹ → §8.9.2, the ADR Status-line wording, #203's reason, the phase-order sentence, the unmatched parenthesis, "plan §9", and D11's naming locations (including the notebook).

A fresh-context pre-push review of this round found 8 leftovers, all fixed before push. The main one: a non-canonical-engine check without a replacement path would have left a PR red for good.

🤖 Generated with Claude Code

[pdlourenco]

Final check of PR #206 at c4b50fe

Posted as a comment review because this session's GitHub identity is the PR author. The verdict is: approve with one change (below).

Scope: fix commit c4b50fe, read against the re-review of dcf8bf4. Five verifiers checked the prior items against the plan text, two hunters looked for regressions, and every candidate went to two refuters with a materiality bar. I re-checked the blockers and the remaining finding first-hand.

Status

• CI is green on c4b50fe. Both lint jobs, the strict site build and both engine jobs pass, and the commit message carries no skip marker.
• Re-review blockers 2 and 3 are fixed everywhere they appear.
  • The tuning row recommends ≥. That matches §8.11.2's prose and both ports (ltv_disc_tune.py:405, sidLTVdiscTune.m:278), so no code changes.
  • SP3 keeps the implemented BTFDR floor.
  • ETFE WindowSize is an E1b decision.
  • E1a starts after its two small choices, consistently in §5, §9, §10.3 and §10.4.
  • A1 is a [decision] with a §0.2 row and starts once its ADR is accepted.
  • A2 also supersedes ADR-0005's bypass.
• Non-blockers 4–7 are fixed. That covers the A2 mechanics, the row premises, the §10 cells, the B2 text, the counts, and the "seven decision items" (A1, A2, A3, A6, A10, D2, E1b Cost). I found no new false premise among them.

One change before merge

D11 still claims four findings that are now E1b decisions (plan:48, plan:177).

• D11 lists "F10–F11, F13–F20" as decision-free minor items, and its only [decision] is the rename.
• This PR gives four of those findings their own E1b decision rows:
  • F14, ETFE WindowSize (plan:109)
  • F16, Welch MIMO σ_G (plan:111)
  • F18, Welch SISO σ_G (plan:110)
  • F19, the BT floor (plan:108)
• Someone working through D11's checklist would settle them without the decision. That is the same defect the re-review raised for E1a's WindowSize.
• Suggested, at both lines: "F10–F11, F13, F15, F17, F20 (F14, F16, F18 and F19 are E1b decisions)". Adjust F15 if it is meant to be E1a's L in §3.4/§3.5.

This is a one-line edit in two places and needs no further review round.

Optional

• The PR description still says "E1a is editorial and can start on day 1" and lists (revised) rows from the earlier round. Update it so the merge record matches the plan.

---

Generated by Claude Code

[pdlourenco]

Applied in 03378a5. D11 now lists F10–F11, F13, F17, F20 and U6–U8. F14, F16, F18 and F19 are noted as E1b decisions, and F15 as E1a's L in §3.4/§3.5, in both places (inventory and item text). The optional item is also done: the PR description is refreshed to match the final plan (E1a's start condition, the seven decision items, the correct (revised) list).

🤖 Generated with Claude Code