From 109b77b0c72f0529ef97ab305315a24e3375644c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 17:09:20 +0000 Subject: [PATCH 1/2] shape-doctor.py: the `placement` row asks a leg's top-level `single-repository.yaml` the staying-single record's question only where `kind:` makes it the record, so one declaring `release-config` is asked the general question as `plan` asks it, and an answered entry names that rule (#691) `audit_leg` walked a leg with `adopt.walk`, whose rule for the record is by name alone, so a leg's `single-repository.yaml` declaring another kind was told it records that the repository stays single, and was answered under `ambiguous-single-repository-record`, on every run of a project adopted before #617 recorded answers under `answered_paths:`. `ask_about_a_record_only` (shape-doctor.py) reads the leg's file with `shape_advisory.read_record` and, unless it silences the advisory, reclassifies that entry with `policy.classify_file(path, skip=(adopt.RECORD_RULE,))`, as `adopt-project.py`'s function of that name does for `plan`. `adopt-project.py` is not changed. The comment at contracts/path-classification.yaml:443-446 now says the doctor asks the record's question of a leg's copy only where it declares the record's kind. Co-Authored-By: Claude Sonnet 5.5 --- contracts/path-classification.yaml | 11 ++-- shape-doctor.py | 41 ++++++++++++++- tests/test_adopt_single_repository_record.py | 54 +++++++++++++++++++- tests/test_doctor_recorded_answers.py | 27 ++++++++++ 4 files changed, 126 insertions(+), 7 deletions(-) diff --git a/contracts/path-classification.yaml b/contracts/path-classification.yaml index 389160c4..f2e6cc7f 100644 --- a/contracts/path-classification.yaml +++ b/contracts/path-classification.yaml @@ -441,14 +441,15 @@ rules: # the file blocks nothing the shape writes and no validator, gate or # doctor row reads it. As for the rules above, what `execute` does is said # of a repository WHEN IT IS ADOPTED: `shape-doctor.py --placement-plan` - # prints this question too, by name, for a leg already split, and reads - # the leg's copy as a question, because the verdict's leg stays null. + # prints this question too, for a leg already split, and reads the leg's + # copy as a question, because the verdict's leg stays null. # # `kind:` DECIDES, NOT THE NAME, as it does for the advisory. `plan` asks # this question only of a file that declares `kind: - # single-repository-record` at the commit it plans. One of this name that - # declares anything else, or does not parse, is no record, and is asked - # the general question below as any other top-level data file is. + # single-repository-record` at the commit it plans, and the doctor only + # of a leg's copy that declares it on the disk (#691). One of this name + # that declares anything else, or does not parse, is no record, and is + # asked the general question below as any other top-level data file is. - id: ambiguous-single-repository-record paths: - "single-repository.yaml" diff --git a/shape-doctor.py b/shape-doctor.py index e4287748..84e58de2 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -5899,6 +5899,44 @@ def walked_row(found, role: str, rel: str) -> dict: } +def ask_about_a_record_only(adopt, policy, walked: list, mount: Path) -> list: + """`walked`, with a leg's top-level `single-repository.yaml` that is no + record asked the question the rules below the record's ask (#691). + + `kind:` DECIDES, NOT THE NAME, as `adopt-project.py plan` reads it + (`ask_about_a_record_only`, #528) and as the Triad advisory does + (`shape_advisory.read_record`). The policy's rule for the record + (`adopt.RECORD_RULE`) tells the person the file records that the + repository stays single, and offers to drop it. A leg's file of that name + declaring `kind: release-config` is no such record: `plan` asked the + general question of it, and this row asked the record's on every run, + and named the record's rule on the entry an `answered_paths:` row + answered. So the record's rule stands only for a file `read_record` + finds to be the record, read in the leg as it is on this disk; any other + -- unreadable, or not on this disk, too -- is classified as if the policy + held no such rule, which for a top-level YAML file is the general + question, asked as before. A leg with no entry that rule gave is not read + at all. + + THE VERDICT'S `leg` STAYS NONE EITHER WAY (#528), and the record's + `default:` is no more written here than it is for the question: what + changes is which question is asked, and which rule an answered entry + names, never the verdict. + """ + if not any(found.path == shape_advisory.RECORD_FILE + and found.rule == adopt.RECORD_RULE for found in walked): + return walked + state, _, _ = shape_advisory.read_record(mount) + if state == shape_advisory.RECORD_SILENCES: + return walked + return [adopt.Entry(found.path, policy.classify_file( + found.path, skip=(adopt.RECORD_RULE,)), + found.files, found.bytes) + if found.path == shape_advisory.RECORD_FILE + and found.rule == adopt.RECORD_RULE else found + for found in walked] + + def walked_findings(walked: list, role: str, rel: str, answered: str | None) -> tuple: """The walked paths of one leg in the wrong leg, the questions, and the @@ -6063,7 +6101,8 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, kept = [(path, size) for path, size in files if path not in gitlinks and not any(regex.match(path) for regex in patterns)] - walked = adopt.walk(policy, kept) + walked = ask_about_a_record_only(adopt, policy, + adopt.walk(policy, kept), mount) submodules, wrong = leg_submodules(policy, role, rel, held, record) answered = gitmodules_the_record_answers(adopt, submodules, unwritable) misplaced, review, settled = walked_findings(walked, role, rel, answered) diff --git a/tests/test_adopt_single_repository_record.py b/tests/test_adopt_single_repository_record.py index 1acd24b0..721baf9e 100644 --- a/tests/test_adopt_single_repository_record.py +++ b/tests/test_adopt_single_repository_record.py @@ -18,7 +18,8 @@ unless a person writes `root`. The verdict keeps `leg` None, so the doctor, which walks a leg with the same `walk`, still reads a leg's copy as a question. And `kind:` decides, not the name: a file of that name that is no -record at the commit `plan` reads is asked the general question. +record at the commit `plan` reads is asked the general question, and so is +a leg's copy that is no record on the disk the doctor reads (#691). """ from __future__ import annotations @@ -356,6 +357,57 @@ def test_the_doctor_reads_a_legs_copy_as_a_question_and_not_misplaced( assert row["review_required"] is True +# --- the doctor asks the record's question of a record only (#691) ----------- + +@pytest.fixture(scope="module") +def doctor(): + return load_without_bytecode(REPO / "shape-doctor.py", + "shape_doctor_single_repository_record_leg") + + +def leg_audited(adopter, doctor, tmp_path, text: str) -> tuple: + """`audit_leg`'s `(paths in the wrong leg, questions)` for a code leg + that holds `RECORD_FILE` with `text`, and the source file beside it.""" + make_source_repo(tmp_path / "code", edits=(), tree={ + "src/main.c": "int main(void) { return 0; }\n", RECORD_FILE: text}) + _, misplaced, review, _, _ = doctor.audit_leg( + adopter, PathPolicy.load(POLICY), [], "code", "code", tmp_path, {}, + {}) + return misplaced, review + + +@pytest.mark.parametrize("body", list(NOT_A_RECORD.values()), + ids=list(NOT_A_RECORD)) +def test_the_doctor_asks_a_leg_file_that_is_no_record_the_general_question( + adopter, doctor, tmp_path, body): + """`plan` asks the general question of a file of that name that is no + record, and so does the placement row: it does not tell the person the + file records that the repository stays single.""" + misplaced, review = leg_audited(adopter, doctor, tmp_path, body) + row, = review + general = PathPolicy.load(POLICY).classify_file("release.yaml") + assert misplaced == [] + assert (row["path"], row["rule"], row["classified_as"]) == ( + f"code/{RECORD_FILE}", GENERAL, None) + assert row["review_required"] is True + assert row["question"] == general.question + assert row["reason"] == general.reason + + +@pytest.mark.parametrize("body", [RECORD, "kind: single-repository-record\n"], + ids=["a full record", "the kind alone"]) +def test_the_doctor_keeps_the_records_question_for_a_record( + adopter, doctor, tmp_path, body): + """`kind:` decides, so a leg's copy that declares the record's kind, with + a field left out or none, is asked the record's question, whose text + names its default, as before.""" + _, review = leg_audited(adopter, doctor, tmp_path, body) + row, = review + assert (row["rule"], row["classified_as"]) == (RULE, None) + assert row["question"] == record_question() + assert "(drop, the default" in row["question"] + + # --- what each answer does ---------------------------------------------------- def adopted(source, tmp_path, leg: str | None): diff --git a/tests/test_doctor_recorded_answers.py b/tests/test_doctor_recorded_answers.py index 32c5ede0..e1c002d5 100644 --- a/tests/test_doctor_recorded_answers.py +++ b/tests/test_doctor_recorded_answers.py @@ -66,6 +66,13 @@ RULES_ANSWER = "editor rules the code leg's tooling reads" #: A golden run, as two files the policy cannot call (`ambiguous-examples`). GOLDEN = ("examples/golden/case.json", "examples/golden/case.expected.json") +#: A top-level file named as the staying-single record is, and what `kind:` +#: makes of it (#691): the record the policy asks its own question of, or a +#: file that is not one, which is asked the general question. +RECORD_FILE = "single-repository.yaml" +RECORD_KINDS = [ + ("kind: single-repository-record\n", "ambiguous-single-repository-record"), + ("kind: release-config\n", "ambiguous-top-level-yaml")] MOVED_CLAUSE = ("1 submodule(s) moved out of the leg project.yaml's " "`submodules:` records for them, which no longer holds them " "(#525): spec/vendor/rules (recorded in the code leg)") @@ -213,6 +220,26 @@ def test_an_answer_no_leg_holds_is_silent(standard, project): COMPLIANT, "ok", before["reason"]), row +@pytest.mark.parametrize("body, rule", RECORD_KINDS, + ids=["a-record", "not-a-record"]) +def test_an_answered_single_repository_yaml_names_the_rule_that_asked_it( + standard, project, body, rule): + """THE RULE IS THE QUESTION'S, AND `kind:` DECIDES WHICH QUESTION (#691). + A leg's file of this name answered in the code leg is listed under + `answered_paths` with the rule that asked it of that file: the record's + where it is the record, and the general one where it declares anything + else, as `adopt-project.py plan` asks it.""" + stage(project / "code", RECORD_FILE, body) + record_answers(project, (RECORD_FILE, "code", "kept in the code leg")) + result, _, row = placement(standard, project) + assert result.returncode == 0, result.stdout + result.stderr + assert row["status"] == "ok", row + [answered] = row["detail"]["answered_paths"] + assert (answered["path"], answered["rule"]) == ( + f"code/{RECORD_FILE}", rule), answered + assert f"code/{RECORD_FILE} (rule {rule})" in row["reason"], row["reason"] + + # --- L1-08: a submodule moved, and one removed ------------------------------ def moved_rules(project: Path, *, gitmodules: bool = False) -> None: From 79e862caffa2406d5b687cdb95dd065de81126c1 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 23:12:18 +0000 Subject: [PATCH 2/2] shape-doctor.py: the `placement` row calls `adopt-project.py`'s own `ask_about_a_record_only` over each leg's index, so a leg's `single-repository.yaml` is asked the record's question where the index holds a record, and a link or a file changed or deleted on disk is read as `plan` reads it (#691) RV-e's round on #734. The first commit gave the doctor a copy of `plan`'s `ask_about_a_record_only` under the same name, which made the doctor's own statements that it carries no second copy of the classification false, and read the leg's disk where the row lists its index: a tracked link was followed, including outside the root, and a record deleted or edited on disk and not staged was asked the other question than `plan` asks. `audit_leg` now calls `adopt.ask_about_a_record_only(policy, adopt.walk( policy, kept), adopt.Source(mount, None, "", ""))`. That function reads only `source.path` and `source.commit`, and a commit of `""` is git's `:`, the index `leg_tracked_files` lists, so a link is read as its blob and never followed. The copy in shape-doctor.py is deleted and `adopt-project.py` is not changed. The comment in contracts/path-classification.yaml says the doctor reads the leg's index. tests/test_adopt_single_repository_record.py: the leg's tree holds `widgets/` beside the source file, so a reclassification of every entry adds a second question; the reclassified and the kept entry assert their `files` and `bytes`; a record deleted on disk, a record edited to another kind and a non-record edited to a record, none staged, are read as the index holds them; and a tracked link to a record beside the leg is asked the general question, skipped where a link cannot be made. Co-Authored-By: Claude Sonnet 5.5 --- contracts/path-classification.yaml | 6 +- shape-doctor.py | 52 +++---------- tests/test_adopt_single_repository_record.py | 79 ++++++++++++++++++-- 3 files changed, 88 insertions(+), 49 deletions(-) diff --git a/contracts/path-classification.yaml b/contracts/path-classification.yaml index f2e6cc7f..1be1c818 100644 --- a/contracts/path-classification.yaml +++ b/contracts/path-classification.yaml @@ -447,9 +447,9 @@ rules: # `kind:` DECIDES, NOT THE NAME, as it does for the advisory. `plan` asks # this question only of a file that declares `kind: # single-repository-record` at the commit it plans, and the doctor only - # of a leg's copy that declares it on the disk (#691). One of this name - # that declares anything else, or does not parse, is no record, and is - # asked the general question below as any other top-level data file is. + # of a leg's copy that declares it in the leg's index (#691). One of this + # name that declares anything else, or does not parse, is no record, and + # is asked the general question below as any other top-level data file is. - id: ambiguous-single-repository-record paths: - "single-repository.yaml" diff --git a/shape-doctor.py b/shape-doctor.py index 42058858..8d7ec170 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -6242,44 +6242,6 @@ def walked_row(found, role: str, rel: str) -> dict: } -def ask_about_a_record_only(adopt, policy, walked: list, mount: Path) -> list: - """`walked`, with a leg's top-level `single-repository.yaml` that is no - record asked the question the rules below the record's ask (#691). - - `kind:` DECIDES, NOT THE NAME, as `adopt-project.py plan` reads it - (`ask_about_a_record_only`, #528) and as the Triad advisory does - (`shape_advisory.read_record`). The policy's rule for the record - (`adopt.RECORD_RULE`) tells the person the file records that the - repository stays single, and offers to drop it. A leg's file of that name - declaring `kind: release-config` is no such record: `plan` asked the - general question of it, and this row asked the record's on every run, - and named the record's rule on the entry an `answered_paths:` row - answered. So the record's rule stands only for a file `read_record` - finds to be the record, read in the leg as it is on this disk; any other - -- unreadable, or not on this disk, too -- is classified as if the policy - held no such rule, which for a top-level YAML file is the general - question, asked as before. A leg with no entry that rule gave is not read - at all. - - THE VERDICT'S `leg` STAYS NONE EITHER WAY (#528), and the record's - `default:` is no more written here than it is for the question: what - changes is which question is asked, and which rule an answered entry - names, never the verdict. - """ - if not any(found.path == shape_advisory.RECORD_FILE - and found.rule == adopt.RECORD_RULE for found in walked): - return walked - state, _, _ = shape_advisory.read_record(mount) - if state == shape_advisory.RECORD_SILENCES: - return walked - return [adopt.Entry(found.path, policy.classify_file( - found.path, skip=(adopt.RECORD_RULE,)), - found.files, found.bytes) - if found.path == shape_advisory.RECORD_FILE - and found.rule == adopt.RECORD_RULE else found - for found in walked] - - def walked_findings(walked: list, role: str, rel: str, answered: str | None) -> tuple: """The walked paths of one leg in the wrong leg, the questions, and the @@ -6496,6 +6458,13 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, the record answers into this leg, which are returned apart, and `unslashed_rows` names the rows for this leg that name a directory without its `/`. + + AND A LEG'S `single-repository.yaml` IS ASKED THE RECORD'S QUESTION ONLY + WHERE IT IS THE RECORD (#691). The walk asks it by name; `plan`'s own + `ask_about_a_record_only` reads the file's `kind:` and gives any other + the general question. It is called here over the leg's INDEX, which is + what this row lists, so a file edited or deleted on disk and a link are + read as `plan` reads them. """ entry = {"role": role, "path": rel} mount = root / rel @@ -6532,8 +6501,11 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, kept = [(path, size) for path, size in files if path not in gitlinks and not any(regex.match(path) for regex in patterns)] - walked = ask_about_a_record_only(adopt, policy, - adopt.walk(policy, kept), mount) + # `plan`'s own `ask_about_a_record_only` (#528, #691), over the leg's + # INDEX -- `""` is git's `:`, the index `leg_tracked_files` + # lists -- so a link is read as its blob and never followed. + walked = adopt.ask_about_a_record_only( + policy, adopt.walk(policy, kept), adopt.Source(mount, None, "", "")) submodules, wrong = leg_submodules(policy, role, rel, held, record) answered = gitmodules_the_record_answers(adopt, submodules, unwritable) misplaced, review, settled = walked_findings(walked, role, rel, answered) diff --git a/tests/test_adopt_single_repository_record.py b/tests/test_adopt_single_repository_record.py index 721baf9e..06f6d476 100644 --- a/tests/test_adopt_single_repository_record.py +++ b/tests/test_adopt_single_repository_record.py @@ -19,12 +19,14 @@ which walks a leg with the same `walk`, still reads a leg's copy as a question. And `kind:` decides, not the name: a file of that name that is no record at the commit `plan` reads is asked the general question, and so is -a leg's copy that is no record on the disk the doctor reads (#691). +a leg's copy that is no record in the leg's index, which the doctor reads +(#691). """ from __future__ import annotations import importlib.util +import os import shutil import sys from argparse import Namespace @@ -42,6 +44,9 @@ needs_filter_repo = pytest.mark.skipif( shutil.which("git-filter-repo") is None, reason="git filter-repo is not installed: `pip install git-filter-repo`") +needs_disk_symlinks = pytest.mark.skipif( + sys.platform == "win32", + reason="a tracked symlink is a plain file in a Windows checkout") PROJECT = "Northwind" RULE = "ambiguous-single-repository-record" @@ -365,24 +370,39 @@ def doctor(): "shape_doctor_single_repository_record_leg") -def leg_audited(adopter, doctor, tmp_path, text: str) -> tuple: +def leg_audited(adopter, doctor, tmp_path, text: str, edit=None) -> tuple: """`audit_leg`'s `(paths in the wrong leg, questions)` for a code leg - that holds `RECORD_FILE` with `text`, and the source file beside it.""" - make_source_repo(tmp_path / "code", edits=(), tree={ - "src/main.c": "int main(void) { return 0; }\n", RECORD_FILE: text}) + that commits `RECORD_FILE` with `text`, a source file and a `widgets/` + the walk folds into one entry beside it, and then has `edit(leg)` done + to its disk. The folded entry is what a reclassification of EVERY entry + would turn into a second question.""" + leg = make_source_repo(tmp_path / "code", edits=(), tree={ + "src/main.c": "int main(void) { return 0; }\n", + "widgets/a.c": "int a;\n", "widgets/b.c": "int b;\n", + RECORD_FILE: text}) + if edit is not None: + edit(leg) _, misplaced, review, _, _ = doctor.audit_leg( adopter, PathPolicy.load(POLICY), [], "code", "code", tmp_path, {}, {}) return misplaced, review +def size_of_the_file(tmp_path) -> int: + """The bytes the leg's `RECORD_FILE` holds on this disk, which is what + the row counts. `write_text` made them, so on Windows they are not + `len(text.encode())`.""" + return len((tmp_path / "code" / RECORD_FILE).read_bytes()) + + @pytest.mark.parametrize("body", list(NOT_A_RECORD.values()), ids=list(NOT_A_RECORD)) def test_the_doctor_asks_a_leg_file_that_is_no_record_the_general_question( adopter, doctor, tmp_path, body): """`plan` asks the general question of a file of that name that is no record, and so does the placement row: it does not tell the person the - file records that the repository stays single.""" + file records that the repository stays single. Nothing else is asked, + and the entry keeps its one file and its bytes.""" misplaced, review = leg_audited(adopter, doctor, tmp_path, body) row, = review general = PathPolicy.load(POLICY).classify_file("release.yaml") @@ -392,6 +412,7 @@ def test_the_doctor_asks_a_leg_file_that_is_no_record_the_general_question( assert row["review_required"] is True assert row["question"] == general.question assert row["reason"] == general.reason + assert (row["files"], row["bytes"]) == (1, size_of_the_file(tmp_path)) @pytest.mark.parametrize("body", [RECORD, "kind: single-repository-record\n"], @@ -406,6 +427,52 @@ def test_the_doctor_keeps_the_records_question_for_a_record( assert (row["rule"], row["classified_as"]) == (RULE, None) assert row["question"] == record_question() assert "(drop, the default" in row["question"] + assert (row["files"], row["bytes"]) == (1, size_of_the_file(tmp_path)) + + +def deleted(leg) -> None: + (leg / RECORD_FILE).unlink() + + +def rewritten(text: str): + """An `edit` that rewrites the leg's `RECORD_FILE` on disk, unstaged.""" + return lambda leg: (leg / RECORD_FILE).write_bytes(text.encode()) + + +@pytest.mark.parametrize("committed, edit, rule", [ + (RECORD, deleted, RULE), + (RECORD, rewritten(NOT_A_RECORD["another kind"]), RULE), + (NOT_A_RECORD["another kind"], rewritten(RECORD), GENERAL), +], ids=["deleted-on-disk", "edited-to-no-record", "edited-to-a-record"]) +def test_the_doctor_reads_the_legs_index_and_not_its_disk( + adopter, doctor, tmp_path, committed, edit, rule): + """THE ROW LISTS THE LEG'S INDEX, so the file is read there, as `plan` + reads its commit: one that is tracked and gone from the disk, or edited + there and not staged, is asked as what it is in the index.""" + _, review = leg_audited(adopter, doctor, tmp_path, committed, edit) + row, = review + assert (row["rule"], row["classified_as"]) == (rule, None) + + +@needs_disk_symlinks +def test_a_tracked_link_to_a_record_is_asked_the_general_question( + adopter, doctor, tmp_path): + """A tracked link is read as its blob, the target's name, which is no + record, and never followed: not to a record beside the leg, which the + leg's own repository does not hold, as `plan` reads it.""" + (tmp_path / "outside.yaml").write_bytes(RECORD.encode()) + + def link(leg) -> None: + (leg / RECORD_FILE).unlink() + try: + os.symlink("../outside.yaml", leg / RECORD_FILE) + except (NotImplementedError, OSError) as exc: + pytest.skip(f"this platform cannot create a symlink here: {exc}") + git("add", "--", RECORD_FILE, cwd=leg) + + _, review = leg_audited(adopter, doctor, tmp_path, RECORD, link) + row, = review + assert (row["rule"], row["classified_as"]) == (GENERAL, None) # --- what each answer does ----------------------------------------------------