P3 (part 1): sidecar provenance, --check mode, in-repo Gate 1 figure, paper workflow (S-RP.1/3/4, S-RE.5, S-FD.4/5)

[mdenolle]

Advances #41 (package P3). Provenance and the real-data comparison figure; the release, archive and DOI items stay open in #41 until #46 is answered and the other packages merge. Independent of P1 (#52) and P4 (#53) except for one Discussion sentence and the Data availability paragraph, which will need a merge resolution against P1.

What is in this PR

Entry	What changed
S-RP.1	Every figure sidecar now records git_dirty (tracked files under src/ differ from the recorded commit) and generator_digest (package version plus the source of the figure-generating modules), beside git_commit. Documented in literature/figs/SOURCES.md and Data availability
Light CI	.github/workflows/paper.yml, on pushes and pull requests to the manuscript branch: (1) python -m codameter.figures --check --skip-slow --rtol 1e-5 regenerates the fast figures in memory and diffs every array against the committed sidecars; (2) paper/build_survey.py must regenerate survey.bib and appendix_table.tex without drift; (3) the PDF builds under quarto with TinyTeX and passes the text checks (??, ,%, <<), PDF uploaded as an artifact. The reviewer stays on demand. First run of this workflow is on this PR; the PDF job may need a toolchain iteration
S-RP.4	bench.score_cell rows carry golden._generator_hash() and the commit; check_shards refuses rows from different digests
S-RE.5, S-FD.4, S-FD.5	codameter.gate1.fig_gate1_comparison regenerates Fig 15 from the archived daily products and comparison.json: the corrected error band, the trailing 90-day matched rule after the 150-day burn-in, matched r and overlap printed per panel (0.99 / 0.90 / 0.83 on 579 / 357 / 446 days), all plotted arrays in the sidecar. The products are untracked, so the generator is skipped with a message where they are absent (codameter.errors.MissingInputs). Caption rewritten; the "caps the correlation near 0.7" sentence now quotes the centred-rule values (0.87 / 0.37 / 0.62) against the matched ones; Discussion and Data availability say which real-data figures are regenerated (the comparison) and which are committed as produced (interferograms, warm-up)
S-RP.3	Data availability names the three calibration invocations, their cost, and where each run's settings and seeds are stored

Not in this PR (still open in #41)

• Archive of the Gate 1 products with sha256 and a licence, the SCEDC and data-release citations with DOIs, the run commit and --use-case (blocked by Author input: Gate 1 run commit, --use-case, product rights and archive target (blocks P3) #46).
• Version bump, clean-tree tag with every figure and the calibration table regenerated at that tag, Zenodo deposit, CITATION.cff (after P0 to P6 merge).

Verification

• pytest tests/test_figures.py tests/test_bench.py: 18 passed (new: sidecar provenance fields, compare_sidecar differences, missing-input skip, Gate 1 registration, shard digest check).
• Fig 15 regenerated locally from the products; PDF rebuilt: 84 pages, ?? 0, ,% 0, << 0.
• --check --skip-slow on the committed sidecars: running locally at the time of opening; the workflow repeats it on Linux.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

A few reproducibility and manuscript-rendering issues remain (git-dirty semantics/documentation mismatch, a committed sidecar marked dirty, and a LaTeX table reference formatting bug).

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR advances package P3 (#41) by improving figure provenance (sidecar metadata), adding a --check mode to validate committed figure arrays, registering an in-repo Gate 1 comparison figure generator, and introducing a GitHub Actions workflow to continuously validate figures, survey outputs, and PDF builds on the manuscript branch.

Changes:

• Add sidecar provenance fields (git_dirty, generator_digest) and implement python -m codameter.figures --check to diff regenerated arrays against committed .npz sidecars.
• Add an in-repo Gate 1 comparison figure generator (codameter.gate1.fig_gate1_comparison) and register it as a data-dependent figure that can be skipped when inputs are absent.
• Add a manuscript-branch CI workflow (paper.yml) that runs the figure sidecar checks, survey regeneration drift check, and a PDF build + text checks.

File summaries

File	Description
src/codameter/figures.py	Adds git provenance fields, generator digest, --check mode, and skip-on-missing-inputs behavior.
src/codameter/gate1.py	New Gate 1 comparison figure generator that emits sidecar arrays/metadata.
src/codameter/errors.py	Adds shared MissingInputs exception for consistent skipping behavior.
src/codameter/bench.py	Records generator digest/commit in bench rows and enforces digest agreement in check_shards.
tests/test_figures.py	Adds tests for provenance fields, sidecar diff reporting, missing-input skip, and Gate 1 generator registration.
tests/test_bench.py	Adds tests for generator digest/commit on rows and digest mismatch rejection.
literature/figs/SOURCES.md	Documents new provenance fields and --check workflow behavior.
literature/figs/realdata_1_validation.json	Commits provenance for the regenerated Gate 1 comparison figure sidecar.
paper/manuscript_marine.qmd	Updates narrative/caption/data-availability text to match matched-rule comparison and reproducibility workflow.
paper/manuscript_marine.tex	Updates rendered manuscript text/caption; currently includes a LaTeX reference formatting issue.
paper/data/gate1/README.md	Updates Gate 1 README to reflect that the comparison figure is now generated in-repo with corrected bars.
.github/workflows/paper.yml	Adds CI workflow to validate figure sidecars, survey regeneration, and PDF build/text checks.

Review details

• Files reviewed: 12/14 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The LaTeX source currently prints a literal tilde in a table reference (Table\textasciitilde{}\ref{...}), which is likely to fail the new PDF text check and render incorrectly.

Review details

Suppressed comments (1)

paper/manuscript_marine.tex:1664

• Table\textasciitilde{}\ref{...} renders a literal ~ in the PDF (not a space/non-breaking space) and is likely to trip the workflow's Table~/Fig.~ text check. If the intent is a normal space before the reference, use Table\ \ref{...} (matches the .qmd source) instead of printing a tilde character.

against 0.99, 0.90 and 0.83 under the matched rule, even on a real

• Files reviewed: 12/14 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are correctness issues in the new Gate 1 generator path handling and in compare_sidecar()’s tolerance scaling that can cause incorrect behavior in valid use cases.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/codameter/gate1.py:38

• fig_gate1_comparison(gate1=...) suggests the generator can be pointed at a different paper/data/gate1 tree, but _rules() always reads comparison.json from the module-level GATE1. This makes the gate1 parameter only partially effective and can silently mix inputs from different directories.

This issue also appears on line 88 of the same file.

src/codameter/gate1.py:88

• cmp = _rules() ignores the gate1 argument passed to fig_gate1_comparison, so calling fig_gate1_comparison(gate1=...) will still read comparison.json from the default repo path.

    cmp = _rules()

• Files reviewed: 13/15 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

codameter.bench.check_shards() can incorrectly treat missing generator_hash as valid, and bench currently imports figures (and thus Matplotlib) just to read the git SHA, which should be decoupled.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/codameter/bench.py:386

• check_shards() can currently report complete even when generator_hash is missing from all rows (it becomes the string "None" and passes the single-digest check). For the new S-RP.4 provenance requirement, missing digests should be treated as an invalid shard set.
  src/codameter/bench.py:201
• Importing codameter.figures here pulls in matplotlib at module import time, which makes codameter-bench heavier/slower to start (and couples benchmark runs to the figures module) just to record the git SHA. Prefer an in-module git rev-parse helper (or a small shared util module) so bench provenance doesn’t depend on figures/Matplotlib.

• Files reviewed: 13/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mdenolle]

CI on 9094f94: all three jobs of the new paper workflow pass (sidecar check on Linux after the rounding-noise tolerance, survey drift, PDF build with text checks).

[mdenolle]

Manuscript text in this PR no longer names scripts, modules, command lines or file paths (the paper is about the method, not the software); the package name and the repository URL remain in Data availability. The pre-existing mentions in the base text are removed in #52, so #52 should merge first; #53 and #54 strip only what they had added.

[Copilot]

🔵 Needs a closer look

compare_sidecar()’s relative-difference diagnostic is inconsistent with its tolerance semantics, and _git_dirty()’s docstring does not match what is actually checked.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/codameter/figures.py:426

• compare_sidecar() reports a "max relative difference" but computes it relative to the stored array magnitude only (abs(a)), ignoring abs(b). When a is near zero and b is not, this can produce a misleadingly huge relative-difference diagnostic even though the tolerance logic is based on the max magnitude from either array; include abs(b) in the denominator to match the comparison semantics.
  src/codameter/figures.py:91
• _git_dirty() is documented as checking tracked files under src/, but the actual pathspec is the package directory (Path(__file__).resolve().parent, i.e. src/codameter). This makes the provenance description misleading if additional tracked sources are ever added under src/ outside the package directory; update the docstring to match what is checked.

• Files reviewed: 13/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mdenolle]

Addressed in 34ed205: the git helpers live in codameter.provenance (no plotting import; bench reads the SHA from there), and arrays differing only in NaN placement produce one message without a RuntimeWarning (test added). Note that matplotlib was already reached by bench through the synthetic generator before this PR; the figure-driver import is gone regardless.

[Copilot]

🔵 Needs a closer look

fig_gate1_comparison(gate1=...) currently ignores the passed gate1 path for comparison.json and can raise uncaught FileNotFoundError instead of being skipped via MissingInputs.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/codameter/gate1.py:90

• fig_gate1_comparison(gate1=...) does not actually use the passed gate1 path for comparison.json: it calls _rules() which is hard-wired to the module-level GATE1. This makes gate1 overrides inconsistent (station series come from the passed path, but rules/stats come from the default), and it also means missing comparison.json raises FileNotFoundError instead of being skipped as MissingInputs (the caller only catches MissingInputs).

• Files reviewed: 14/16 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mdenolle]

Addressed in fb8ee58: comparison.json is read from the passed gate1 path and its absence raises MissingInputs, so the generator is skipped rather than failing (test added). The committed figure and sidecar are unchanged.

[Copilot]

🔵 Needs a closer look

It introduces new provenance primitives, figure generation/checking semantics, and a new CI workflow, and at least one correctness issue was identified that should be addressed before approval.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/codameter/bench.py:386

• check_shards() stringifies git_commit values, so missing commits become the literal string "None" in the returned git_commits list. That’s misleading for downstream reporting/JSON (it should be null or omitted).
  src/codameter/provenance.py:34
• git_commit() is called in hot paths (e.g., bench.score_cell per cell). As written, it runs a git subprocess every time; caching the result once per process would avoid significant overhead during large sweeps while preserving semantics (HEAD commit does not change during a run).

• Files reviewed: 14/16 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mdenolle]

Addressed in 6977a16: git_commit is cached per process (HEAD does not change during a sweep), and check_shards lists only the commits that are present instead of stringifying None (test extended).

[Copilot]

🔵 Needs a closer look

It changes CI behavior and core reproducibility tooling (figure provenance, sidecar diffing, and real-data figure generation), which merits final human verification of end-to-end behavior and workflow stability.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/codameter/figures.py:100

• generator_digest() re-reads and hashes multiple module source files on every call; during build_all_figures() this happens once per figure even though the digest is constant for the life of the process. Caching the computed digest avoids repeated disk I/O and makes sidecar generation cheaper.
  src/codameter/provenance.py:43
• git_dirty() runs git status each time it’s called; when building multiple figures this can spawn many redundant subprocesses even though the working tree can’t change during a single run. Caching the result (like git_commit()) avoids repeated process overhead.

• Files reviewed: 14/16 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mdenolle]

Addressed in 5b1ec85: generator_digest and git_dirty are cached per process like git_commit.

[Copilot]

🔵 Needs a closer look

git_dirty() uses an absolute git pathspec that can cause provenance detection to silently fail even inside a normal checkout.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/codameter/provenance.py:55

• git_dirty() passes an absolute pathspec (str(_HERE.parent)) to git status. Git pathspecs are evaluated relative to the work tree; an absolute path can fail to match and cause _git(...) to return None even inside a normal checkout, making git_dirty silently lose information.

• Files reviewed: 14/16 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mdenolle]

Addressed in 5a71e42: the pathspec is now relative (.. from the package directory, i.e. src/); verified true with a modified tracked file and false on a clean tree.

[Copilot]

🔵 Needs a closer look

There are a couple of concrete issues (including a confirmed per-cell performance hotspot in bench.score_cell() and a documentation inconsistency) that should be addressed before approval.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/codameter/bench.py:222

• score_cell() calls golden._generator_hash() for every scored cell, which rereads multiple source files each time (see golden._generator_hash() implementation). In large benchmark sweeps this adds avoidable I/O and CPU overhead; the generator hash is process-constant, so cache it once and reuse the cached value.
  literature/figs/SOURCES.md:44
• This section now explains that realdata_1_validation is generated in-repo, but the final sentence still says “these three figures cannot be regenerated”, which contradicts the updated description (only the two remaining real-data figures are not reproducible from this checkout).

• Files reviewed: 14/16 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mdenolle]

Addressed in 87bb9d6: the golden generator hash is computed once per process in the benchmark, and the last sentence of SOURCES.md now says that only the two interferogram-based figures cannot be regenerated from a checkout.

[Copilot]

🟢 Approval recommended

The changes are internally consistent, well-covered by targeted tests, and align the code, documentation, and CI checks around reproducible figure and manuscript generation.

Review details

• Files reviewed: 14/16 changed files
• Comments generated: 0 new
• Review effort level: Lite