diff --git a/docs/roadmap/plan-recheck.md b/docs/roadmap/plan-recheck.md index c5fbff65b..72d36e0fc 100644 --- a/docs/roadmap/plan-recheck.md +++ b/docs/roadmap/plan-recheck.md @@ -63,6 +63,7 @@ Each refusal of a `new-plan` folder prints with the name of its gate, in square | :--- | :--- | | `spec-delta` | A spec delta that the plan needs and lacks, or that it does not need, or a bad requirement or capability. | | `content` | A text that is too short or a placeholder, a `feasibility` that quotes no probe line, a task that names a message and changes no message file, and a plan that builds its own code and gives no cost. | +| `citation` | A citation of a file that the staged commit does not hold as cited: a `path:line:text` line with other text, or a `path:def name` symbol that the file lacks. | | `form` | A task text of one word, a `verify` command outside the project form, and a new test module outside the mirror directories. | | `eval-replay` | A plan that changes the stage prompt or a module of the stage checks and holds no task that runs the replay and states the score before and after. | | `paths` | A path that the tree lacks and that no group up to the one that names it creates. | diff --git a/openspec/changes/3422-the-plan-check-fails-a-citation/.openspec.yaml b/openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/.openspec.yaml similarity index 100% rename from openspec/changes/3422-the-plan-check-fails-a-citation/.openspec.yaml rename to openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/.openspec.yaml diff --git a/openspec/changes/3422-the-plan-check-fails-a-citation/design.md b/openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/design.md similarity index 100% rename from openspec/changes/3422-the-plan-check-fails-a-citation/design.md rename to openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/design.md diff --git a/openspec/changes/3422-the-plan-check-fails-a-citation/proposal.md b/openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/proposal.md similarity index 100% rename from openspec/changes/3422-the-plan-check-fails-a-citation/proposal.md rename to openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/proposal.md diff --git a/openspec/changes/3422-the-plan-check-fails-a-citation/specs/roadmap-stage/spec.md b/openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/specs/roadmap-stage/spec.md similarity index 100% rename from openspec/changes/3422-the-plan-check-fails-a-citation/specs/roadmap-stage/spec.md rename to openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/specs/roadmap-stage/spec.md diff --git a/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md b/openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/tasks.md similarity index 91% rename from openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md rename to openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/tasks.md index 67dc0ae3f..82a6de860 100644 --- a/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md +++ b/openspec/changes/archive/2026-11-06-3422-the-plan-check-fails-a-citation/tasks.md @@ -4,25 +4,25 @@ Reads: `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage_paths.py`, `tests/roadmap/test_roadmap_stage_content.py`. Changes: `src/devops_cli/roadmap/stage_change.py`, `tests/roadmap/test_roadmap_stage_content.py`. -- [ ] 1.1 In `tests/roadmap/test_roadmap_stage_content.py`, add a failing test that passes a dict as `read_text` to `check_proposal`: a citation of line 4 that holds other text is refused with the field and the path, a citation of the line that holds the text is accepted, a line past the end is refused, a `path:def f` citation of a file that defines `def f(` at line 7 is accepted, a `path:def f` citation of a file that defines only `def g(` is refused, a stale citation of `what_changes` names that field, a plan with no `read_text` gets no citation problem, and a citation whose spacing differs is accepted +- [x] 1.1 In `tests/roadmap/test_roadmap_stage_content.py`, add a failing test that passes a dict as `read_text` to `check_proposal`: a citation of line 4 that holds other text is refused with the field and the path, a citation of the line that holds the text is accepted, a line past the end is refused, a `path:def f` citation of a file that defines `def f(` at line 7 is accepted, a `path:def f` citation of a file that defines only `def g(` is refused, a stale citation of `what_changes` names that field, a plan with no `read_text` gets no citation problem, and a citation whose spacing differs is accepted - Verification: `uv run devops ci test tests/roadmap/test_roadmap_stage_content.py` fails on the base -- [ ] 1.2 Add a helper in `src/devops_cli/roadmap/stage_change.py` that finds each citation of the form `path:line:text`, `path:def name` and `path:class name` in `feasibility`, `why` and each line of `what_changes`. Give `check_proposal` an optional `read_text: Callable[[str], str | None] | None = None`, default it to `None`, and call the helper with the texts of the three fields and that reader. Refuse a citation of fewer than 3 characters, and compare the cited text with the cited line as a substring after collapsing white space in both and stripping backticks and one trailing period. Refuse a citation whose path or line the reader does not return. Name the field and the cited path in each problem +- [x] 1.2 Add a helper in `src/devops_cli/roadmap/stage_change.py` that finds each citation of the form `path:line:text`, `path:def name` and `path:class name` in `feasibility`, `why` and each line of `what_changes`. Give `check_proposal` an optional `read_text: Callable[[str], str | None] | None = None`, default it to `None`, and call the helper with the texts of the three fields and that reader. Refuse a citation of fewer than 3 characters, and compare the cited text with the cited line as a substring after collapsing white space in both and stripping backticks and one trailing period. Refuse a citation whose path or line the reader does not return. Name the field and the cited path in each problem - Verification: `uv run devops ci test tests/roadmap/test_roadmap_stage_content.py` passes ## 2. Pass a reader of the staged commit to check_proposal Reads: `src/devops_cli/roadmap/stage.py`, `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/refine.py`, `src/devops_cli/roadmap/stage_eval_gates.py`. Changes: `src/devops_cli/roadmap/stage.py`. -- [ ] 2.1 In `src/devops_cli/roadmap/stage.py`, where `_Planner` calls `check_proposal` in `_settle`, pass a `read_text` callable built from `self._source` and `self._sha` that returns the text of a cited file at the staged commit with the `git show :` read (the form `refine.py` uses in `_read_git_file`), or `None` when the file is absent at that commit, so the check reads the staged commit and not the working tree +- [x] 2.1 In `src/devops_cli/roadmap/stage.py`, where `_Planner` calls `check_proposal` in `_settle`, pass a `read_text` callable built from `self._source` and `self._sha` that returns the text of a cited file at the staged commit with the `git show :` read (the form `refine.py` uses in `_read_git_file`), or `None` when the file is absent at that commit, so the check reads the staged commit and not the working tree - Verification: `uv run devops ci test tests/roadmap/test_roadmap_stage_plan.py` passes -- [ ] 2.2 Confirm the replay of the golden set in `src/devops_cli/roadmap/stage_eval_gates.py` and the plan-check command in `src/devops_cli/roadmap/plan_check.py` call `check_proposal` with no reader, so the default `read_text` judges no citation and the recorded plans keep passing +- [x] 2.2 Confirm the replay of the golden set in `src/devops_cli/roadmap/stage_eval_gates.py` and the plan-check command in `src/devops_cli/roadmap/plan_check.py` call `check_proposal` with no reader, so the default `read_text` judges no citation and the recorded plans keep passing - Verification: `uv run devops roadmap eval stage --replay` passes ## 3. Ask the planner for a symbol citation Reads: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/test_roadmap_stage_prompt_rules.py`, `docs/ROADMAP_WORKFLOW.md`. Changes: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/test_roadmap_stage_prompt_rules.py`. -- [ ] 3.1 Add a rule to the stage prompt `src/devops_cli/ai/tasks/roadmap_stage_change.md` that states the `path:line:text`, `path:def name` and `path:class name` citation forms, and tells the planner to cite a symbol instead of a line number when a line is not needed. Add a test in `tests/roadmap/test_roadmap_stage_prompt_rules.py` that reads the prompt and asserts the symbol form and the instruction to prefer it +- [x] 3.1 Add a rule to the stage prompt `src/devops_cli/ai/tasks/roadmap_stage_change.md` that states the `path:line:text`, `path:def name` and `path:class name` citation forms, and tells the planner to cite a symbol instead of a line number when a line is not needed. Add a test in `tests/roadmap/test_roadmap_stage_prompt_rules.py` that reads the prompt and asserts the symbol form and the instruction to prefer it - Verification: `uv run devops ci test tests/roadmap/test_roadmap_stage_prompt_rules.py` passes ## Workflow follow-up diff --git a/openspec/specs/roadmap-stage/spec.md b/openspec/specs/roadmap-stage/spec.md index 596d93dd3..24fd89080 100644 --- a/openspec/specs/roadmap-stage/spec.md +++ b/openspec/specs/roadmap-stage/spec.md @@ -2375,6 +2375,80 @@ The prompt of the verification pass SHALL define `criterion_dropped` for a crite - **WHEN** The prompt of the verification pass is read. - **THEN** The line of `criterion_dropped` names a group that changes only test files, names the earlier group that changes the code, and says that such a group is no `criterion_dropped`. +### Requirement: Refuse a stale line citation +The plan check SHALL refuse a `path:line:text` citation when the cited line of the staged file does not hold the cited text. + +#### Scenario: The line holds another text +- **WHEN** `feasibility` holds `src/sample.py:4:def f():` and line 4 of the staged `src/sample.py` holds `def g():`. +- **THEN** `check_proposal` returns a problem that names `feasibility` and `src/sample.py`. + +### Requirement: Accept a true line citation +The plan check SHALL accept a `path:line:text` citation when the cited line of the staged file holds the cited text. + +#### Scenario: The line holds the cited text +- **WHEN** `feasibility` holds `src/sample.py:4:def f():` and line 4 of the staged `src/sample.py` holds `def f():`. +- **THEN** `check_proposal` returns no problem that names `src/sample.py`. + +### Requirement: Refuse a citation of an absent line +The plan check MUST refuse a citation when the reader returns no such path or the file holds no such line. + +#### Scenario: The line number is past the end of the file +- **WHEN** `feasibility` holds `src/sample.py:99:def f():` and the reader returns the 10 lines of `src/sample.py`. +- **THEN** `check_proposal` returns a problem that names `feasibility` and `src/sample.py`. + +### Requirement: Resolve a symbol citation +The plan check SHALL accept a `path:def name` or a `path:class name` citation when the staged file defines the symbol at any line. + +#### Scenario: The definition moved +- **WHEN** `feasibility` holds `src/sample.py:def f` and the staged `src/sample.py` defines `def f(` at line 7. +- **THEN** `check_proposal` returns no problem that names `src/sample.py`. + +### Requirement: Refuse an absent symbol +The plan check MUST refuse a symbol citation when the staged file holds no definition of the symbol. + +#### Scenario: The file defines another symbol +- **WHEN** `feasibility` holds `src/sample.py:def f` and the staged `src/sample.py` defines only `def g(`. +- **THEN** `check_proposal` returns a problem that names `feasibility` and `src/sample.py`. + +### Requirement: Scan the three evidence fields +The plan check SHALL scan `feasibility`, `why` and each line of `what_changes` for citations. + +#### Scenario: A stale citation sits in what_changes +- **WHEN** a line of `what_changes` holds a citation of a line that holds other text. +- **THEN** `check_proposal` returns a problem that names `what_changes` and the cited path. + +### Requirement: Judge no citation without file text +The plan check SHALL judge no citation when it holds no reader of the staged files. + +#### Scenario: The check holds no reader +- **WHEN** `check_proposal` runs with no `read_text` callable and the plan holds a stale citation. +- **THEN** `check_proposal` returns no problem for that citation. + +### Requirement: Compare a citation with collapsed white space +The plan check SHALL compare the cited text with the staged line after it collapses white space in both. + +#### Scenario: The spacing differs +- **WHEN** `feasibility` holds `src/sample.py:4:def f():` and line 4 holds `def f():`. +- **THEN** `check_proposal` returns no problem that names `src/sample.py`. + +### Requirement: The stage job reads the text of a cited file at the staged commit +The stage job SHALL pass `check_proposal` a reader that returns the text of a cited file at the staged commit, and the replay MUST pass no reader. + +#### Scenario: The job holds the checkout and the commit +- **WHEN** the stage job holds the checkout and the staged commit, and a plan cites `src/sample.py`, and the reader is asked for its text. +- **THEN** the reply holds the lines of `src/sample.py` at that commit. + +#### Scenario: The replay passes no reader +- **WHEN** the replay of the golden set calls `check_proposal` with no reader, and a plan cites `src/sample.py`. +- **THEN** the check judges no citation. + +### Requirement: The stage prompt prefers a symbol citation +The stage prompt MUST tell the planner to cite a symbol instead of a line number when a line is not needed. + +#### Scenario: The prompt states the symbol form +- **WHEN** a test reads `src/devops_cli/ai/tasks/roadmap_stage_change.md`. +- **THEN** the text holds the `path:def name` form and the instruction to prefer it. + ### Requirement: Two tasks of a plan state different work `check_proposal` SHALL refuse two tasks whose normalized work text is the same, and SHALL name both task numbers and the group or groups of those tasks. diff --git a/src/devops_cli/ai/tasks/roadmap_stage_change.md b/src/devops_cli/ai/tasks/roadmap_stage_change.md index d3b4c7b2c..51bd95f99 100644 --- a/src/devops_cli/ai/tasks/roadmap_stage_change.md +++ b/src/devops_cli/ai/tasks/roadmap_stage_change.md @@ -9,6 +9,7 @@ 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. - 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. +- When `feasibility`, `why` or an item of `what_changes` cites a line of a file, write the citation as `path:line:text`, with the text of that line in a code span, or cite a symbol as `path:def name` or `path:class name`. The check reads each citation at the staged commit and refuses one whose line does not hold the text, one whose symbol the file does not define, and one that quotes fewer than 3 characters of the line. Cite a symbol instead of a line number when a line is not needed: a symbol citation stays true when the code moves, and a line number copied from another commit does not. - In `existing_tool`, name the open, maintained tool or library that covers the need. When the change builds its own code instead, give the written cost: the tokens to build, keep and fix it, and the elapsed time. Name the internal code that the change deletes. - A change that alters no behavior that a consumer observes (tests, workflows, documents, a refactor that keeps behavior) has no spec delta: set `skip_specs` to `true` and give `specs` as an empty list. A `devops` command, option or message, a manifest and the behavior of a service are observable, so the change carries a spec delta. Set `skip_specs` from the paths of `changes`: when every path is under `tests/`, `.github/workflows/`, `docs/` or `openspec/`, the check refuses a spec delta. - Reuse a capability from `capabilities`: choose it by its `purpose`, and add requirements to it. When `capabilities` holds only names, call `read_spec` with a name to read its purpose and its requirement names, and choose by the purpose. Never add a requirement with the name of one in its `requirements`; the check refuses it. Change that requirement with `modified` instead. A capability that is not in the list is new, and its entry gives a `purpose` of at least 50 characters. Change an existing requirement, with `modified`, only when the capability exists. A `modified` requirement keeps the name of an existing requirement exactly and holds all of its scenarios, because it replaces the whole requirement. Write only scenarios that a test can assert offline: tests double only process edges and reach no network, no cluster and no real scanner. Add no requirement for compatibility with an old behavior. diff --git a/src/devops_cli/lang/en/messages.py b/src/devops_cli/lang/en/messages.py index acbaf07aa..86f52c604 100644 --- a/src/devops_cli/lang/en/messages.py +++ b/src/devops_cli/lang/en/messages.py @@ -3545,6 +3545,16 @@ class RoadmapMessages: "The open issue #{number} plans to create `{path}`. This plan creates it too. Remove the " "path from this plan, or wait until #{number} closes." ) + stage_problem_citation_stale: str = ( + "The {field} of the plan cites `{path}`, and the staged file holds no such text there. " + "Cite the text of the line in a code span at the staged commit, or cite a symbol as " + "`path:def name` or `path:class name`, which stays true when the line moves." + ) + stage_problem_citation_short: str = ( + "The {field} of the plan cites `{path}` with fewer than {least} characters of the line. " + "Quote at least {least} characters of the line, or cite a symbol as `path:def name` or " + "`path:class name`." + ) 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.py b/src/devops_cli/roadmap/stage.py index 6b7a470d0..491a7b40d 100644 --- a/src/devops_cli/roadmap/stage.py +++ b/src/devops_cli/roadmap/stage.py @@ -148,6 +148,7 @@ NextStep, body_dependencies, collect_context, + git_result, inspect_checkout, recorded_blockers, stage_comment_text, @@ -1101,6 +1102,9 @@ def _settle( overlaps=self._overlaps, search_evidence=evidence, plan_creates=self._creators(), + # The citation rule reads the staged commit, the one the probe and the model read + # (#3422), and never the working tree. + read_text=partial(_read_staged_file, self._source, self._sha), ) if problems: return _Refused(" ".join(problems), proposal) @@ -1611,6 +1615,14 @@ def _git(cwd: Path, *args: str) -> str: return git(cwd, *args, operation=_GIT_OPERATION) +def _read_staged_file(source: Path, sha: str, path: str) -> str | None: + """The text of `path` at the staged commit `sha`, or None when that commit holds no such file + (#3422). The citation check reads the commit that the job plans on, and not the working tree, + whose lines can differ from that commit.""" + shown, content = git_result(source, "show", f"{sha}:{path}") + return content if shown == 0 else None + + def _move_clone(checkout: Path, sha: str) -> None: """Move the clone to the staged commit with a fast-forward, so that its HEAD, its index and its files are that commit. The toolbox refuses any other HEAD (#3301). A clone that has diff --git a/src/devops_cli/roadmap/stage_change.py b/src/devops_cli/roadmap/stage_change.py index 62abbfc9d..dcde24538 100644 --- a/src/devops_cli/roadmap/stage_change.py +++ b/src/devops_cli/roadmap/stage_change.py @@ -3,9 +3,10 @@ The reply is one `StageProposal`. `check_proposal` finds what the rules of `openspec/config.yaml` ask and a schema cannot say: a Purpose on a new capability, a MODIFIED requirement only on a -capability that exists, a spec delta only when the change is observable, and a path that is a file -of the staged commit or that a group creates (`stage_paths`). `render_change` -turns the reply into the files of `openspec/changes/-/`. `measure_groups` counts the +capability that exists, a spec delta only when the change is observable, a path that is a file +of the staged commit or that a group creates (`stage_paths`), and a citation of a line or a symbol +that the staged file does not hold (#3422). `render_change` turns the reply into the files of +`openspec/changes/-/`. `measure_groups` counts the inputs of one implementation step for each group with tiktoken. It counts the files that the tasks of the group name, each packed as the step packs it (`step_files`), so a group costs what its step reads. @@ -16,7 +17,7 @@ import re import string import unicodedata -from collections.abc import Collection, Mapping, Sequence +from collections.abc import Callable, Collection, Mapping, Sequence from dataclasses import dataclass from datetime import date from enum import StrEnum @@ -119,6 +120,20 @@ # in `changes`, so the stage check names it when no group lists it (#3405). _MESSAGES_FILE: Final[str] = "src/devops_cli/lang/en/messages.py" _MESSAGE_NAME: Final[re.Pattern[str]] = re.compile(r"\bMESSAGES\.roadmap\b") +# A plan cites a file of the staged commit in its evidence fields: `path:line:text`, or +# `path:def name` and `path:class name`, which stay true when the code moves (#3422). The path +# starts at a word boundary, so the host of a URL is no citation, and the cited text of a line +# ends at the backtick that closes its code span. +_CITED_PATH: Final[str] = r"[A-Za-z0-9_][A-Za-z0-9_./-]*\.[A-Za-z0-9_]+" +_LINE_CITATION: Final[re.Pattern[str]] = re.compile( + rf"(?{_CITED_PATH}):(?P\d+):(?P[^`]*)" +) +_SYMBOL_CITATION: Final[re.Pattern[str]] = re.compile( + rf"(?{_CITED_PATH}):(?Pdef|class)\s+(?P[A-Za-z_]\w*)" +) +# The fewest characters of the cited text that the check judges: a shorter text matches almost any +# line, so the check refuses it and asks for a longer one. +_CITATION_MIN_CHARS: Final[int] = 3 # The plan check itself. A plan that adds a rule of it writes the text that the new rule judges, # so the plan gives a group test that builds its own `StageProposal` and asserts that # `content_problems` returns no problem for it (#3404). @@ -675,6 +690,98 @@ def _what_changes_problems(proposal: StageProposal, tree: StagedTree) -> list[st ] +def _evidence_texts(proposal: StageProposal) -> list[tuple[str, str]]: + """The texts of the plan that hold evidence, each with the name of its field: `why`, the lines + of `feasibility` and each line of `what_changes` (#3422).""" + named = [("why", proposal.why)] + named.extend(("feasibility", line) for line in proposal.feasibility) + named.extend(("what_changes", line) for line in proposal.what_changes) + return named + + +def _cited_text(text: str) -> str: + """The cited text as the check compares it: without its backticks and its one trailing full + stop, and with each run of white space as one space, as a line of the file reads.""" + return _squash(text.replace("`", "").strip().removesuffix(".")) + + +def _line_at(content: str, number: int) -> str | None: + """The line `number` of `content`, counting from 1, or None when the file is shorter.""" + lines = content.splitlines() + return lines[number - 1] if 1 <= number <= len(lines) else None + + +def _defines(content: str, kind: str, symbol: str) -> bool: + """Whether `content` defines `symbol` with `kind` at any line, as `def name` or + `class name` does.""" + pattern = re.compile(rf"\b{re.escape(kind)}\s+{re.escape(symbol)}\b") + return any(pattern.search(_squash(line)) for line in content.splitlines()) + + +def _line_citation_problem( + field: str, path: str, line: str, text: str, content: str | None +) -> str | None: + """The problem of one `path:line:text` citation, or None when the staged line holds the text.""" + if content is None: + return MESSAGES.roadmap.stage_problem_citation_stale.format(field=field, path=path) + cited = _cited_text(text) + if len(cited) < _CITATION_MIN_CHARS: + return MESSAGES.roadmap.stage_problem_citation_short.format( + field=field, path=path, least=_CITATION_MIN_CHARS + ) + cited_line = _line_at(content, int(line)) + if cited_line is not None and cited in _squash(cited_line): + return None + return MESSAGES.roadmap.stage_problem_citation_stale.format(field=field, path=path) + + +def _symbol_citation_problem( + field: str, path: str, kind: str, symbol: str, content: str | None +) -> str | None: + """The problem of one `path:def name` or `path:class name` citation, or None when the staged + file defines the symbol at some line.""" + if content is not None and _defines(content, kind, symbol): + return None + return MESSAGES.roadmap.stage_problem_citation_stale.format(field=field, path=path) + + +def _citation_problems(field: str, text: str, read_text: Callable[[str], str | None]) -> list[str]: + """One problem for each citation of `text` that the staged file does not hold.""" + problems: list[str] = [] + for match in _LINE_CITATION.finditer(text): + path = match.group("path") + problem = _line_citation_problem( + field, path, match.group("line"), match.group("text"), read_text(path) + ) + if problem is not None: + problems.append(problem) + for match in _SYMBOL_CITATION.finditer(text): + path = match.group("path") + problem = _symbol_citation_problem( + field, path, match.group("kind"), match.group("symbol"), read_text(path) + ) + if problem is not None: + problems.append(problem) + return problems + + +def _plan_citation_problems( + proposal: StageProposal, read_text: Callable[[str], str | None] | None +) -> list[str]: + """One problem for each citation of the evidence fields that the staged file does not hold, a + citation of the same field and path once. A check with no reader judges no citation: the + replay of the golden set holds no file text.""" + if read_text is None: + return [] + return list( + dict.fromkeys( + problem + for field, text in _evidence_texts(proposal) + for problem in _citation_problems(field, text, read_text) + ) + ) + + def _group_task_texts(group: StageGroup) -> list[str]: """The texts of `group` that a task holds: the title and each part of the work.""" return [text for field in _GROUP_TASK_FIELDS if (text := getattr(group, field)) is not None] @@ -752,6 +859,7 @@ class ProposalGate(StrEnum): SPEC_DELTA = "spec-delta" CONTENT = "content" + CITATION = "citation" FORM = "form" EVAL_REPLAY = "eval-replay" PATHS = "paths" @@ -775,6 +883,7 @@ def check_gates( overlaps: Sequence[Overlap] = (), search_evidence: Mapping[str, Sequence[str]] | None = None, plan_creates: Mapping[str, int] | None = None, + read_text: Callable[[str], str | None] | 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 @@ -782,6 +891,7 @@ def check_gates( return { ProposalGate.SPEC_DELTA: _delta_problems(proposal, known, requirements or {}), ProposalGate.CONTENT: content_problems(proposal, probes), + ProposalGate.CITATION: _plan_citation_problems(proposal, read_text), ProposalGate.FORM: form_problems(proposal.groups), ProposalGate.EVAL_REPLAY: eval_replay_problems(proposal.groups), ProposalGate.PATHS: path_problems(proposal.groups, tree), @@ -808,6 +918,7 @@ def check_proposal( overlaps: Sequence[Overlap] = (), search_evidence: Mapping[str, Sequence[str]] | None = None, plan_creates: Mapping[str, int] | None = None, + read_text: Callable[[str], str | None] | 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 @@ -820,8 +931,11 @@ def check_proposal( change a path the item cites: the plan names each in `dependencies` (#3252). `search_evidence` maps each negative-search literal to the paths that the staged tree holds: the job records it, the replay reads it from its case, and None records no search. `plan_creates` - maps each path that the plan of an open item creates to that item's number (#3418). The - problems are those of `check_gates`, joined in the order of the gates.""" + maps each path that the plan of an open item creates to that item's number (#3418). `read_text` + returns the text of a cited file at the staged commit, or None when the commit holds no such + file: the stage job passes the reader of its checkout and commit, and a check with none judges + no citation (#3422). The problems are those of `check_gates`, joined in the order of the + gates.""" found = check_gates( proposal, known, @@ -832,6 +946,7 @@ def check_proposal( overlaps=overlaps, search_evidence=search_evidence, plan_creates=plan_creates, + read_text=read_text, ) return [problem for problems in found.values() for problem in problems] diff --git a/tests/roadmap/test_roadmap_stage_content.py b/tests/roadmap/test_roadmap_stage_content.py index bbe9a71b7..70a683d99 100644 --- a/tests/roadmap/test_roadmap_stage_content.py +++ b/tests/roadmap/test_roadmap_stage_content.py @@ -258,6 +258,110 @@ def test_a_what_changes_folder_that_a_group_writes_under_is_accepted() -> None: assert not any("src/devops_cli/sample/" in p for p in problems) +# ── A citation of the staged file (#3422) ───────────────────────────────────── + +CITED = "src/sample.py" +OTHER_CITED = "src/other.py" +CITED_FILES = { + CITED: "\n".join( + [ + "import os", + "", + "", + "def f(): pass", + "", + "", + "def f():", + " return 1", + "", + "class G:", + " pass", + ] + ), + OTHER_CITED: "\n".join(["import os", "", "", "def g(): pass", "", "", "def g():"]), +} + + +def _cited(text: str, **overrides: Any) -> list[str]: + plan = make_stage_proposal(feasibility=[text], **overrides) + return check_proposal(plan, ["comment-thread"], TREE, read_text=CITED_FILES.get) + + +def test_the_sample_plan_cites_no_line_that_the_staged_file_lacks() -> None: + assert ( + check_proposal(make_stage_proposal(), ["comment-thread"], TREE, read_text=CITED_FILES.get) + == [] + ) + + +def test_a_citation_of_a_line_that_holds_other_text_is_refused() -> None: + problems = _cited(f"The probe shows `{CITED}:4:def g():` at the commit.") + + assert any("feasibility" in p and CITED in p for p in problems) + + +def test_a_citation_of_the_line_that_holds_the_text_is_accepted() -> None: + problems = _cited(f"The probe shows `{CITED}:4:def f():` at the commit.") + + assert not any(CITED in p for p in problems) + + +def test_a_citation_of_a_line_past_the_end_is_refused() -> None: + problems = _cited(f"The probe shows `{CITED}:99:def f():` at the commit.") + + assert any("feasibility" in p and CITED in p for p in problems) + + +def test_a_citation_of_an_absent_path_is_refused() -> None: + problems = _cited("The probe shows `src/absent.py:1:def f():` at the commit.") + + assert any("feasibility" in p and "src/absent.py" in p for p in problems) + + +def test_a_citation_of_a_text_of_fewer_than_three_characters_is_refused() -> None: + problems = _cited(f"The probe shows `{CITED}:4:de` at the commit.") + + assert any("feasibility" in p and CITED in p for p in problems) + + +def test_a_citation_of_a_symbol_is_accepted_when_the_file_defines_it() -> None: + problems = _cited(f"The probe shows `{CITED}:def f` at line 7.") + + assert not any(CITED in p for p in problems) + + +def test_a_citation_of_a_class_is_accepted_when_the_file_defines_it() -> None: + problems = _cited(f"The probe shows `{CITED}:class G` at line 10.") + + assert not any(CITED in p for p in problems) + + +def test_a_citation_of_an_absent_symbol_is_refused() -> None: + problems = _cited(f"The probe shows `{OTHER_CITED}:def f` at the commit.") + + assert any("feasibility" in p and OTHER_CITED in p for p in problems) + + +def test_a_stale_citation_of_what_changes_names_that_field() -> None: + plan = make_stage_proposal(what_changes=[f"The probe reads `{CITED}:4:def g():` now."]) + + problems = check_proposal(plan, ["comment-thread"], TREE, read_text=CITED_FILES.get) + + assert any("what_changes" in p and CITED in p for p in problems) + + +def test_a_plan_with_no_reader_gets_no_citation_problem() -> None: + plan = make_stage_proposal(feasibility=[f"The probe shows `{CITED}:4:def g():` at the commit."]) + + assert not any(CITED in p for p in _check(plan)) + + +def test_a_citation_holds_when_the_white_space_differs() -> None: + problems = _cited(f"The probe shows `{CITED}:4:def f():` at the commit.") + + assert not any(CITED in p for p in problems) + + def test_a_rule_of_the_plan_check_needs_its_self_acceptance_test() -> None: plan = _plan_that_adds_a_rule( "Write `test_the_rule_refuses_the_plan` in `tests/roadmap/test_roadmap_stage_content.py`" diff --git a/tests/roadmap/test_roadmap_stage_gates.py b/tests/roadmap/test_roadmap_stage_gates.py index 465a0dee2..8477a4d0d 100644 --- a/tests/roadmap/test_roadmap_stage_gates.py +++ b/tests/roadmap/test_roadmap_stage_gates.py @@ -75,3 +75,11 @@ def test_check_proposal_joins_the_problems_of_the_gates_in_the_order_of_the_gate problem for problems in found.values() for problem in problems ] assert list(found) == list(ProposalGate) + + +def test_a_stale_citation_is_a_problem_of_the_gate_citation() -> None: + plan = make_stage_proposal(feasibility=["The probe shows `src/sample.py:4:def g():` here."]) + + found = check_gates(plan, KNOWN, TREE, read_text={"src/sample.py": "x\n\n\ndef f():\n"}.get) + + assert [gate for gate, problems in found.items() if problems] == [ProposalGate.CITATION] diff --git a/tests/roadmap/test_roadmap_stage_plan.py b/tests/roadmap/test_roadmap_stage_plan.py index 6f3b70f0b..e0190667a 100644 --- a/tests/roadmap/test_roadmap_stage_plan.py +++ b/tests/roadmap/test_roadmap_stage_plan.py @@ -35,7 +35,14 @@ from devops_cli.github.rate_limiter import get_github_rate_limiter from devops_cli.openspec.tool import isolated_openspec from devops_cli.roadmap.memory_store import InMemoryRoadmapStore -from devops_cli.roadmap.stage import StagedChange, StagePlan, parse_filters, plan_item, plan_stage +from devops_cli.roadmap.stage import ( + StagedChange, + StagePlan, + _read_staged_file, + parse_filters, + plan_item, + plan_stage, +) from devops_cli.roadmap.stage_change import ( CREATED_LINE_TOKENS, StageGroup, @@ -1747,6 +1754,17 @@ def test_plan_item_plans_with_a_toolbox_client( assert type(planned) is StagedChange +def test_the_citation_reader_returns_the_text_at_the_staged_commit(stage_repo: StageRepo) -> None: + stage_repo.track({"src/devops_cli/cited.py": "def staged(): pass\n"}) + sha = stage_repo.git("rev-parse", "HEAD") + (stage_repo.clone / "src/devops_cli/cited.py").write_text("def worktree(): pass\n") + + assert _read_staged_file(stage_repo.clone, sha, "src/devops_cli/cited.py") == ( + "def staged(): pass\n" + ) + assert _read_staged_file(stage_repo.clone, sha, "src/devops_cli/absent.py") is None + + def test_a_toolbox_plan_of_a_checkout_with_a_change_fails_before_any_model_call( store: InMemoryRoadmapStore, item: int, stage_repo: StageRepo, openspec: OpenSpecFake ) -> None: diff --git a/tests/roadmap/test_roadmap_stage_prompt_rules.py b/tests/roadmap/test_roadmap_stage_prompt_rules.py index 23ec81b8c..367cc1ef4 100644 --- a/tests/roadmap/test_roadmap_stage_prompt_rules.py +++ b/tests/roadmap/test_roadmap_stage_prompt_rules.py @@ -406,6 +406,17 @@ def test_the_prompt_asks_for_the_hit_count_of_a_negative_search() -> None: assert "can never pass after it" in _PROMPT +# ── A citation of a line or a symbol (#3422) ───────────────────────────────── + + +def test_the_prompt_states_the_citation_forms_and_prefers_a_symbol() -> None: + assert "`path:line:text`" in _PROMPT + assert "`path:def name`" in _PROMPT + assert "`path:class name`" in _PROMPT + assert "Cite a symbol instead of a line number when a line is not needed" in _PROMPT + assert "refuses one whose line does not hold the text" in _PROMPT + + # ── The phrases that a document and the prompt keep together ────────────────── _SHARED = [