Skip to content

tests/test_value_pattern_line_break.py: ID_READERS says checked_value refuses a line feed in adopt-project.py's --id, which never calls it, and the docstrings say a fullmatch reader passes the guard when it must be listed (#1166 follow-up) #1229

Open

Description

@brettheap

(lane openRepoShape-5, 2026-10-23)

Goal: tests/test_value_pattern_line_break.py says only what is true of the readers of PROJECT_ID_RE: no row of ID_READERS gives a reason that does not hold for the tool it names, and the guard test_every_reader_of_the_id_pattern_is_in_the_table tells the author of a new reader, in its docstrings and in its failure message, that the one way to pass is a row in ID_READERS, whichever method the reader calls.

Problem

This issue applies once #1166 (#1139) has landed. #1166 is open at 5102b4294 (read with gh pr view 1166 on 2026-10-23, updated 18:04:21Z), and its second commit adds the guard. Its body was corrected for the two wordings below that it repeated (it says that adopt-project.py never calls checked_value on --id, and that "ID_READERS still gives it the checked_value reason"), so only the test file is left. Line numbers are tests/test_value_pattern_line_break.py at 5102b4294.

  1. A false reason for adopt-project.py's argv --id (rc1166b's NIT 2).
    • The module docstring's "LEFT AS THEY ARE" bullet (:42-45) reads: "The argv readers of PROJECT_ID_RE (--id in scaffold-project.py, family.py init and adopt-project.py): each holds the id to checked_value after the pattern, which refuses a line feed as unsafe-value naming --id (tests/test_derived_id_refusals.py)."
    • The ID_READERS rows for scripts/family.py (:158-159) and scaffold-project.py (:160-161) say "argv --id: checked_value refuses a line feed after it". The adopt-project.py row (:162-166) begins "argv --id, as above", which is the same reason.
    • It holds for two of the three. scaffold-project.py:2517 calls checked_value("--id", project_id) and scripts/family.py:2626 calls checked_value("--id", family_id); tests/test_derived_id_refusals.py's TOOLS (:59) pins that answer for scaffold and family only.
    • adopt-project.py never calls checked_value on --id. Its calls (git grep 'checked_value(' 5102b4294 -- adopt-project.py) are for --project (:5401), the tracking branch, --pin-owner, the leg paths and --org, and for the plan's keys. Its reader of the id plan takes, passed or derived, is PROJECT_ID_RE.match(project_id) at :5140, which takes northwind followed by a line feed.
    • A line feed is still refused, by another door: the project's own validate-manifest.py (measured below, for plan).
  2. The docstrings say fullmatch is enough (rc1166b's NIT 1).
    • The module docstring (:57-60): "A NEW READER of PROJECT_ID_RE is held to the same rule by test_every_reader_of_the_id_pattern_is_in_the_table: it asks with fullmatch, or it is listed in ID_READERS with the reason a line feed cannot reach it."
    • The test's docstring (:196-198): "A reader written later that asks PROJECT_ID_RE with match (or search) takes "atlas\n": it is read with fullmatch, or it is in ID_READERS with the reason a line feed cannot reach it, or this fails."
    • The guard is assert id_pattern_calls() == expected (:200), a dict equality over every (module, method) of every call. It is exact in both directions, so a new fullmatch reader fails it too, and so does a listed reader taken away. The assert has no message, so the author of a sibling PR sees only a dict diff.
  3. The guard reads spellings, not meaning (rc1166b's NIT 4, optional). _ID = PROJECT_ID_RE followed by _ID.match(value) passes it, and by reading so does from repo_shape import PROJECT_ID_RE as X. id_pattern_calls (:170-192) counts only a call whose receiver is the name PROJECT_ID_RE or an attribute of that name. The body of validate-manifest.py, validate-family.py, family.py and update-shape.py: an id:, a date or a repository: ending in a line feed is refused by name, and a pin's source_repository: that is no path or owner/repo name is named as the pin's (#1139) #1166 claims only the two spellings, and id_pattern_calls's docstring names the same two, so nothing in either is false; an alias is simply outside the guard.

Measured

Lane 5's re-check of #1166 (#1139), rc1166b, NIT 1, NIT 2 and NIT 4 (#1166 issuecomment-6371408915), at 5102b4294:

  • NIT 2. adopt-project.py plan --id=northwind<LF>, from a local source through conftest's write_plan, exits 2 and writes no plan: REFUSED adopt-manifest-invalid: --id is 'northwind\n', which the project's own validate-manifest.py refuses: manifest-id: id is 'northwind\n'; it must match ^[a-z0-9][a-z0-9-]*$. Measured for plan only; apply and the other adopt paths were not.
  • NIT 1. def _rc_probe(value): return PROJECT_ID_RE.fullmatch(value) appended to update-shape.py fails the guard with Left contains 1 more item: {('update-shape.py', 'fullmatch'): 1}. Making one adopt-project.py reader unreachable (if False: in place of the plan-id: test at :6791) fails it too: {('adopt-project.py', 'match'): 2} != {('adopt-project.py', 'match'): 3}. Neither failure names ID_READERS.
  • NIT 4. _ID = PROJECT_ID_RE then _ID.match(value) in update-shape.py: both guards, 2 passed. A PROJECT_ID_RE.match(...) inside a string or a comment is correctly not counted: 2 passed.
  • The file on the head, through the review's run9.sh (stub gh, GH_TOKEN=bogus, EMAIL exported, dirty=0): 30 passed on git 2.43.0 (31.59s) and on 2.47.1 (29.43s).

The drafter re-read the three places and the checked_value calls at 5102b4294 (git show of the test file, adopt-project.py, scaffold-project.py, scripts/family.py and tests/test_derived_id_refusals.py).

Suggested fix

Wording and one message, in the test file only; no production file.

  1. Correct the reason. In the module docstring bullet (:42-45), say that the first two hold the id to checked_value after the pattern, and that adopt-project.py does not: plan takes it with match and the project's own validate-manifest.py refuses the line feed (adopt-manifest-invalid, in manifest-id's words). In the adopt-project.py row (:162-166), replace "argv --id, as above" with that reason. The scripts/family.py and scaffold-project.py rows can stay.
    • Optional, so the reason is true by test as the other two are by TOOLS: one row that runs adopt-project.py plan --id with a trailing line feed and asserts exit 2 and adopt-manifest-invalid naming --id. The review measured plan only.
  2. Say "listed in ID_READERS" for any new reader. In both docstrings (:57-60, :196-198), drop "it asks with fullmatch, or": a new reader, fullmatch included, needs a row, and a reader taken away loses its row, because the table is exact. A sibling that adds any reader must add one. The body of validate-manifest.py, validate-family.py, family.py and update-shape.py: an id:, a date or a repository: ending in a line feed is refused by name, and a pin's source_repository: that is no path or owner/repo name is named as the pin's (#1139) #1166 found none among the open PRs.
  3. Give the assert a message that names ID_READERS and what to do, for example that it lists every call of a method of PROJECT_ID_RE outside tests/, fullmatch included, and that the author adds, changes or drops the row of the module and method the diff shows.
  4. Optional, NIT 4: make the guard see an alias, or say in the docstring that it reads spellings and stops there. To see one: an ast.Assign whose value is the PROJECT_ID_RE name or an attribute of that name, and an import as that renames it, each a failure that names the alias and the file. A match in a string or a comment must stay uncounted. Leaving it and saying so is a fair answer.

Verify

  • git grep -n 'checked_value' tests/test_value_pattern_line_break.py shows the checked_value reason only against scaffold-project.py and scripts/family.py, and the adopt-project.py row and the module docstring name validate-manifest.py and adopt-manifest-invalid for it.
  • A scratch clone with one reader added to a tracked module outside tests/, once per method (fullmatch, match, search) on update-shape.py: the guard fails each time, and its message names ID_READERS. Each plant is restored and the file's sha256 checked, as rc1166b did (restored=True each time).
  • A listed reader made unreachable (if False: at adopt-project.py:6791): fails, and the message names ID_READERS.
  • The file passes: 30 passed on git 2.43 and 2.47.1 as at 5102b4294 (31 if the optional plan row is added), and the hygiene five (test_pinned_copy_text.py, test_path_guard_hygiene.py, test_repo_hygiene.py, test_agents_md_codes.py, test_cli_reference.py: 306 passed on git 2.43.0 at that head).
  • The diff touches only the test file, adds no non-ASCII line and no assert that joins conditions with and or or.
  • If NIT 4 is done: the alias plants (_ID = PROJECT_ID_RE then _ID.match(value), and from repo_shape import PROJECT_ID_RE as X) fail the guard, and a PROJECT_ID_RE.match(...) in a string or a comment still passes it.

Found by lane 5's re-check of #1166 (#1139), rc1166b, NIT 1, NIT 2 and NIT 4 (#1166 issuecomment-6371408915), at 5102b4294; the three places and the checked_value calls were re-read at that head by the drafter.

Lane: openreposhape-5 (openRepoShape-5)

Activity

  1. brettheap commented on Oct 23, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-5, session 50cede29-ab82-4ca3-9f67-d98e1a58c8d2@Eagle, 2026-10-23T21:20:52Z, for #1229

    Logged in lanes/log/openRepoShape-5.md at 894023c9e4d4c26ba08f34f6a22fb363785b0740.

    The three reads (lane-collision-protocol Rule 1):

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#1229:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#1229"`, kept where the row or its body names `#1229`:
       none
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `1229` as a whole token:
       none
    
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions