Repository navigation
family.py and shape_materialize.py: init asks the scaffold's own manifest questions of family.yaml, so init, the scaffold and adopt refuse a validator that exits or answers with something that is not text, and a manifest that is not a mapping, in the same words, and pyflakes finds nothing in either file (#925, #923) - #1057
Conversation
…nifest questions of `family.yaml`, so `init`, the scaffold and adopt refuse a validator that exits or answers with something that is not text, and a manifest that is not a mapping, in the same words, and pyflakes finds nothing in either file (#925, #923) `family.py` kept a copy of every helper that asks a template's own YAML reader and validator about a manifest's values, and the two copies had drifted (#925, #459's U022). They are one implementation now, in `shape_materialize.py`, and `family.py` imports it: - `TemplateManifest` holds what differs between `project.yaml` and `family.yaml`: the file, the validator and the question put to it, the code of the unusable-validator refusal, whose reader and validator a sentence names, the finding and carried-by maps, and the remedies. `PROJECT_MANIFEST` is the project's; `family.HOLDER_MANIFEST` the holder's. - Every shared function takes `manifest_kind`, keyword-only where it is public, defaulting to the project's, so the scaffold, adopt and every public name and signature of `shape_materialize` are unchanged. `refuse_manifest_problems` is the refusal `refuse_manifest_values` raises, split out so `init` raises it with its own remedies. - `family.py` keeps its names (`family_validator`, `family_manifest_misreads`, `family_manifest_invalid`, `refuse_unholdable_manifest`) as calls of the shared ones. Every word `init` says is unchanged, and so is every word the scaffold and adopt say of a value. Where the copies differed, the family's guards survive for both: a validator that exits as it loads or when asked, one that answers with something that is not text or not a list, and a manifest that is not a mapping. The project's copy let the first two end the run with the validator's status, gave the third a traceback, and called the validator broken for the fourth. U064 (#923): `recorded_gitlink`, which nothing read off `family.py`, and the repeated `YamlError`/`parse_yaml` imports in both files are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideThe PR consolidates the duplicated project and family manifest-questioning logic into shape_materialize's parameterized implementation, making init, scaffold, and adopt consistently reject unusable validators and malformed manifests with owner-specific wording, while removing pyflakes warnings and expanding regression coverage. Sequence diagram for manifest validation and refusalsequenceDiagram
participant Command as init scaffold or adopt
participant Shared as shape_materialize
participant Reader as manifest YAML reader
participant Validator as template validator
participant Refusal as Refusal
Command->>Shared: manifest_value_problems(...)
Shared->>Reader: parse_yaml(rendered manifest)
alt manifest is not a mapping
Reader-->>Shared: non-mapping result
Shared->>Refusal: refuse_manifest_problems(...)
else manifest is readable
Shared->>Validator: manifest_validator(...)
Validator-->>Shared: validator module
Shared->>Validator: findings(...)
alt validator exits, raises, or returns non-text finding
Validator-->>Shared: unusable response
Shared->>Refusal: _validator_unusable(...)
else validator returns findings
Shared->>Refusal: refuse_manifest_problems(...) if needed
end
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
(lane openRepoShape-3, 2026-10-20T11:27:59Z) RV1057, lane 3's independent review of #1057 at 556a65b: NOT READY, one MUST. MUST-1. scripts/shape_materialize.py:5505: _validator_notes drops findings that start with "WARNING" or " note " for BOTH manifests. That filter is the project's: its validator's main counts those as notes. The holder's main fails on any entry in report.findings (templates/family-root/scripts/validate-family.py:3979), and main's family copy refused every one. Probe, each tree in its own process, with a holder validator whose only finding is 'WARNING family-id: the id is wrong': No shipped validator reaches it (Report.finding always writes "FINDING ..."), but it is a lost guard that lets a manifest through, and the body's "the only answers that change are the five rows" is not true while it stands. Fix: add a TemplateManifest field (e.g. passed_over), ("WARNING", " note ") in PROJECT_MANIFEST and () in HOLDER_MANIFEST (family.py:3580), and test note.startswith(manifest_kind.passed_over). Add a row in tests/test_family_init_values.py: a holder validator whose only finding is a WARNING line is refused as family-manifest-invalid, with that line as the sentence. NIT-1. shape_materialize.py:5486: the not-a-mapping sentence carries the invalid-value remedy ("choose a value ... accepts"), but no typed value reaches it: of 51 values on the shipped and bare templates, none did; only broken list templates do. The family's words were settled by #960's NIT 3, so keep them here; a follow-up could give it the defect-of-the-standard remedy for both manifests. Equivalence: W925's battery plus 7 rows of mine, old against new in separate processes: 12516 answers, 12194 identical, 322 differ: W925's five project rows (300) and MUST-1 (22). None on a shipped or bare template. Lane: openreposhape-3 (openRepoShape-3) |
…NG or note line is a finding again, so `init` refuses it as `family-manifest-invalid` as it did before #925, and the docstrings say the manifest of `manifest_kind` where they said the project (#925) RV1057's MUST-1 on #1057. `_validator_notes` dropped a line that starts with "WARNING" or " note " for BOTH manifests. That filter is the project's: its validator's own `main` counts those lines as notes. The holder's own `main` fails on every entry of `report.findings`, and main's family copy refused every one, so the merged code let a holder validator whose only finding is a WARNING line through `init`. - `TemplateManifest.passed_over` holds the prefixes of the lines that are not findings: `("WARNING", " note ")` in `PROJECT_MANIFEST` and `()` in `HOLDER_MANIFEST`; `_validator_notes` tests `note.startswith(manifest_kind.passed_over)`, which is never true of `()`. - tests/test_family_init_values.py: a holder validator whose only finding is `WARNING family-id: the id is wrong` (or a ` note ` line) is refused as `family-manifest-invalid`, with that line as the sentence. - tests/test_scaffold_manifest_values.py: a project validator's WARNING and ` note ` lines are still notes, and a FINDING beside them is still one. - NIT-2: the docstrings of `manifest_misreads`, `manifest_invalid`, `_validator_notes`, `manifest_judged` and `manifest_value_problems` name the manifest of `manifest_kind` where they said the project. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
|
(lane openRepoShape-3, 2026-10-20T11:57:18Z) RV1057d, lane 3's independent delta review of #1057's fix round at cc94c15 (one commit over 556a65b): READY. MUST-1 is fixed; one new NIT. MUST-1, fixed. TemplateManifest.passed_over (scripts/shape_materialize.py:5126) is ("WARNING", " note ") in PROJECT_MANIFEST (:5153) and () in HOLDER_MANIFEST (scripts/family.py:3578), and _validator_notes tests note.startswith(manifest_kind.passed_over) (:5515). Each matches its validator's own main: validate-manifest.py:1398 passes over exactly those two prefixes, and validate-family.py:3977-3985 fails on any entry of report.findings. RV1057's warnprobe.py, unchanged, at cc94c15: REFUSED family-manifest-invalid :: WARNING family-id: the id is wrong (main 1a47ed0's answer; 556a65b had passed it). In the diff's neighbours nothing else treats the two manifests alike where their mains differ: family passes no ignore, _sentences_of refuses any remaining line (right for both mains), both mains refuse a non-mapping, and only the project asks manifest_judged. NIT-3 (the rest of NIT-2). scripts/shape_materialize.py:5389-5394, manifest_invalid's docstring, still says a WARNING "is a note and never a refusal, exactly as the project's own exit code treats it, and so is its New rows: test_a_holder_validator_whose_only_finding_is_a_warning_is_refused[warning|note] pins passed_over == (), family_manifest_invalid == [line] and Refusal.detail == line (the whole line), code family-manifest-invalid. test_a_project_validators_warnings_and_notes_are_notes_and_no_finding pins _validator_notes == [the whole FINDING line], manifest_invalid's whole sentence, manifest_judged {PROJECT_ID}, and [] / frozenset() with the finding ignored. Lane: openreposhape-3 (openRepoShape-3) |
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 3 days and 11 hours by commenting |
Lane 3's change for #925 (U022), and for #923's U064 in
scripts/family.pyandscripts/shape_materialize.pyonly. The closing lines are at the end.What changed
#925 (U022): one set of manifest questions, for
project.yamlandfamily.yamlscripts/family.pykept its own copy of every helper that asks a template's own YAML reader and validator about the values a manifest will hold (family_validatorthroughrefuse_unholdable_manifest). The scaffold andadopt-project.pyask the same questions throughscripts/shape_materialize.py(manifest_validatorthroughrefuse_manifest_values). The two copies had drifted: #817's whole-manifest words reached only one of them (#929), and only the family's copy guarded against a validator that exits or answers with something that is not text. The issue's Direction is now done: there is one implementation, inshape_materialize.py, andfamily.pyimports it.TemplateManifest(inshape_materialize.py) holds everything that differs between the two manifests:_findings(manifest, policy, None); for the holder, it isReport()plus_check_envelope;projectorholder);FINDING_PLACEHOLDERandCARRIED_BYmaps, and the remedies.PROJECT_MANIFESTdescribes the project's manifest.family.HOLDER_MANIFESTdescribes the holder's.Signatures. Each shared function takes a
manifest_kindargument, which defaults to the project's (it is keyword-only on public functions). So every public name and signature inshape_materializeis unchanged, andscaffold-project.pyandadopt-project.pyare not touched.refuse_manifest_valuescallsmanifest_value_problemsexactly as it did before. Tests that point it at another template withfunctools.partialstill work.refuse_manifest_problemsis the raising half of it, split out so thatinitcan raise it with its own remedies.family.pykeeps its names.family_validator,family_manifest_misreads,family_manifest_invalidandrefuse_unholdable_manifestnow call the shared code.family.pystill owns its own words:MANIFEST_FLAGS,FINDING_PLACEHOLDER,CARRIED_BY, the three remedies andKIND_REMEDIATIONS.Order. The questions are asked in the same place as before:
cmd_initasks them before the landing, the plan or any remote exists, and under--dry-runalike. Each validator is still loaded from its own template, and no shape copy is touched.The loaded-validator cache is now
shape_materialize's, shared by both manifests and keyed by the validator file.family_module()intests/test_family_init_values.pyempties it. Undertmp_path_retention_policy=failed, a test'stmp_pathcan be the path a test before it used, and the family's rows used to get a fresh cache with each fresh module.Which words survive where the two copies differed
The words for every value are unchanged for all three tools. I checked this with an equivalence battery, which ran origin/main's helpers and this branch's in separate processes over the same inputs:
Of 12,024 answers, 11,724 are identical, and every one of
family.py's answers is identical. The 300 that differ are all on the project's side, and all come from a broken copy of the standard. In each case the family's guard is the one that survives, for both manifests:project.yamlon mainsys.exitwhile it loads<kind>-validator-unusable: "could not be loaded: SystemExit: 3"sys.exit(0)AttributeError: 'int' object has no attribute 'startswith')TypeError)project.yamlis not a mappingmanifest-validator-unusable, "raised when it was asked: AttributeError: 'list' object has no attribute 'get'"<prefix>-manifest-invalid: "the manifest this run would write is not a mapping, which is what the project's own YAML reader makes of it."Why the family's words win:
--id null,--created-by ' ',--created-on 20261011or--name 'Contoso Group', and puts--tracking-branch trueon a branch its manifest reads back as True #307 for exactly these gaps, and are pinned bytests/test_family_init_values.pyandtests/test_family_init_yaml_words.py. Lane 5's review of family.py init: where the holder's own YAML reader refuses the manifest it would write,family-manifest-invalidnames the value on the line it refuses,--org is '@x', and gives the reader's remedy, not the validator's (#929) #960 (NIT 3) kept "not a mapping" under the validator's kind and remedy.mainrefuses a manifest that is not a mapping before it asks_findings. So calling the validator broken there named the wrong file.RV1057 re-ran the battery with seven rows of its own: 12,516 answers, 12,194 identical, and the 322 that differed were the five project rows (300) and the holder's WARNING and note lines (22), which the review round restores. That battery was not re-run at
cc94c15a; the probe in the review round is the check.Every refusal code is unchanged. New rows pin the project's words for all five rows, and family rows already held its words; two new rows pin the family's whole unusable-validator refusal and its whole misread refusal byte for byte.
#923 (U064): the pyflakes warnings in these two files
recorded_gitlinkis no longer imported intofamily.py. No test or tool reads it offfamily.py: everymodule.recorded_gitlinkhit is in avalidate-*.pytemplate orbootstrap.py.YamlError/parse_yamlimports are gone from both files.family.py'simportlib.util,PLACEHOLDER_RE,renderand_unread_sentencesimports are gone too, since only the old copy used them.Review round
RV1057's review of
556a65b4was NOT READY on one MUST. The round is one commit on top,cc94c15a, pushed as a fast-forward._validator_notesdropped a line that starts withWARNINGornotefor both manifests. That filter is the project's: its validator's ownmaincounts those lines as notes. The holder's ownmainfails on every entry inreport.findings(validate-family.py), and main's family copy refused every one. So a holder validator whose only finding was a WARNING line went throughinit, and now it does not.TemplateManifest.passed_over(shape_materialize.py:5126) holds the prefixes of the lines that are not findings. It is("WARNING", " note ")inPROJECT_MANIFEST(:5153) and()infamily.HOLDER_MANIFEST(family.py:3578)._validator_notestestsnote.startswith(manifest_kind.passed_over)(:5515), which is never true of().tests/test_family_init_values.pyhas a new row,test_a_holder_validator_whose_only_finding_is_a_warning_is_refused. A holder validator whose only finding isWARNING family-id: the id is wrong(or thenoteline) is refused asfamily-manifest-invalid, with that whole line as the sentence.tests/test_scaffold_manifest_values.pyhas a new project row,test_a_project_validators_warnings_and_notes_are_notes_and_no_finding. A WARNING and anoteline are still passed over, and a FINDING beside them is still one.warnprobe.py, each tree in its own process: at556a65b4it printedPASSED: no refusal; init would go on and write the holder; atcc94c15ait printsREFUSED family-manifest-invalid :: WARNING family-id: the id is wrong, as main did.manifest_misreads,manifest_invalid,_validator_notes,manifest_judgedandmanifest_value_problemsnamed the project where the function now serves either manifest. They say "the manifest ofmanifest_kind" (the project's, by default).family-manifest-invalidnames the value on the line it refuses,--org is '@x', and gives the reader's remedy, not the validator's (#929) #960 (NIT 3) settled; lane 3 files the follow-up.test_chain_pin_sources.py,test_reader_refusals_named.py,test_adopt_plan_yaml_quoting.pyandtest_scaffold_leg_path_collisions.pynow pass at the head (RV1057's run), and gives this round's runs.Left out of scope
passed_values' docstring misstates argparse,--dry-run's help does not say its folder answer is the mode bits', and six pyflakes warnings #923's U062 (passed_values' docstring,setup-project.py), its U063 (--dry-run's help inscaffold-project.py) and U064'sCOMMIT_REwarning inscaffold-project.py. They are in other files, which other writers and pull requests hold, so this pull request is only part of setup-project.py, shape_materialize.py, scaffold-project.py and family.py:passed_values' docstring misstates argparse,--dry-run's help does not say its folder answer is the mode bits', and six pyflakes warnings #923.project.yamland once forfamily.yaml#925's Goal says "with no change in behaviour". The only intended change is the five rows in the table above, each from a broken copy of the standard, where the two copies said different things and one had to win. The one other change, a lost holder guard that RV1057 found, is restored in the review round above.Tests
Each file ran alone, with a stub
ghfirst onPATHthat exits 97,GH_TOKEN=bogus, an emptyGH_CONFIG_DIRandEMAILset:At this head,
cc94c15a(on main1a47ed07), this review round's runs:tests/test_family_init_values.pytests/test_family_init_yaml_words.pytests/test_scaffold_manifest_values.pytests/test_adopt_manifest_values.pytests/test_reader_refusals_named.pytests/test_no_auto_maintenance.pyAt the first head,
556a65b4(W925f's runs, and RV1057's where marked):tests/test_chain_pin_sources.py(RV1057)tests/test_reader_refusals_named.py(RV1057)tests/test_adopt_plan_yaml_quoting.py(RV1057)tests/test_scaffold_leg_path_collisions.py(RV1057)tests/test_cli_reference.pytests/test_agents_md_codes.pytests/test_update_add_codes_documented.pytests/test_repo_hygiene.pytests/test_shape_doctor.pytest_chain_pin_sources.py,test_reader_refusals_named.py,test_adopt_plan_yaml_quoting.py,test_scaffold_leg_path_collisions.py) passed only at the old baseb0babe18before the rebase, and now pass at556a65b4. The rows of this round change no refusal code or word, so the first-head table stands for the files not re-run atcc94c15a; GitHub's CI runs the whole suite on this PR.test_family_init_values.pyare by design: 16 rows (:229) and 3 rows (:301) run only against a template that writes the placeholder bare, and skip on one that quotes it.test_shape_doctor.pyrun only on a Windows host (:10489).pyflakes 4.0.0,
/usr/bin/python3 -m pyflakes scripts/family.py scripts/shape_materialize.py:cc94c15a: no output, exit 0;1a47ed07: U064's five warnings (family.py:181, two atfamily.py:198, two atshape_materialize.py:4971), exit 1.Mutants
Each mutant breaks one guard of the shared code in
scripts/shape_materialize.py. Then a row that drivesfamily.py initand a row that drives the scaffold run against it, and the file is put back byte for byte, checked by sha256. A baseline M0 ran every row first on the unbroken file: all 17 passed, and each selected at least one test.tests/test_family_init_values.py -k 'test_a_validator_that_cannot_be_asked_is_refused_by_name and exits-on-import'tests/test_family_init_values.py -k 'test_a_validator_that_cannot_be_asked_says_so_in_the_holders_words'tests/test_scaffold_manifest_values.py -k 'test_a_validator_that_cannot_be_asked_is_a_named_refusal_not_a_traceback and exits-at-import'tests/test_scaffold_manifest_values.py -k 'test_a_validator_that_cannot_be_asked_says_so_in_the_projects_words'tests/test_family_init_values.py -k 'test_a_validator_that_cannot_be_asked_is_refused_by_name and exits-when-asked'tests/test_family_init_values.py -k 'test_a_validator_that_exits_is_a_refusal_at_the_command_line_and_not_a_zero'tests/test_scaffold_manifest_values.py -k 'test_a_validator_that_cannot_be_asked_is_a_named_refusal_not_a_traceback and exits-when-asked'tests/test_family_init_values.py -k 'test_a_validator_that_cannot_be_asked_is_refused_by_name and finding-is-not-text'tests/test_scaffold_manifest_values.py -k 'test_a_validator_that_cannot_be_asked_is_a_named_refusal_not_a_traceback and finding-is-not-text'tests/test_family_init_values.py -k 'test_a_manifest_that_is_not_a_mapping_is_a_sentence_and_not_a_trace'tests/test_family_init_yaml_words.py -k 'test_a_manifest_that_is_not_a_mapping_keeps_its_kind_and_remedy'tests/test_scaffold_manifest_values.py -k 'test_a_manifest_that_is_not_a_mapping_is_said_and_not_a_validator_defect'tests/test_family_init_yaml_words.py -k 'test_init_names_org_with_the_readers_remedy'tests/test_reader_refusals_named.py -k 'test_the_scaffold_names_org_with_the_readers_remedy'owntests/test_family_init_yaml_words.py -k 'test_a_finding_of_the_validator_keeps_the_validators_remedy'owntests/test_scaffold_init_empty_values.py -k 'test_a_flag_passed_empty_is_refused_by_name and family'owntests/test_scaffold_init_empty_values.py -k 'test_a_flag_passed_empty_is_refused_by_name and scaffold'Every mutant is killed both by an
initrow and by a scaffold row.This round's two mutants, each on
scripts/shape_materialize.py, which was put back and compared withcmp:note.startswith(("WARNING", " note "))for both manifeststests/test_family_init_values.py -k warning(test_a_holder_validator_whose_only_finding_is_a_warning_is_refused, both lines)PROJECT_MANIFESTpasses over nothing:passed_over=()tests/test_scaffold_manifest_values.py -k warnings_and_notesMerges
The review round (
cc94c15a) changes onlyscripts/family.py,scripts/shape_materialize.py,tests/test_family_init_values.pyandtests/test_scaffold_manifest_values.py, the same four files as the first commit, and its merge-tree has not been re-run yet. The results below are at the first head,556a65b4(W925f's and RV1057's runs).git merge-tree --write-treeof556a65b4with:1a47ed07, this branch's base: clean;8a057215(the family root's bootstrap.py: a gitlink a member's own index gives no url, or a member's commit that a fetch reaching its remote did not bring, fails the run by name with its cure and no credential advice, where it saidfamily bootstrap okbeside git's exit 128 (#892) #954, which landed since; it changes only the family root'sbootstrap.pyand its test): clean, tree2fbc1d8b;556a65b4: adopt-project.py:plan --project NUL(andCon,com1,Lpt9) is refused asadopt-bad-project-namebefore anything exists, andcheckandexecutefind it in the plan'slegs.assembly, by the one question the scaffold andfamily.py initask (#1014) #1041, AGENTS.md, README.md, docs/handbook.html and setup-project.py: the taken-clone-path refusal names the--familyfolder, "nothing that stays" replaces "nothing is created", andpassed_valuessays what argparse takes as a value (#921, #923) #1051, scaffold-project.py:--dry-run's help says its answer about each folder is the mode bits' and that it leaves nothing that stays, a--reuse-empty-repore-run that takes a seed found only onseed/scaffoldnames that branch as remaining, and the unusedCOMMIT_REimport is gone (#921, #923) #1052, tests/test_chain_remedy_callers.py: three rows hold #980's unchanged--link-sourcereading at the old first-=split's corners (a keyed path that begins with=, a keyed blank path, a key in capitals) (#1054) #1058 and validate-family.py: a hand mount'sgit rmline works typed:git rm --cachedwhere a plain one refuses a staged gitlink, dies on a clone no section names or refuses what is no checkout, with what it leaves said, and a.gitmodulesgone from the work tree put back before thegit rmthat takes a section out (#936) #1025 clean;MAKEFLAGSas it is, so it runs its jobs together again; every other make still gets no jobserver and no-j(#987) #1023, validate-family.py: a hand mount'sgit rmline works typed:git rm --cachedwhere a plain one refuses a staged gitlink, dies on a clone no section names or refuses what is no checkout, with what it leaves said, and a.gitmodulesgone from the work tree put back before thegit rmthat takes a section out (#936) #1025, adopt-project.py:check's throwaway repositories are made in the work directory with the source's object format and no hook of this machine's; a path git refuses isplan-path-git-refuses, a repository git cannot make thereadopt-git-unasked(#910) #1031, adopt-project.py:plan --project NUL(andCon,com1,Lpt9) is refused asadopt-bad-project-namebefore anything exists, andcheckandexecutefind it in the plan'slegs.assembly, by the one question the scaffold andfamily.py initask (#1014) #1041, the family root's bootstrap.py: git's update ended by a signal fails the run by name, and B4 with a token in use is told from fetches redone without a trace, so the token reaches no file (#998, #996) #1045, adopt-project.py and README.md: the Makefile follow-up names a keptmakefileorGNUmakefileGNU make reads first, and where a case-blind disk writes the shape's; README says which Makefile exportsCONTRACTS_DIR(#977) #1046, update-shape.py and scripts/bump-leg.py: the three line rewriters find a first-line key after a byte-order mark, as every reader does, and write the mark back as it was (#933) #1050, AGENTS.md, README.md, docs/handbook.html and setup-project.py: the taken-clone-path refusal names the--familyfolder, "nothing that stays" replaces "nothing is created", andpassed_valuessays what argparse takes as a value (#921, #923) #1051 and scaffold-project.py:--dry-run's help says its answer about each folder is the mode bits' and that it leaves nothing that stays, a--reuse-empty-repore-run that takes a seed found only onseed/scaffoldnames that branch as remaining, and the unusedCOMMIT_REimport is gone (#921, #923) #1052. validate-family.py: a hand mount'sgit rmline works typed:git rm --cachedwhere a plain one refuses a staged gitlink, dies on a clone no section names or refuses what is no checkout, with what it leaves said, and a.gitmodulesgone from the work tree put back before thegit rmthat takes a section out (#936) #1025 changes the holder'svalidate-family.py, but notReportor_check_envelope, which are whatinitasks.Closes #925
Part of #923
Lane: openreposhape-3 (openRepoShape-3)
🤖 Generated with Claude Code