diff --git a/openspec/changes/3308-the-plan-check-compares-a-number/tasks.md b/openspec/changes/3308-the-plan-check-compares-a-number/tasks.md deleted file mode 100644 index c9cc13bf6..000000000 --- a/openspec/changes/3308-the-plan-check-compares-a-number/tasks.md +++ /dev/null @@ -1,59 +0,0 @@ -# Tasks - -## 1. Read the staged integer constants and pair them with a plan number - -Reads: `src/devops_cli/roadmap/stage_probe.py`, `src/devops_cli/roadmap/pull_form.py`, `src/devops_cli/lang/en/messages.py`. Changes: `src/devops_cli/roadmap/stage_constants.py` (new), `src/devops_cli/lang/en/messages.py`, `tests/roadmap/test_roadmap_stage_constants.py` (new). - -- [ ] 1.1 In `tests/roadmap/test_roadmap_stage_constants.py`, assert that a source with `SUMMARY_MAX_CHARS: Final[int] = 60` gives 60, that an alias resolves to the integer in another module, that a value from a call is left out, that a name that two modules define with different values is left out, and that a plan text with `SUMMARY_MAX_CHARS` and 40 in one sentence yields one problem that names the constant, 40 and 60. Assert that a matching number yields no problem, that a note reference (`#2714`) and a line location (`:12`) are no candidate number, and that a sentence with two candidate numbers yields no problem. Verify that `uv run devops ci test tests/roadmap/test_roadmap_stage_constants.py` fails on the base - - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_stage_constants.py` fails on the base -- [ ] 1.2 Add `src/devops_cli/roadmap/stage_constants.py` with `candidate_names(texts)`, `staged_constants(source, sha, names)` and `constant_problems(texts, constants)`. `staged_constants` finds each name with `git grep` under `src/` at `sha`, reads only the files it names with `git show`, parses them with `ast.parse`, resolves an alias across modules, and returns no value for an assignment that is no integer literal and no value for an ambiguous name. `constant_problems` splits a text into sentences, finds one known constant and one candidate number in a sentence, and returns one message for each number that differs, naming the constant, the plan's number and the staged value. Add the message to `src/devops_cli/lang/en/messages.py`, and verify that `uv run devops ci test tests/roadmap/test_roadmap_stage_constants.py` passes - - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_stage_constants.py` passes - -## 2. Refuse the plan in `check_proposal` - -Reads: `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage.py`, `src/devops_cli/roadmap/stage_constants.py`, `tests/roadmap/test_roadmap_stage_plan.py`. Changes: `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage.py`, `tests/roadmap/test_roadmap_stage_plan.py`. - -- [ ] 2.1 In `tests/roadmap/test_roadmap_stage_plan.py`, assert that `check_proposal` with `constants={"SUMMARY_MAX_CHARS": 60}` returns one problem for a proposal whose spec scenario says 40, returns none for one that says 60, returns none for a proposal with no constants, and returns none for the text of a task group that holds 40, and verify that `uv run devops ci test tests/roadmap/test_roadmap_stage_plan.py` fails on the base - - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_stage_plan.py` fails on the base -- [ ] 2.2 Add `plan_texts(proposal)` to `src/devops_cli/roadmap/stage_change.py`: the fields that render to `proposal.md`, `design.md` and each spec delta, with no task group. Add the `constants` keyword to `check_proposal` and add its problems to the list that the function returns. In `Stage._settle` in `src/devops_cli/roadmap/stage.py`, read the constants with `staged_constants(self._source, self._sha, candidate_names(plan_texts(proposal)))` and pass them to `check_proposal`. Verify that `uv run devops ci test tests/roadmap/test_roadmap_stage_plan.py` passes - - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_stage_plan.py` passes - -## 3. Ask the plan stage prompt for the staged value of a constant - -Reads: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/test_roadmap_stage_prompt_rules.py`. Changes: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/test_roadmap_stage_prompt_rules.py`. - -- [ ] 3.1 Add one rule to `src/devops_cli/ai/tasks/roadmap_stage_change.md`: cite the staged value of each constant that the plan relies on, with the constant name and its value, as `feasibility` quotes the probe lines, and give no new name to the bound of a constant that the staged tree defines. Add the assertion of that rule to `tests/roadmap/test_roadmap_stage_prompt_rules.py` - - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_stage_prompt_rules.py` passes - -## 4. Carry recorded constants through the stage eval replay - -Reads: `src/devops_cli/roadmap/stage_eval_cases.py`, `src/devops_cli/roadmap/stage_eval_gates.py`, `tests/roadmap/test_roadmap_stage_eval_gates.py`. Changes: `src/devops_cli/roadmap/stage_eval_cases.py`, `src/devops_cli/roadmap/stage_eval_gates.py`, `tests/roadmap/test_roadmap_stage_eval_gates.py`. - -- [ ] 4.1 In `tests/roadmap/test_roadmap_stage_eval_gates.py`, assert that a case with no `constants` field replays as it does today, and that a case with `constants` refuses a plan whose spec scenario names a different value. Verify that `uv run devops ci test tests/roadmap/test_roadmap_stage_eval_gates.py` fails on the base - - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_stage_eval_gates.py` fails on the base -- [ ] 4.2 Add `constants: tuple[tuple[str, int], ...] = ()` to `CaseFacts`, and a `constants` field with an empty default to `PlanUnderTest`. `PlanUnderTest.of_case` reads `dict(case.facts.constants)`, and `_path_gate` passes `plan.constants` to `check_proposal`. Verify that `uv run devops ci test tests/roadmap/test_roadmap_stage_eval_gates.py` passes - - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_stage_eval_gates.py` passes - -## 5. Check the OpenSpec change against the base specs - -Reads: `openspec/config.yaml`. Changes: `openspec/changes/3308-the-plan-check-compares-a-number/proposal.md`, `openspec/changes/3308-the-plan-check-compares-a-number/design.md`, `openspec/changes/3308-the-plan-check-compares-a-number/tasks.md`, `openspec/changes/3308-the-plan-check-compares-a-number/.openspec.yaml`, `openspec/changes/3308-the-plan-check-compares-a-number/specs/roadmap-stage/spec.md`. - -- [ ] 5.1 Keep the change folder `openspec/changes/3308-the-plan-check-compares-a-number/` with its proposal, its design, its `tasks.md` and the spec delta of `roadmap-stage`, then run the validation - - Verification: `$ uv run devops openspec check` passes -- [ ] 5.2 The change folder is the record of the item, and run `uv run devops docs generate --sync-readme` - - Verification: `$ uv run devops ci docs` passes - -## Criteria - -- Group 2: The plan check SHALL refuse a plan when one known integer constant and one candidate number in a sentence differ from the staged value. -- Group 2: The plan check SHALL accept a plan when the number beside a known integer constant equals the staged value of that constant. -- Group 1: The plan check MUST ignore a constant name that the staged tree does not define as a module-level integer constant. -- Group 1: The plan check MUST skip a sentence with more than one candidate number and a name that two modules define with different values. -- Group 2: The plan check MUST NOT compare a number in `tasks.md` with a constant. -- Group 4: The stage eval replay MUST read recorded constants and MUST read no git. -- Group 3: The plan stage prompt SHALL tell the model to cite the staged value of each constant that the plan relies on. -- Group 1: The plan check MUST read constant values with Python AST and MUST NOT import or execute staged code. - -## Workflow follow-up - -- Pending a person: review the diff in tuicr (tuicr -r ..HEAD). diff --git a/openspec/changes/3308-the-plan-check-compares-a-number/.openspec.yaml b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/.openspec.yaml similarity index 100% rename from openspec/changes/3308-the-plan-check-compares-a-number/.openspec.yaml rename to openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/.openspec.yaml diff --git a/openspec/changes/3308-the-plan-check-compares-a-number/design.md b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/design.md similarity index 90% rename from openspec/changes/3308-the-plan-check-compares-a-number/design.md rename to openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/design.md index 86b19064d..461fee7ca 100644 --- a/openspec/changes/3308-the-plan-check-compares-a-number/design.md +++ b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/design.md @@ -12,14 +12,14 @@ - **Carry the constants in `CaseFacts` with an empty default, and pass them through `PlanUnderTest` to `check_proposal`.**: The recorded cases keep no text of a file, so an empty mapping leaves them as they are and a later case can record the values it needs. - **Name the constant, the plan's number and the staged value in one problem line.**: The model corrects the number in one turn, as `SUMMARY_MAX_CHARS is 60 in the staged tree, but the plan says 40` shows. - **Add the citation rule to the plan stage prompt beside the rule that asks for the quoted probe lines.**: The model has the read tools, and it writes the plan before the check runs, so the rule makes the first reply pass. -- **Keep the check in a new module `src/devops_cli/roadmap/stage_constants.py` and call it from `check_proposal`.**: `check_proposal` is the one seam that the stage job and the replay both call, so the rule runs in both paths. +- **Keep the check in a new module `src/devops_cli/roadmap/stage/stage_constants.py` and call it from `check_proposal`.**: `check_proposal` is the one seam that the stage job and the replay both call, so the rule runs in both paths. ## Rejected alternatives -- **Import the staged modules to read the constant values.**: `src/devops_cli/roadmap/stage_probe.py` reads the commit with `git cat-file`, `git grep` and `git log` only, so the check runs no staged code. +- **Import the staged modules to read the constant values.**: `src/devops_cli/roadmap/stage/stage_probe.py` reads the commit with `git cat-file`, `git grep` and `git log` only, so the check runs no staged code. - **Find the pairs with a regular expression over the source text.**: `src/devops_cli/roadmap/pull_form.py` holds the docstring `Its pattern `^.{1,60}$` runs in JavaScript` and the assignment `BODY_MAX_CHARS: Final[int] = CONST_ROADMAP_IMPLEMENT_BODY_MAX_CHARS`, so a regex over text reads 60 from a docstring and finds no literal for an aliased constant. - **Read every Python file of the staged tree, with no `git grep` first.**: The tree holds hundreds of files, and a plan cites a few names. `git grep` gives the files to parse and keeps the read inside the command budget. -- **Scan every file of the change folder, `tasks.md` included.**: `src/devops_cli/roadmap/stage_change.py` renders `tasks.md` from `group_text`, where a verify command holds a test path and a number that is no claim about a constant. +- **Scan every file of the change folder, `tasks.md` included.**: `src/devops_cli/roadmap/stage/stage_change.py` renders `tasks.md` from `group_text`, where a verify command holds a test path and a number that is no claim about a constant. - **Pair every number in a sentence with every constant in it.**: A sentence that says `SUMMARY_MAX_CHARS` is 60, while the tree says 47, and the issue says 40 is a counterexample and not a claim. The strict rule skips it. - **Refuse a plan that names a constant which the staged tree does not define.**: The plan folder `openspec/changes/3200-refuse-a-plan-with-placeholder-or/design.md` names `PLAN_SUMMARY_MAX_CHARS = 47`, which `src/` does not define, so a refusal would stop a valid plan. - **Read the constants through `StagedTree`.**: The class holds file names only, and every test that builds a tree would need the source and the commit. The explicit mapping leaves the tree and those tests alone. diff --git a/openspec/changes/3308-the-plan-check-compares-a-number/proposal.md b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/proposal.md similarity index 70% rename from openspec/changes/3308-the-plan-check-compares-a-number/proposal.md rename to openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/proposal.md index e167acc5e..18b1892d9 100644 --- a/openspec/changes/3308-the-plan-check-compares-a-number/proposal.md +++ b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/proposal.md @@ -8,12 +8,12 @@ A sentence that holds more than one number is ambiguous: it can show an example, ## What Changes -- Add `src/devops_cli/roadmap/stage_constants.py`: +- Add `src/devops_cli/roadmap/stage/stage_constants.py`: - `candidate_names(texts)` reads the constant names that the plan text cites. - `staged_constants(source, sha, names)` finds the definition of each name with `git grep` at the staging commit, reads only those files with `git show`, parses them with `ast`, and resolves an alias across modules. It imports and runs no staged code. - `constant_problems(texts, constants)` returns one problem for each sentence that names one known constant and holds one candidate number that differs. -- Add `plan_texts(proposal)` to `src/devops_cli/roadmap/stage_change.py`: the plan fields that render to `proposal.md`, `design.md` and the spec deltas. It leaves out every task group, so `tasks.md` is never scanned. -- Add a `constants` keyword to `check_proposal` in `stage_change.py`, and pass the mapping that `Stage._settle` reads with `staged_constants` from `self._source` and `self._sha`. +- Add `plan_texts(proposal)` to `src/devops_cli/roadmap/stage/stage_change.py`: the plan fields that render to `proposal.md`, `design.md` and the spec deltas. It leaves out every task group, so `tasks.md` is never scanned. +- Add a `constants` keyword to `check_proposal` in `src/devops_cli/roadmap/stage/stage_change.py`, and pass the mapping that `Stage._settle` reads with `staged_constants` from `self._source` and `self._sha`. - Carry recorded constants through the stage eval replay: `CaseFacts` gets an optional `constants` field, and `PlanUnderTest` passes it to `check_proposal`. A case with no field keeps an empty mapping, so the recorded cases do not change. - Add the message that names the constant, the plan's number and the staged value to `src/devops_cli/lang/en/messages.py`. - Add the citation rule to `src/devops_cli/ai/tasks/roadmap_stage_change.md`: cite the staged value of each constant that the plan relies on, and do not give a new name to the bound of a constant that the staged tree defines. @@ -32,9 +32,9 @@ None. ## Impact -- Code: `src/devops_cli/roadmap/stage_constants.py`, `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage.py`, `src/devops_cli/roadmap/stage_eval_cases.py`, `src/devops_cli/roadmap/stage_eval_gates.py`, `src/devops_cli/lang/en/messages.py`, `openspec/changes/3308-the-plan-check-compares-a-number/.openspec.yaml` -- Documents: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `openspec/changes/3308-the-plan-check-compares-a-number/proposal.md`, `openspec/changes/3308-the-plan-check-compares-a-number/design.md`, `openspec/changes/3308-the-plan-check-compares-a-number/tasks.md`, `openspec/changes/3308-the-plan-check-compares-a-number/specs/roadmap-stage/spec.md` -- Tests: `tests/roadmap/test_roadmap_stage_constants.py`, `tests/roadmap/test_roadmap_stage_plan.py`, `tests/roadmap/test_roadmap_stage_prompt_rules.py`, `tests/roadmap/test_roadmap_stage_eval_gates.py` +- Code: `src/devops_cli/roadmap/stage/stage_constants.py`, `src/devops_cli/roadmap/stage/stage_change.py`, `src/devops_cli/roadmap/stage/stage.py`, `src/devops_cli/roadmap/stage/stage_eval_cases.py`, `src/devops_cli/roadmap/stage/stage_eval_gates.py`, `src/devops_cli/lang/en/messages.py` +- Documents: `src/devops_cli/ai/tasks/roadmap_stage_change.md` +- Tests: `tests/roadmap/stage/test_roadmap_stage_constants.py`, `tests/roadmap/stage/test_roadmap_stage_plan.py`, `tests/roadmap/stage/test_roadmap_stage_prompt_rules.py`, `tests/roadmap/stage/test_roadmap_stage_eval_gates.py` ## Feasibility @@ -42,11 +42,11 @@ None. - The probes give `The path `src/devops_cli/roadmap/stage_change.py` is a file of the staging commit.`, and that file holds `def check_proposal(` with `problems.extend(path_problems(proposal.groups, tree))`, so the plan check has one place for the new rule. - The probes give `The path `src/devops_cli/roadmap/stage_paths.py` is a file of the staging commit.`, and `class StagedTree` holds `self._files = frozenset(files)` and no text of a file. The content therefore comes from `Stage._settle`, which holds `self._source` and `self._sha` and calls `check_proposal(`, and not from the tree. - The probes give `The path `src/devops_cli/roadmap/stage_probe.py` is a file of the staging commit.`, and that file holds `probe_premise(source: Path, sha: str, body: str)`, the read-only probe that the new reader follows with `git grep` and `git show`. -- `src/devops_cli/roadmap/stage_change.py` holds `def render_change(` and `def _spec_file(`, so the plan fields that render to the change files are known and the check can read them without the change folder name. -- `src/devops_cli/roadmap/stage_eval_cases.py` holds `class CaseFacts` with `tracked` and `capabilities`, so the replay holds no file text. An optional `constants` field keeps every recorded case valid and lets a later case record the values. +- `src/devops_cli/roadmap/stage/stage_change.py` holds `def render_change(` and `def _spec_file(`, so the plan fields that render to the change files are known and the check can read them without the change folder name. +- `src/devops_cli/roadmap/stage/stage_eval_cases.py` holds `class CaseFacts` with `tracked` and `capabilities`, so the replay holds no file text. An optional `constants` field keeps every recorded case valid and lets a later case record the values. - The plan of the pull request for #3260 said `StageProposal.summary` takes the bound `PLAN_SUMMARY_MAX_CHARS = 47`, and that the issue text says 40 and the schema says 60. The staged tree defines no `PLAN_SUMMARY_MAX_CHARS`, so the check skips that name, as the design decided to avoid a false refusal. The check refuses the explicit claim that the issue records, `SUMMARY_MAX_CHARS` beside 40, and the prompt rule makes the model state the staged 60. - The probes give `The path `src/devops_cli/ai/tasks/roadmap_stage_change.md` is a file of the staging commit.`, so the prompt rule goes into a file of the commit. -- First checks against the real system: `uv run devops ci test tests/roadmap/test_roadmap_stage_constants.py`, then `uv run devops openspec check`, then `uv run devops roadmap eval stage --dry-run` to see the gate on the golden set. +- First checks against the real system: `uv run devops ci test tests/roadmap/stage/test_roadmap_stage_constants.py`, then `uv run devops openspec check`, then `uv run devops roadmap eval stage --dry-run` to see the gate on the golden set. ## Existing Tool Considered diff --git a/openspec/changes/3308-the-plan-check-compares-a-number/specs/roadmap-stage/spec.md b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/specs/roadmap-stage/spec.md similarity index 100% rename from openspec/changes/3308-the-plan-check-compares-a-number/specs/roadmap-stage/spec.md rename to openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/specs/roadmap-stage/spec.md diff --git a/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/tasks.md b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/tasks.md new file mode 100644 index 000000000..9f77169ce --- /dev/null +++ b/openspec/changes/archive/2026-11-07-3308-the-plan-check-compares-a-number/tasks.md @@ -0,0 +1,50 @@ +# Tasks + +## 1. Read the staged integer constants and pair them with a plan number + +Reads: `src/devops_cli/roadmap/stage/stage_probe.py`, `src/devops_cli/roadmap/pull_form.py`, `src/devops_cli/lang/en/messages.py`. Changes: `src/devops_cli/roadmap/stage/stage_constants.py` (new), `src/devops_cli/lang/en/messages.py`, `tests/roadmap/stage/test_roadmap_stage_constants.py` (new). + +- [x] 1.1 In `tests/roadmap/stage/test_roadmap_stage_constants.py`, assert that a source with `SUMMARY_MAX_CHARS: Final[int] = 60` gives 60, that an alias resolves to the integer in another module, that a value from a call is left out, that a name that two modules define with different values is left out, and that a plan text with `SUMMARY_MAX_CHARS` and 40 in one sentence yields one problem that names the constant, 40 and 60. Assert that a matching number yields no problem, that a note reference (`#2714`) and a line location (`:12`) are no candidate number, and that a sentence with two candidate numbers yields no problem. Verify that `uv run devops ci test tests/roadmap/stage/test_roadmap_stage_constants.py` fails on the base + - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_constants.py` fails on the base +- [x] 1.2 Add `src/devops_cli/roadmap/stage/stage_constants.py` with `candidate_names(texts)`, `staged_constants(source, sha, names)` and `constant_problems(texts, constants)`. `staged_constants` finds each name with `git grep` under `src/` at `sha`, reads only the files it names with `git show`, parses them with `ast.parse`, resolves an alias across modules, and returns no value for an assignment that is no integer literal and no value for an ambiguous name. `constant_problems` splits a text into sentences, finds one known constant and one candidate number in a sentence, and returns one message for each number that differs, naming the constant, the plan's number and the staged value. Add the message to `src/devops_cli/lang/en/messages.py`, and verify that `uv run devops ci test tests/roadmap/stage/test_roadmap_stage_constants.py` passes + - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_constants.py` passes + +## 2. Refuse the plan in `check_proposal` + +Reads: `src/devops_cli/roadmap/stage/stage_change.py`, `src/devops_cli/roadmap/stage/stage.py`, `src/devops_cli/roadmap/stage/stage_constants.py`, `tests/roadmap/stage/test_roadmap_stage_plan.py`. Changes: `src/devops_cli/roadmap/stage/stage_change.py`, `src/devops_cli/roadmap/stage/stage.py`, `tests/roadmap/stage/test_roadmap_stage_plan.py`. + +- [x] 2.1 In `tests/roadmap/stage/test_roadmap_stage_plan.py`, assert that `check_proposal` with `constants={"SUMMARY_MAX_CHARS": 60}` returns one problem for a proposal whose spec scenario says 40, returns none for one that says 60, returns none for a proposal with no constants, and returns none for the text of a task group that holds 40, and verify that `uv run devops ci test tests/roadmap/stage/test_roadmap_stage_plan.py` fails on the base + - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_plan.py` fails on the base +- [x] 2.2 Add `plan_texts(proposal)` to `src/devops_cli/roadmap/stage/stage_change.py`: the fields that render to `proposal.md`, `design.md` and each spec delta, with no task group. Add the `constants` keyword to `check_proposal` and add its problems to the list that the function returns. In `Stage._settle` in `src/devops_cli/roadmap/stage/stage.py`, read the constants with `staged_constants(self._source, self._sha, candidate_names(plan_texts(proposal)))` and pass them to `check_proposal`. Verify that `uv run devops ci test tests/roadmap/stage/test_roadmap_stage_plan.py` passes + - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_plan.py` passes + +## 3. Ask the plan stage prompt for the staged value of a constant + +Reads: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/stage/test_roadmap_stage_prompt_rules.py`. Changes: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/stage/test_roadmap_stage_prompt_rules.py`. + +- [x] 3.1 Add one rule to `src/devops_cli/ai/tasks/roadmap_stage_change.md`: cite the staged value of each constant that the plan relies on, with the constant name and its value, as `feasibility` quotes the probe lines, and give no new name to the bound of a constant that the staged tree defines. Add the assertion of that rule to `tests/roadmap/stage/test_roadmap_stage_prompt_rules.py` + - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_prompt_rules.py` passes + +## 4. Carry recorded constants through the stage eval replay + +Reads: `src/devops_cli/roadmap/stage/stage_eval_cases.py`, `src/devops_cli/roadmap/stage/stage_eval_gates.py`, `tests/roadmap/stage/test_roadmap_stage_eval_gates.py`. Changes: `src/devops_cli/roadmap/stage/stage_eval_cases.py`, `src/devops_cli/roadmap/stage/stage_eval_gates.py`, `tests/roadmap/stage/test_roadmap_stage_eval_gates.py`. + +- [x] 4.1 In `tests/roadmap/stage/test_roadmap_stage_eval_gates.py`, assert that a case with no `constants` field replays as it does today, and that a case with `constants` refuses a plan whose spec scenario names a different value. Verify that `uv run devops ci test tests/roadmap/stage/test_roadmap_stage_eval_gates.py` fails on the base + - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_eval_gates.py` fails on the base +- [x] 4.2 Add `constants: tuple[tuple[str, int], ...] = ()` to `CaseFacts`, and a `constants` field with an empty default to `PlanUnderTest`. `PlanUnderTest.of_case` reads `dict(case.facts.constants)`, and `_path_gate` passes `plan.constants` to `check_proposal`. Run `uv run devops roadmap eval stage --replay` and record that the score before the change was 69 of 306 (23%) and the score after is 69 of 306 (23%). Verify that `uv run devops ci test tests/roadmap/stage/test_roadmap_stage_eval_gates.py` passes + - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_eval_gates.py` passes + +## Criteria + +- Group 2: The plan check SHALL refuse a plan when one known integer constant and one candidate number in a sentence differ from the staged value. +- Group 2: The plan check SHALL accept a plan when the number beside a known integer constant equals the staged value of that constant. +- Group 1: The plan check MUST ignore a constant name that the staged tree does not define as a module-level integer constant. +- Group 1: The plan check MUST skip a sentence with more than one candidate number and a name that two modules define with different values. +- Group 2: The plan check MUST NOT compare a number in `tasks.md` with a constant. +- Group 4: The stage eval replay MUST read recorded constants and MUST read no git. +- Group 3: The plan stage prompt SHALL tell the model to cite the staged value of each constant that the plan relies on. +- Group 1: The plan check MUST read constant values with Python AST and MUST NOT import or execute staged code. + +## Workflow follow-up + +- Pending a person: review the diff in tuicr (tuicr -r ..HEAD). diff --git a/openspec/specs/roadmap-stage/spec.md b/openspec/specs/roadmap-stage/spec.md index 65243f2d9..eb939f3e9 100644 --- a/openspec/specs/roadmap-stage/spec.md +++ b/openspec/specs/roadmap-stage/spec.md @@ -2713,3 +2713,77 @@ The planner prompt SHALL name the `manifest` kind, its case-first rule and its v #### Scenario: Prompt and kinds agree - **WHEN** the prompt-vs-rules test reads `src/devops_cli/ai/tasks/roadmap_stage_change.md` and the names of `GroupKind`. - **THEN** the test finds `manifest` in both and passes. + +### Requirement: A plan number beside a constant matches the staged value +The plan check SHALL refuse a plan when a sentence names one known module-level integer constant of the staged tree, holds one candidate decimal number, and that number differs from the value of the constant. + +#### Scenario: Summary bound mismatch +- **WHEN** A spec scenario says `SUMMARY_MAX_CHARS` is 40, and the staged tree sets `SUMMARY_MAX_CHARS = 60`. +- **THEN** The check returns one problem that names `SUMMARY_MAX_CHARS`, 40 and 60. + +#### Scenario: Summary bound match +- **WHEN** A design decision says `SUMMARY_MAX_CHARS` is 60, and the staged tree sets `SUMMARY_MAX_CHARS = 60`. +- **THEN** The check returns no problem about `SUMMARY_MAX_CHARS`. + +#### Scenario: A bound after the constant name +- **WHEN** A requirement says `SUMMARY_MAX_CHARS` holds 60 characters, and the staged tree sets `SUMMARY_MAX_CHARS = 60`. +- **THEN** The check returns no problem about `SUMMARY_MAX_CHARS`. + +#### Scenario: A note reference beside the constant +- **WHEN** A requirement says `SUMMARY_MAX_CHARS` is 60 (#2714), and the staged tree sets `SUMMARY_MAX_CHARS = 60`. +- **THEN** The check returns no problem about `SUMMARY_MAX_CHARS`. + +#### Scenario: Two numbers in one sentence +- **WHEN** A design decision says `SUMMARY_MAX_CHARS` is 40, while the staged tree sets it to 60. +- **THEN** The check returns no problem about `SUMMARY_MAX_CHARS`, because the sentence is ambiguous. + +### Requirement: A constant that the staged tree does not define is ignored +The plan check MUST skip a constant name that the staged tree does not define as a module-level integer assignment. + +#### Scenario: New plan constant +- **WHEN** A design decision names `PLAN_SUMMARY_MAX_CHARS = 47`, and the staged tree does not define `PLAN_SUMMARY_MAX_CHARS`. +- **THEN** The check returns no problem about `PLAN_SUMMARY_MAX_CHARS`. + +#### Scenario: Non-literal assignment +- **WHEN** A staged module sets `SUMMARY_MAX_CHARS` from a call, and the plan says `SUMMARY_MAX_CHARS` is 40. +- **THEN** The check returns no problem about `SUMMARY_MAX_CHARS`. + +#### Scenario: Ambiguous definition +- **WHEN** Two staged modules set `SUMMARY_MAX_CHARS` to different integers, and the plan says `SUMMARY_MAX_CHARS` is 40. +- **THEN** The check returns no problem about `SUMMARY_MAX_CHARS`, because the tree holds no one value. + +### Requirement: The check reads constants without running staged code +The plan check MUST read the constant values with the Python `ast` module and MUST NOT import or execute a staged file. + +#### Scenario: A staged module with a side effect +- **WHEN** A staged Python module writes a file at import, and the plan says `SUMMARY_MAX_CHARS` is 60. +- **THEN** The check writes no file and returns no problem about `SUMMARY_MAX_CHARS`. + +#### Scenario: An alias to another constant +- **WHEN** A staged module sets `BODY_MAX_CHARS = CONST_ROADMAP_IMPLEMENT_BODY_MAX_CHARS` and another staged module sets `CONST_ROADMAP_IMPLEMENT_BODY_MAX_CHARS = 1000`, and the plan says `BODY_MAX_CHARS` is 1000. +- **THEN** The check returns no problem about `BODY_MAX_CHARS`. + +### Requirement: The task commands carry no constant claim +The plan check MUST NOT compare a number in `tasks.md` with a constant. + +#### Scenario: Task command number +- **WHEN** `tasks.md` holds `SUMMARY_MAX_CHARS` and 40 in one command, and the staged tree sets `SUMMARY_MAX_CHARS = 60`. +- **THEN** The check returns no problem about `SUMMARY_MAX_CHARS`. + +### Requirement: The replay passes recorded constants +The stage eval replay MUST pass the constants of a case to the plan check and MUST read no git. + +#### Scenario: Recorded constants +- **WHEN** A case records `SUMMARY_MAX_CHARS` as 60, and its spec scenario says 40. +- **THEN** The replay refuses the plan with the problem that names the constant, 40 and 60. + +#### Scenario: A case with no constants +- **WHEN** A case records no constants. +- **THEN** The replay runs the check with an empty mapping and keeps the result it gives today. + +### Requirement: The prompt asks for the staged value of a constant +The plan stage prompt SHALL tell the model to cite the staged value of each constant that the plan relies on, and to give no new name to the bound of a constant that the staged tree defines. + +#### Scenario: Prompt rule +- **WHEN** The prompt rules test reads `src/devops_cli/ai/tasks/roadmap_stage_change.md`. +- **THEN** The text holds an instruction to cite the staged value of each constant that the plan relies on. diff --git a/src/devops_cli/ai/tasks/roadmap_stage_change.md b/src/devops_cli/ai/tasks/roadmap_stage_change.md index cf742b592..2b54f2ee9 100644 --- a/src/devops_cli/ai/tasks/roadmap_stage_change.md +++ b/src/devops_cli/ai/tasks/roadmap_stage_change.md @@ -7,6 +7,7 @@ Write the change as it will read after the merge. State the final behavior and n The rules of the project: - Unless a section on tools follows these rules, you cannot run a command. Write only what the inputs and your tool answers show, and cite the repository paths that show it. In `feasibility`, say how the premise of the issue was confirmed against the inputs: quote the lines of `probes` that confirm it, word for word. Then name the first checks that the implementation runs against the real system, each with its command. If the inputs show that the issue cannot be built as written, say so there, and plan no code that compensates for it. +- Cite the staged value of each constant that the plan relies on, with the constant name and its value, as `feasibility` quotes the probe lines, and give no new name to the bound of a constant that the staged tree defines. - In `feasibility`, give one line for each function that the plan calls whose arguments need a checkout, a client or a network. Name that need in the line as a precondition of the call: a checkout of the repository, a client of a service, or a network. - When `rejected_plan` is in the input, state in `feasibility` how the new plan answers that reason. Do not plan again the premise that the reason shows to be false. If the issue still cannot be built as written, say so in `feasibility` and plan no code that compensates for it. - The reply has no key to report a false premise. When `probes` or the other inputs show that the issue cannot be built as written, for example a cited path or identifier that a commit deleted, renamed or removed, say so in `feasibility` with a quote of the lines that show it, and plan no code that compensates for it. A path or identifier that no commit ever held can be a thing that the issue asks to create: it is no false premise. diff --git a/src/devops_cli/lang/en/messages.py b/src/devops_cli/lang/en/messages.py index abf98fab8..6c319c903 100644 --- a/src/devops_cli/lang/en/messages.py +++ b/src/devops_cli/lang/en/messages.py @@ -3737,6 +3737,9 @@ class RoadmapMessages: stage_problem_missing_caller: str = ( "The plan changes `{function}` and leaves out `{path}`, which calls it." ) + stage_problem_constant_mismatch: str = ( + "{constant} is {staged} in the staged tree, but the plan says {plan}." + ) stage_path_same_name: str = "These files have the same name: {names}." stage_path_no_same_name: str = "No file has this name." stage_path_places: dict[str, str] = field( diff --git a/src/devops_cli/roadmap/stage/stage.py b/src/devops_cli/roadmap/stage/stage.py index dbf32bd72..fa4612761 100644 --- a/src/devops_cli/roadmap/stage/stage.py +++ b/src/devops_cli/roadmap/stage/stage.py @@ -162,6 +162,7 @@ closing_keyword_in, creates_problems, measure_groups, + plan_texts, pull_request_body, render_change, safe_text, @@ -169,6 +170,7 @@ staging_title, write_change_files, ) +from devops_cli.roadmap.stage.stage_constants import candidate_names, staged_constants from devops_cli.roadmap.stage.stage_criteria import criterion_problems from devops_cli.roadmap.stage.stage_filter import split_filter_entry from devops_cli.roadmap.stage.stage_form import ( @@ -1123,6 +1125,7 @@ def _settle( evidence = ( record_search_evidence(proposal.groups, self._grep) if self._grep is not None else None ) + constants = staged_constants(self._source, self._sha, candidate_names(plan_texts(proposal))) problems = check_proposal( proposal, self._known, @@ -1138,6 +1141,7 @@ def _settle( # (#3422), and never the working tree. read_text=partial(_read_staged_file, self._source, self._sha), registry=registry_table(self._source), + constants=constants, ) if problems: return _Refused(" ".join(problems), proposal) diff --git a/src/devops_cli/roadmap/stage/stage_change.py b/src/devops_cli/roadmap/stage/stage_change.py index 5e0ddfd97..d905dc19e 100644 --- a/src/devops_cli/roadmap/stage/stage_change.py +++ b/src/devops_cli/roadmap/stage/stage_change.py @@ -25,7 +25,7 @@ from enum import StrEnum from itertools import pairwise from pathlib import Path, PurePosixPath -from typing import TYPE_CHECKING, Annotated, Final, Self, assert_never +from typing import TYPE_CHECKING, Annotated, Any, Final, Self, assert_never from pydantic import ( AfterValidator, @@ -54,6 +54,7 @@ summary_length, ) from devops_cli.roadmap.refine.refine import sanitize_text +from devops_cli.roadmap.stage.stage_constants import constant_problems from devops_cli.roadmap.stage.stage_criteria import ( PENDING_PERSON, StageCriterion, @@ -695,6 +696,64 @@ def _text_problems(field: str, text: str) -> list[str]: return problems +def _scenario_texts(scenario: Any) -> list[str]: + if isinstance(scenario, dict): + return [scenario.get("name", ""), scenario.get("when", ""), scenario.get("then", "")] + return [ + getattr(scenario, "name", ""), + getattr(scenario, "when", ""), + getattr(scenario, "then", ""), + ] + + +def _req_texts(req: Any) -> list[str]: + if isinstance(req, dict): + texts = [req.get("name", ""), req.get("text", "")] + for sc in req.get("scenarios", []): + texts.extend(_scenario_texts(sc)) + return texts + texts = [getattr(req, "name", ""), getattr(req, "text", "")] + for sc in getattr(req, "scenarios", []): + texts.extend(_scenario_texts(sc)) + return texts + + +def _spec_texts(spec: Any) -> list[str]: + if isinstance(spec, dict): + texts = [spec.get("purpose", "")] + reqs = (*spec.get("added", []), *spec.get("modified", [])) + else: + texts = [getattr(spec, "purpose", "")] + reqs = (*getattr(spec, "added", ()), *getattr(spec, "modified", ())) + for req in reqs: + texts.extend(_req_texts(req)) + return texts + + +def _decision_texts(item: Any, first_key: str, second_key: str) -> list[str]: + if isinstance(item, dict): + return [item.get(first_key, ""), item.get(second_key, "")] + return [getattr(item, first_key, ""), getattr(item, second_key, "")] + + +def plan_texts(proposal: StageProposal) -> list[str]: + """The plan fields that render to `proposal.md`, `design.md` and the spec deltas (#3308).""" + texts: list[str] = [ + proposal.why, + *proposal.what_changes, + *proposal.feasibility, + proposal.existing_tool, + *proposal.dependencies, + ] + for d in proposal.decisions: + texts.extend(_decision_texts(d, "decision", "reason")) + for r in proposal.rejected: + texts.extend(_decision_texts(r, "alternative", "evidence")) + for s in proposal.specs: + texts.extend(_spec_texts(s)) + return [t for t in texts if t] + + def _plan_texts(proposal: StageProposal) -> list[tuple[str, str]]: """The texts of the plan that must read as a sentence, each with the name of its field: `why`, `existing_tool`, the lines of `feasibility`, the decisions and the tasks of each group.""" @@ -1022,6 +1081,7 @@ class ProposalGate(StrEnum): DEPENDENCIES = "dependencies" CREATES = "creates" REGISTRY = "registry" + CONSTANTS = "constants" def check_gates( @@ -1038,6 +1098,7 @@ def check_gates( plan_creates: Mapping[str, int] | None = None, read_text: Callable[[str], str | None] | None = None, registry: Sequence[RegistryCount] | None = None, + constants: Mapping[str, int] | None = None, ) -> dict[ProposalGate, list[str]]: """What each gate of `check_proposal` finds wrong in `proposal`, one sentence each, under the name of the gate. Every gate has a key, and a gate that finds nothing has an empty list. The @@ -1067,6 +1128,7 @@ def check_gates( proposal, registry if registry is not None else registry_table(Path.cwd()), ), + ProposalGate.CONSTANTS: constant_problems(plan_texts(proposal), constants or {}), } @@ -1084,6 +1146,7 @@ def check_proposal( plan_creates: Mapping[str, int] | None = None, read_text: Callable[[str], str | None] | None = None, registry: Sequence[RegistryCount] | None = None, + constants: Mapping[str, int] | None = None, ) -> list[str]: """What the rules of `openspec/config.yaml` and the files of the commit find wrong in `proposal`, one sentence each. `known` holds the names of the capabilities under @@ -1117,6 +1180,7 @@ def check_proposal( plan_creates=plan_creates, read_text=read_text, registry=registry, + constants=constants, ) return [problem for problems in found.values() for problem in problems] diff --git a/src/devops_cli/roadmap/stage/stage_constants.py b/src/devops_cli/roadmap/stage/stage_constants.py new file mode 100644 index 000000000..a76a8d75e --- /dev/null +++ b/src/devops_cli/roadmap/stage/stage_constants.py @@ -0,0 +1,275 @@ +"""`devops roadmap stage`: check plan numbers against staged constants (#3308). + +A plan that carries a number from an issue or plan text is a claim that the number holds in the +code. This module extracts constant names and candidate numbers from plan texts, reads module-level +integer constants from the staged repository commit using `git grep`, `git show`, and `ast`, and +refuses a plan when a sentence states a different number. +""" + +from __future__ import annotations + +import ast +import re +from collections import defaultdict +from collections.abc import Iterable, Mapping +from pathlib import Path +from typing import Any, Final + +from devops_cli.exceptions.git import GitOperationError +from devops_cli.lang.en.messages import MESSAGES +from devops_cli.roadmap.clone_git import git + +# An uppercase identifier with at least two characters. +_CONSTANT_NAME: Final[re.Pattern[str]] = re.compile(r"\b[A-Z][A-Z0-9_]+\b") + +# Keywords in uppercase that are not constant names. +_EXCLUDED_NAMES: Final[frozenset[str]] = frozenset( + { + "ADDED", + "AND", + "AST", + "CI", + "HEAD", + "JSON", + "MODIFIED", + "MUST", + "NOT", + "OR", + "PR", + "REST", + "SHALL", + "SHOULD", + "THEN", + "WHEN", + "YAML", + } +) + +# A candidate decimal integer: not part of a note reference (#2714), line location (:12), +# date (2026-11-03), or version/path (v1.0.0, a/40). +_CANDIDATE_NUMBER: Final[re.Pattern[str]] = re.compile(r"(? set[str]: + """The constant names that `texts` cite: uppercase identifiers with at least two characters.""" + names: set[str] = set() + for text in texts: + for match in _CONSTANT_NAME.finditer(text): + name = match.group() + if name not in _EXCLUDED_NAMES: + names.add(name) + return names + + +def _sentences(text: str) -> list[str]: + """The sentences of `text`.""" + sentences: list[str] = [] + for block in _PARAGRAPH_SPLIT.split(text): + normalized = " ".join(block.split()) + if not normalized: + continue + for sentence in _SENTENCE_SPLIT.split(normalized): + sentence = sentence.strip() + if sentence: + sentences.append(sentence) + return sentences + + +def _eval_literal_or_alias(val_node: ast.expr | None) -> Any: + if val_node is None: + return _NON_LITERAL + if ( + isinstance(val_node, ast.Constant) + and isinstance(val_node.value, int) + and not isinstance(val_node.value, bool) + ): + return int(val_node.value) + if ( + isinstance(val_node, ast.UnaryOp) + and isinstance(val_node.op, ast.USub) + and isinstance(val_node.operand, ast.Constant) + and isinstance(val_node.operand.value, int) + and not isinstance(val_node.operand.value, bool) + ): + return -int(val_node.operand.value) + if isinstance(val_node, ast.Name): + return val_node.id + return _NON_LITERAL + + +def _stmt_targets_and_val(stmt: ast.stmt) -> tuple[list[str], ast.expr | None]: + if isinstance(stmt, ast.Assign): + targets = [t.id for t in stmt.targets if isinstance(t, ast.Name)] + return targets, stmt.value + if isinstance(stmt, ast.AnnAssign) and isinstance(stmt.target, ast.Name): + return [stmt.target.id], stmt.value + return [], None + + +def _parse_module_definitions(source: str, filename: str) -> dict[str, list[Any]]: + try: + tree = ast.parse(source, filename=filename) + except SyntaxError: + return {} + defs: dict[str, list[Any]] = defaultdict(list) + for stmt in tree.body: + targets, val_node = _stmt_targets_and_val(stmt) + if not targets or val_node is None: + continue + parsed = _eval_literal_or_alias(val_node) + for target in targets: + defs[target].append(parsed) + return defs + + +def _extract_raw_definitions(sources: Mapping[str, str]) -> dict[str, set[Any]]: + """The raw definitions found at module level in `sources`: integer literals, aliases, or non-literals.""" + definitions: dict[str, set[Any]] = defaultdict(set) + for path, source in sources.items(): + for name, values in _parse_module_definitions(source, path).items(): + definitions[name].update(values) + return definitions + + +def _resolve_constant( + name: str, + definitions: Mapping[str, set[Any]], + visited: set[str], +) -> int | None: + if name in visited: + return None + visited.add(name) + defs = definitions.get(name) + if not defs: + return None + resolved: set[int] = set() + for item in defs: + if item is _NON_LITERAL: + return None + if isinstance(item, int): + resolved.add(item) + elif isinstance(item, str): + alias_val = _resolve_constant(item, definitions, visited.copy()) + if alias_val is None: + return None + resolved.add(alias_val) + else: + return None + if len(resolved) == 1: + return next(iter(resolved)) + return None + + +def parse_constants( + sources: Mapping[str, str], + names: Iterable[str] | None = None, +) -> dict[str, int]: + """Parse module-level integer constants from `sources`, resolving aliases across modules.""" + definitions = _extract_raw_definitions(sources) + result: dict[str, int] = {} + candidate_keys = set(names) if names is not None else set(definitions) + for key in candidate_keys: + value = _resolve_constant(key, definitions, set()) + if value is not None: + result[key] = value + return result + + +def _git_grep_paths(source: Path, sha: str, names: set[str]) -> list[str]: + args = ["grep", "-l"] + for name in sorted(names): + args.extend(["-e", name]) + args.extend([sha, "--", "src/"]) + try: + output = git(source, *args, operation="stage constants grep") + except GitOperationError: + return [] + prefix = f"{sha}:" + return [ + line.removeprefix(prefix).strip() + for line in output.splitlines() + if line.strip() and line.removeprefix(prefix).strip().endswith(".py") + ] + + +def _load_git_sources(source: Path, sha: str, paths: list[str], loaded: dict[str, str]) -> None: + for path in paths: + if path in loaded: + continue + try: + loaded[path] = git(source, "show", f"{sha}:{path}", operation="stage constants show") + except GitOperationError: + continue + + +def _discover_aliases(loaded: Mapping[str, str], searched: set[str]) -> set[str]: + new_aliases: set[str] = set() + for defs in _extract_raw_definitions(loaded).values(): + for item in defs: + if isinstance(item, str) and item not in searched: + new_aliases.add(item) + return new_aliases + + +def staged_constants(source: Path, sha: str, names: Iterable[str]) -> dict[str, int]: + """Find each constant in `names` at `sha` in `source`, read the files with `git show`, and parse with AST.""" + candidate_set = set(names) + if not candidate_set: + return {} + + loaded_sources: dict[str, str] = {} + pending_names = set(candidate_set) + searched_names: set[str] = set() + + while pending_names: + to_search = pending_names - searched_names + if not to_search: + break + searched_names.update(to_search) + matched = _git_grep_paths(source, sha, to_search) + _load_git_sources(source, sha, matched, loaded_sources) + pending_names.update(_discover_aliases(loaded_sources, searched_names)) + + return parse_constants(loaded_sources, names=candidate_set) + + +def constant_problems(texts: Iterable[str], constants: Mapping[str, int]) -> list[str]: + """One problem for each sentence that names one known constant and holds one candidate number + that differs from the staged tree value (#3308).""" + if not constants: + return [] + problems: list[str] = [] + texts_msg = MESSAGES.roadmap + for text in texts: + for sentence in _sentences(text): + known = [name for name in constants if re.search(rf"\b{re.escape(name)}\b", sentence)] + if len(known) != 1: + continue + numbers = [int(m.group(1)) for m in _CANDIDATE_NUMBER.finditer(sentence)] + if len(numbers) != 1: + continue + constant = known[0] + plan_value = numbers[0] + staged_value = constants[constant] + if plan_value != staged_value: + problems.append( + texts_msg.stage_problem_constant_mismatch.format( + constant=constant, + staged=staged_value, + plan=plan_value, + ) + ) + return list(dict.fromkeys(problems)) + + +__all__ = [ + "candidate_names", + "constant_problems", + "parse_constants", + "staged_constants", +] diff --git a/src/devops_cli/roadmap/stage/stage_eval_cases.py b/src/devops_cli/roadmap/stage/stage_eval_cases.py index add2898c7..14a75a603 100644 --- a/src/devops_cli/roadmap/stage/stage_eval_cases.py +++ b/src/devops_cli/roadmap/stage/stage_eval_cases.py @@ -162,6 +162,7 @@ class CaseFacts(_Record): tracked: tuple[str, ...] capabilities: tuple[str, ...] search_evidence: Mapping[str, tuple[str, ...]] = Field(default_factory=dict) + constants: tuple[tuple[str, int], ...] = () class StageCase(_Record): diff --git a/src/devops_cli/roadmap/stage/stage_eval_gates.py b/src/devops_cli/roadmap/stage/stage_eval_gates.py index 0a1b2fa33..600f2bec4 100644 --- a/src/devops_cli/roadmap/stage/stage_eval_gates.py +++ b/src/devops_cli/roadmap/stage/stage_eval_gates.py @@ -79,6 +79,7 @@ class PlanUnderTest: # records no search, as the cases recorded before #3410 do. search_evidence: Mapping[str, Sequence[str]] = field(default_factory=dict) registry: Sequence[RegistryCount] | None = None + constants: Mapping[str, int] = field(default_factory=dict) @classmethod def of_case( @@ -111,6 +112,7 @@ def of_case( mapped="criteria" in case.plan, search_evidence=case.facts.search_evidence, registry=registry, + constants=dict(case.facts.constants), ) @classmethod @@ -173,6 +175,7 @@ def _path_gate(plan: PlanUnderTest) -> list[str]: requirements=plan.requirements, search_evidence=plan.search_evidence or None, registry=plan.registry, + constants=plan.constants, ) diff --git a/tests/roadmap/stage/test_roadmap_stage_constants.py b/tests/roadmap/stage/test_roadmap_stage_constants.py new file mode 100644 index 000000000..bdd071932 --- /dev/null +++ b/tests/roadmap/stage/test_roadmap_stage_constants.py @@ -0,0 +1,92 @@ +"""`devops roadmap stage`: check plan numbers against staged constants (#3308).""" + +from __future__ import annotations + +from devops_cli.roadmap.stage.stage_constants import ( + candidate_names, + constant_problems, + parse_constants, + staged_constants, +) +from tests.support.paths import REPO_ROOT + + +def test_parse_constants_reads_annotated_assignment() -> None: + source = "SUMMARY_MAX_CHARS: Final[int] = 60\n" + assert parse_constants({"src/devops_cli/roadmap/pull_form.py": source}) == { + "SUMMARY_MAX_CHARS": 60 + } + + +def test_parse_constants_resolves_alias_across_modules() -> None: + sources = { + "src/devops_cli/roadmap/pull_form.py": ( + "BODY_MAX_CHARS = CONST_ROADMAP_IMPLEMENT_BODY_MAX_CHARS\n" + ), + "src/devops_cli/config/constants.py": "CONST_ROADMAP_IMPLEMENT_BODY_MAX_CHARS = 1000\n", + } + assert parse_constants(sources) == { + "BODY_MAX_CHARS": 1000, + "CONST_ROADMAP_IMPLEMENT_BODY_MAX_CHARS": 1000, + } + + +def test_parse_constants_leaves_out_call_value() -> None: + source = "SUMMARY_MAX_CHARS = int('60')\n" + assert parse_constants({"src/devops_cli/roadmap/pull_form.py": source}) == {} + + +def test_parse_constants_leaves_out_ambiguous_definition() -> None: + sources = { + "src/devops_cli/module_a.py": "SUMMARY_MAX_CHARS = 40\n", + "src/devops_cli/module_b.py": "SUMMARY_MAX_CHARS = 60\n", + } + assert parse_constants(sources) == {} + + +def test_candidate_names_extracts_constant_identifiers() -> None: + texts = [ + "A requirement says `SUMMARY_MAX_CHARS` is 40.", + "WHEN `BODY_MAX_CHARS` is 1000.", + ] + assert candidate_names(texts) == {"SUMMARY_MAX_CHARS", "BODY_MAX_CHARS"} + + +def test_constant_problems_yields_problem_on_mismatch() -> None: + texts = ["A plan text with `SUMMARY_MAX_CHARS` and 40 in one sentence."] + constants = {"SUMMARY_MAX_CHARS": 60} + assert constant_problems(texts, constants) == [ + "SUMMARY_MAX_CHARS is 60 in the staged tree, but the plan says 40." + ] + + +def test_constant_problems_accepts_matching_number() -> None: + texts = ["A plan text with `SUMMARY_MAX_CHARS` and 60 in one sentence."] + constants = {"SUMMARY_MAX_CHARS": 60} + assert constant_problems(texts, constants) == [] + + +def test_constant_problems_ignores_note_reference() -> None: + texts = ["A requirement says `SUMMARY_MAX_CHARS` is 60 (#2714)."] + constants = {"SUMMARY_MAX_CHARS": 60} + assert constant_problems(texts, constants) == [] + + +def test_constant_problems_ignores_line_location() -> None: + texts = ["See pull_form.py:12 where `SUMMARY_MAX_CHARS` is 60."] + constants = {"SUMMARY_MAX_CHARS": 60} + assert constant_problems(texts, constants) == [] + + +def test_constant_problems_skips_sentence_with_two_candidate_numbers() -> None: + texts = [ + "A design decision says `SUMMARY_MAX_CHARS` is 40, while the staged tree sets it to 60." + ] + constants = {"SUMMARY_MAX_CHARS": 60} + assert constant_problems(texts, constants) == [] + + +def test_staged_constants_reads_repository() -> None: + # `src/devops_cli/roadmap/pull_form.py` defines `SUMMARY_MAX_CHARS: Final[int] = 60` + constants = staged_constants(REPO_ROOT, "HEAD", ["SUMMARY_MAX_CHARS"]) + assert constants.get("SUMMARY_MAX_CHARS") == 60 diff --git a/tests/roadmap/stage/test_roadmap_stage_eval_gates.py b/tests/roadmap/stage/test_roadmap_stage_eval_gates.py index a98c33bd2..d99d86b1b 100644 --- a/tests/roadmap/stage/test_roadmap_stage_eval_gates.py +++ b/tests/roadmap/stage/test_roadmap_stage_eval_gates.py @@ -485,3 +485,42 @@ def test_a_content_gate_that_stops_refusing_a_plan_fails_the_replay( DefectStatus.MISSED ] assert any("no longer refuses" in failure for failure in replay.failures) + + +def test_the_replay_passes_recorded_constants(cases: list[StageCase]) -> None: + good_case = next(case for case in cases if case.verdict is Verdict.GOOD) + replay = replay_case(good_case) + assert not replay.refused + + specs = list(good_case.plan["specs"]) + spec = specs[0] + bad_spec = { + **spec, + "added": [ + { + "name": "Summary bound", + "text": "The summary SHALL be bounded.", + "scenarios": [ + { + "name": "Mismatch", + "when": "A spec scenario says `SUMMARY_MAX_CHARS` is 40.", + "then": "Refuse.", + } + ], + } + ], + } + facts_with_constants = good_case.facts.model_copy( + update={"constants": (("SUMMARY_MAX_CHARS", 60),)} + ) + case_with_constants = good_case.model_copy( + update={ + "facts": facts_with_constants, + "plan": {**good_case.plan, "specs": [bad_spec]}, + } + ) + replay_constants = replay_case(case_with_constants) + assert any( + "SUMMARY_MAX_CHARS is 60 in the staged tree, but the plan says 40." in p + for p in replay_constants.problems[GateName.PATHS] + ) diff --git a/tests/roadmap/stage/test_roadmap_stage_plan.py b/tests/roadmap/stage/test_roadmap_stage_plan.py index 5e2f296d6..6f0e07c81 100644 --- a/tests/roadmap/stage/test_roadmap_stage_plan.py +++ b/tests/roadmap/stage/test_roadmap_stage_plan.py @@ -2424,3 +2424,41 @@ def test_check_proposal_refuses_a_new_roadmap_limit_without_ratchet_task() -> No plan_with_task = make_stage_proposal(groups=[group_with_task]) problems_with_task = check_proposal(plan_with_task, ["comment-thread"], tree) assert not any("tests/roadmap/test_roadmap_limit_classes.py" in p for p in problems_with_task) + + +def test_check_proposal_refuses_a_plan_number_differing_from_a_constant() -> None: + constants = {"SUMMARY_MAX_CHARS": 60} + spec = make_stage_proposal().specs[0] + req = spec.added[0] + + scenario_40 = req.scenarios[0].model_copy( + update={"when": "A spec scenario says `SUMMARY_MAX_CHARS` is 40."} + ) + spec_40 = spec.model_copy( + update={"added": [req.model_copy(update={"scenarios": [scenario_40]})]} + ) + proposal_40 = make_stage_proposal(specs=[spec_40]) + problems = check_proposal(proposal_40, ["comment-thread"], SAMPLE_TREE, constants=constants) + assert problems == ["SUMMARY_MAX_CHARS is 60 in the staged tree, but the plan says 40."] + + scenario_60 = req.scenarios[0].model_copy( + update={"when": "A spec scenario says `SUMMARY_MAX_CHARS` is 60."} + ) + spec_60 = spec.model_copy( + update={"added": [req.model_copy(update={"scenarios": [scenario_60]})]} + ) + proposal_60 = make_stage_proposal(specs=[spec_60]) + assert check_proposal(proposal_60, ["comment-thread"], SAMPLE_TREE, constants=constants) == [] + + # Returns none for a proposal with no constants + assert check_proposal(proposal_40, ["comment-thread"], SAMPLE_TREE, constants={}) == [] + assert check_proposal(proposal_40, ["comment-thread"], SAMPLE_TREE) == [] + + # Returns none for the text of a task group that holds 40 + group_40 = ( + make_stage_proposal() + .groups[0] + .model_copy(update={"implementation": "Set `SUMMARY_MAX_CHARS` to 40 in code."}) + ) + proposal_task = make_stage_proposal(groups=[group_40]) + assert check_proposal(proposal_task, ["comment-thread"], SAMPLE_TREE, constants=constants) == [] diff --git a/tests/roadmap/stage/test_roadmap_stage_prompt_rules.py b/tests/roadmap/stage/test_roadmap_stage_prompt_rules.py index c80fe9442..8fa8a2af2 100644 --- a/tests/roadmap/stage/test_roadmap_stage_prompt_rules.py +++ b/tests/roadmap/stage/test_roadmap_stage_prompt_rules.py @@ -543,3 +543,16 @@ def test_the_prompts_state_the_prompt_rule_test_rule(task: str) -> None: "`tests/roadmap/stage/test_roadmap_stage_prompt_rules.py` in `reads` or `changes` and " "gives it as `verify`." ) in prompt + + +# ── Cite staged constant values (#3308) ─────────────────────────────────────── + + +def test_the_stage_prompt_requires_citation_of_staged_constant_values() -> None: + prompt = load_task_prompt("roadmap_stage_change.md") + + assert ( + "Cite the staged value of each constant that the plan relies on, with the constant name " + "and its value, as `feasibility` quotes the probe lines, and give no new name to the " + "bound of a constant that the staged tree defines." + ) in prompt