Skip to content

Report the scientific decisions a PR touches - #926

Merged
cailmdaley merged 3 commits into
developfrom
feat/decisions-touched-report
Oct 1, 2026
Merged

cailmdaley merged 3 commits into
developfrom
feat/decisions-touched-report

Conversation

@cailmdaley

@cailmdaley cailmdaley commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

A PR that changes the logic at a site governed by a decision in astra.yaml passes CI silently when no pinned Values: entry moves, even if the record's rationale is now false. This makes it visible without gating.

  • python -m tests.helpers.decisions --diff <base> lists each decision whose @sc-tagged site overlaps the diff (against the merge base; deleted sites are mapped on the old side and marked (base); renames are not changes). Each decision is one bullet saying whether its record changed in the same diff, with the overlapping sites nested under it. Decisions whose record is unchanged come first, since those are the ones where someone should check that the rationale still holds.
  • CI appends the list to the job summary on every PR. A failure of the report step writes the error into the summary and never fails the job.
  • One sentence in CLAUDE.md points at it.

Tests: tests/unit/test_decisions_diff.py builds throwaway git repos covering edits, deletions, a moved base, and renames/binaries. Each was seen to fail under an injected bug, including a test that unchanged records are listed first.

Example (--diff f477c410~5, trimmed):

- `catalogue_assembly.tile_overlap_handling` — record unchanged; check the rationale still holds
  - `src/shapepipe/modules/make_cat_package/make_cat.py:100-146`
- `shape_measurement.ngmix_seed_mode` — record unchanged; check the rationale still holds
  - `src/shapepipe/modules/ngmix_package/ngmix.py:449-501`
  - `src/shapepipe/modules/ngmix_package/ngmix.py:1241-1440`
- `catalogue_assembly.star_galaxy_classification` — record amended
  - `src/shapepipe/modules/make_cat_package/make_cat.py:143-212` (base)
  - `src/shapepipe/modules/make_cat_runner.py:30-111`

Sites that tag a whole function (e.g. ngmix_runner.py:37-227) flag any edit inside it; tightening those is a record-side change for later.

Claude Opus 5.5 on behalf of Cail

🤖 Generated with Claude Code

cailmdaley and others added 2 commits September 30, 2026 03:55
`python -m tests.helpers.decisions --diff <base>` prints a Markdown table
of the decisions whose `@sc`-tagged sites overlap `git diff <base>...HEAD`,
and whether each decision's `astra.yaml` block changed in the same diff
("record amended" / "record unchanged — check the rationale still holds").

Changed lines are matched against sites parsed with the existing tag
parser (now also callable on text, `_parse_source_tags`): old-side hunks
against the file at the merge base, so deleted governed sites are seen;
new-side hunks against HEAD. Both the old side and the record comparison
use the merge base, not the base tip, so work landed on the base branch
since the fork does not shift line mapping or read as an amendment.
Renames are detected; binary files have no hunks.

On pull requests, CI runs it in the built image against the PR base and
appends the table to the job summary. It is a report, never a gate: if
the command fails, its error goes to the summary and the job continues.
The checkout fetches full history so the base SHA is available.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cailmdaley

Copy link
Copy Markdown
Contributor Author

This looks good, but i don't think it needs to be a table. i think a set of bullet points would be more productive and easier to read.

@cailmdaley

Copy link
Copy Markdown
Contributor Author

Done: the report is now one bullet per decision, with its tagged sites nested underneath. Decisions whose record is unchanged come first, since those are the ones to check. The PR description shows an example, and CI is green.

Claude Opus 5.5 on behalf of Cail

@cailmdaley
cailmdaley merged commit 0de51d3 into develop Oct 1, 2026
2 checks passed
@cailmdaley
cailmdaley deleted the feat/decisions-touched-report branch October 1, 2026 02:04
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.

1 participant