Skip to content

Comments that describe the present: sweep of files changed since August - #960

Open
cailmdaley wants to merge 50 commits into
developfrom
docs/comment-sweep
Open

cailmdaley wants to merge 50 commits into
developfrom
docs/comment-sweep

Conversation

@cailmdaley

Copy link
Copy Markdown
Contributor

Rewrites the comments, docstrings and in-repo prose of the files changed on develop since 1 August, so that they describe what the code does now and why. No code changes: the check at the bottom shows that the diff, with comments and docstrings stripped, is empty.

Why. Between August and October the workflow and modules changed fast through dozens of PRs, and their comments recorded the path: what an earlier version did, which PR fixed which campaign's failure, chains of "now / no longer / since #NNN". A reader arriving cold had to reconstruct that history to learn the present. After this PR, each comment states the present behaviour and the reason for it. Where history justified a choice (a measured failure a guard prevents), the reason stays as a present-tense fact with its measurement. Long essays at a site shrink to what that site needs, and a fact stated in several places keeps one home with pointers to it. Net −1,950 lines.

How it was done. An Opus chair split the 71 files into eight groups and wrote one style brief with before/after examples from this code. Eight GPT-6.1 Sol workers each rewrote one group in its own worktree. Two fresh Sol reviewers then read the merged diff as first-time readers. They checked every new statement against the code, looked for explanations lost in the cuts, and audited each new contract. Their ten findings (five false statements, three unclear, one contract demoted, one amended) are fixed here. They found no lost hazard or ordering explanation.

@sc practice. Where an existing comment already stated a load-bearing local constraint informally, it is now an @sc [label:…] contract (47 new, all reviewed as true and load-bearing; ids unique). No astra.yaml decision changed, and no existing @sc [decision:…] tag moved.

Checks run (candide, image shapepipe_develop-dev-20260926, this branch's src/ on PYTHONPATH): tests/unit 582 passed, 1 skipped (same as develop), including test_decisions.py and test_contracts.py. tests/workflow: 21 passed. uvx astra-tools@0.2.17 validate passes.

Merge order. This should land after #922, #925, #955, #954 and #957. It overlaps them in tile.smk, Snakefile, config.yaml, bin/sp, make_cat.py, clean_tile.py and completeness.py, and comment-only conflicts are cheap to resolve on this side. Not swept, left for a follow-up once their PRs land: ngmix.py, ngmix_runner.py, sextractor_script.py, sextractor_runner.py and match_catalogue.py (#922/#925 rewrite them), docs/source/pipeline_tutorial.md and the files #880 deletes.

Defects found along the way are filed rather than fixed here: #958 (workflow: sp run refreshes the code snapshot before Snakemake's lock, so even sp run -n against a live campaign rewrites the code its pending jobs read; plus two smaller defects) and #959 (latent defects outside the fiducial path).

Comments-only check: output
ok    .github/workflows/deploy-image.yml [yaml]
prose CLAUDE.md
ok    Dockerfile [hash]
prose README.rst
ok    conftest.py [py]
prose docs/source/clusters.md
prose docs/source/container.md
prose docs/source/dependencies.md
prose docs/source/installation.md
prose docs/source/testing.md
prose docs/source/workflow.md
prose example/cfis/README.md
prose example/cfis_image_sims/README.md
ok    example/cfis_image_sims/config_tile_PiViVi_canfar_sx.ini [hash]
ok    profiles/candide/config.yaml [yaml]
ok    profiles/nibi/config.yaml [yaml]
ok    pyproject.toml [hash]
ok    scripts/python/create_final_cat.py [py]
ok    scripts/python/run_breakdown_grid.py [py]
ok    src/shapepipe/modules/fake_interp_runner.py [py]
ok    src/shapepipe/modules/get_images_runner.py [py]
ok    src/shapepipe/modules/make_cat_package/make_cat.py [py]
ok    src/shapepipe/modules/mask_query_package/__init__.py [py]
ok    src/shapepipe/modules/mask_query_package/mask_query.py [py]
ok    src/shapepipe/modules/mask_query_runner.py [py]
ok    src/shapepipe/modules/mccd_package/shapepipe_auxiliary_mccd.py [py]
ok    src/shapepipe/modules/merge_sep_cats_package/merge_sep_cats.py [py]
ok    src/shapepipe/modules/merge_starcat_package/merge_starcat.py [py]
ok    src/shapepipe/modules/psfex_interp_package/psfex_interp.py [py]
ok    src/shapepipe/modules/setools_package/setools.py [py]
ok    src/shapepipe/modules/vignetmaker_package/vignetmaker.py [py]
ok    src/shapepipe/modules/vignetmaker_runner.py [py]
ok    src/shapepipe/pipeline/sqlite_store.py [py]
ok    src/shapepipe/testing/simulate.py [py]
ok    src/shapepipe/utilities/cfis.py [py]
ok    src/shapepipe/utilities/field_corners_extractor.py [py]
ok    src/shapepipe/utilities/final_cat.py [py]
ok    src/shapepipe/utilities/header_downloader.py [py]
ok    src/shapepipe/utilities/mask_query.py [py]
prose workflow/CONTRACTS
prose workflow/README.md
ok    workflow/Snakefile [smk]
ok    workflow/bin/sp [hash]
ok    workflow/config.yaml [yaml]
ok    workflow/config/cfis/config_MCCD.ini [hash]
ok    workflow/config/cfis/config_exp_mccd.ini [hash]
ok    workflow/config/cfis/config_tile_Mc.ini [hash]
ok    workflow/config/cfis/config_tile_PiViVi_mccd.ini [hash]
ok    workflow/config/cfis/config_tile_PiViVi_psfex.ini [hash]
ok    workflow/config/cfis/final_cat.param [hash]
ok    workflow/config/cfis/star_selection.setools [hash]
ok    workflow/config/cfis_image_sims/config_tile_PiViVi_fake.ini [hash]
ok    workflow/config/cfis_image_sims/final_cat.param [hash]
ok    workflow/rules/exposure.smk [smk]
ok    workflow/rules/prepare.smk [smk]
ok    workflow/rules/tile.smk [smk]
ok    workflow/scripts/build_forest.py [py]
ok    workflow/scripts/build_index.py [py]
ok    workflow/scripts/clean_exposure.py [py]
ok    workflow/scripts/clean_tile.py [py]
ok    workflow/scripts/completeness.py [py]
ok    workflow/scripts/container.py [py]
ok    workflow/scripts/exp_maps.py [py]
ok    workflow/scripts/hdf5_reconcile.py [py]
ok    workflow/scripts/merge_exposure_maps.py [py]
ok    workflow/scripts/merge_final_cat.py [py]
ok    workflow/scripts/merge_star_cat.py [py]
ok    workflow/scripts/ngmix_range.py [py]
ok    workflow/scripts/persist_exp.py [py]
ok    workflow/scripts/run_config.py [py]
ok    workflow/scripts/run_report.py [py]

71 files changed, 0 with code changes

For .py: ast.dump with docstrings removed. For Snakefile/.smk: the Python token stream without comments or statement-level strings. For YAML: parsed equality plus comment-stripped lines. For INI/param/sex/sh: comment-stripped lines. For .md/.rst/README/CONTRACTS: prose, always allowed. The check was seen to fail when tokens were changed in .py, .smk and .ini.

The checker script
#!/usr/bin/env python3
"""Prove a diff touches only comments, docstrings and prose.

Usage: comments_only.py <repo> <base-ref> [<head-ref>]   (head defaults to the working tree)

For every file changed between base and head:
  * .py                          -> ast.dump with docstrings removed must be equal
  * Snakefile, .smk, bin/sp      -> Python token stream without COMMENT/NL tokens and
                                    without statement-level string literals (docstrings)
  * .yaml/.yml                   -> yaml.safe_load equal AND comment-stripped lines equal
  * .ini .param .sex .psfex .conv .setools .sh .bash .cfg .toml Dockerfile and
    other '#'-comment formats     -> lines with '#' comments stripped (outside quotes) equal
  * .md .rst README* CONTRACTS *.txt -> prose, always allowed (listed)
Exits 1 if any non-prose file differs in code.
"""
import ast
import io
import subprocess
import sys
import tokenize
from pathlib import Path

PROSE_SUFFIX = {".md", ".rst", ".txt"}
PROSE_NAMES = {"CONTRACTS", "README", "AGENTS.md", "CLAUDE.md"}
SMK_LIKE = {"Snakefile"}


def git(repo, *args):
    return subprocess.run(["git", "-C", repo, *args], check=True,
                          capture_output=True, text=True).stdout


def read(repo, ref, path):
    if ref is None:
        p = Path(repo) / path
        return p.read_text() if p.exists() else None
    try:
        return git(repo, "show", f"{ref}:{path}")
    except subprocess.CalledProcessError:
        return None


def strip_docstrings(tree):
    for node in ast.walk(tree):
        if isinstance(node, (ast.Module, ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
            body = node.body
            if body and isinstance(body[0], ast.Expr) and isinstance(getattr(body[0], "value", None), ast.Constant) \
                    and isinstance(body[0].value.value, str):
                node.body = body[1:] or [ast.Pass()]
    return tree


def norm_py(text):
    return ast.dump(strip_docstrings(ast.parse(text)), include_attributes=False)


def norm_tokens(text):
    toks = list(tokenize.generate_tokens(io.StringIO(text).readline))
    out, prev = [], tokenize.NEWLINE
    for i, t in enumerate(toks):
        if t.type in (tokenize.COMMENT, tokenize.NL):
            continue
        if t.type == tokenize.STRING and prev in (tokenize.NEWLINE, tokenize.INDENT, tokenize.DEDENT):
            # statement-level string (docstring): drop it if the statement is only strings
            j = i
            while j < len(toks) and toks[j].type in (tokenize.STRING, tokenize.NL, tokenize.COMMENT):
                j += 1
            if j < len(toks) and toks[j].type in (tokenize.NEWLINE, tokenize.ENDMARKER, tokenize.DEDENT):
                continue
        if t.type == tokenize.STRING and out and out[-1][0] == tokenize.STRING and prev == tokenize.STRING and out[-1][1] is None:
            pass
        out.append((t.type, t.string))
        prev = t.type
    # collapse docstring-removal leftovers: NEWLINE runs
    clean = []
    for tok in out:
        if tok[0] == tokenize.NEWLINE and clean and clean[-1][0] in (tokenize.NEWLINE, tokenize.INDENT):
            continue
        clean.append(tok)
    return clean


def strip_hash(line):
    q = None
    for i, c in enumerate(line):
        if q:
            if c == q:
                q = None
        elif c in "'\"":
            q = c
        elif c == "#" and (i == 0 or line[i - 1].isspace()):
            return line[:i].rstrip()
    return line.rstrip()


def norm_hash(text):
    return [l for l in (strip_hash(x) for x in text.splitlines()) if l.strip()]


def kind(path):
    p = Path(path)
    if p.suffix in PROSE_SUFFIX or p.name in PROSE_NAMES or p.name.startswith("README"):
        return "prose"
    if p.suffix == ".py":
        return "py"
    if p.suffix == ".smk" or p.name in SMK_LIKE:
        return "smk"
    if p.suffix in (".yaml", ".yml"):
        return "yaml"
    return "hash"


def main():
    repo, base = sys.argv[1], sys.argv[2]
    head = sys.argv[3] if len(sys.argv) > 3 else None
    rng = [base, head] if head else [base]
    files = [f for f in git(repo, "diff", "--name-only", *rng).split() if f]
    bad = 0
    for f in files:
        k = kind(f)
        a, b = read(repo, base, f), read(repo, head, f)
        if a is None or b is None:
            print(f"FAIL  {f}: added or deleted"); bad += 1; continue
        if k == "prose":
            print(f"prose {f}"); continue
        try:
            if k == "py":
                same = norm_py(a) == norm_py(b)
            elif k == "smk":
                try:
                    same = norm_py(a) == norm_py(b)
                except SyntaxError:
                    same = norm_tokens(a) == norm_tokens(b)
            elif k == "yaml":
                import yaml
                same = yaml.safe_load(a) == yaml.safe_load(b) and norm_hash(a) == norm_hash(b)
            else:
                same = norm_hash(a) == norm_hash(b)
        except Exception as e:  # noqa: BLE001
            print(f"FAIL  {f}: {type(e).__name__}: {e}"); bad += 1; continue
        print(f"{'ok   ' if same else 'FAIL '} {f} [{k}]")
        bad += not same
    print(f"\n{len(files)} files changed, {bad} with code changes")
    sys.exit(1 if bad else 0)


if __name__ == "__main__":
    main()

Claude (Opus) and GPT-6.1 Sol on behalf of Cail.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JhKts6NHr9YjfnatBZPogK

cailmdaley and others added 30 commits October 10, 2026 18:42
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
… documentation

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
…ry overview

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
cailmdaley and others added 20 commits October 10, 2026 18:47
…ration prose

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
…config

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Correct five false or unclear statements, move five in-function @sc
contracts into their docstrings (the record checker accepts # tags only
at module level), and demote completeness-report-unit-key to prose.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhKts6NHr9YjfnatBZPogK

This branch has not been deployed

No deployments
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