From 4410e43358a9e5ca6519ffc7ad16de3d45182d5d Mon Sep 17 00:00:00 2001 From: "devops-cli-service[bot]" <349441588+devops-cli-service[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:23:29 +0000 Subject: [PATCH 1/2] fix(roadmap): Resolve a plan citation against the staged tree (#3422) --- .../tasks.md | 4 +- src/devops_cli/roadmap/stage_change.py | 130 +++++++++++++++++- tests/roadmap/test_roadmap_stage_content.py | 104 ++++++++++++++ 3 files changed, 231 insertions(+), 7 deletions(-) diff --git a/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md b/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md index 67dc0ae3f..61e5b6b65 100644 --- a/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md +++ b/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md @@ -4,9 +4,9 @@ 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 diff --git a/src/devops_cli/roadmap/stage_change.py b/src/devops_cli/roadmap/stage_change.py index 4979e7b2e..dfd56ceb4 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 @@ -118,6 +119,30 @@ # 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 +_CITATION_STALE: Final[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." +) +_CITATION_SHORT: Final[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`." +) _SENTENCE_END: Final[str] = f"{string.whitespace}." REGENERATE_DOCUMENTS: Final[str] = "uv run devops docs generate --sync-readme" @@ -574,6 +599,96 @@ 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 _CITATION_STALE.format(field=field, path=path) + cited = _cited_text(text) + if len(cited) < _CITATION_MIN_CHARS: + return _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 _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 _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 content_problems(proposal: StageProposal, probes: Sequence[str] = ()) -> list[str]: """What is wrong with the text of `proposal` that no schema says (#3200): a text that is too short or a placeholder word (`is_placeholder` of the pull request form), a `feasibility` that @@ -607,6 +722,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 @@ -619,9 +735,13 @@ 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).""" + 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).""" problems = _delta_problems(proposal, known, requirements or {}) problems.extend(content_problems(proposal, probes)) + problems.extend(_plan_citation_problems(proposal, read_text)) problems.extend(form_problems(proposal.groups)) problems.extend(path_problems(proposal.groups, tree)) problems.extend(_what_changes_problems(proposal, tree)) diff --git a/tests/roadmap/test_roadmap_stage_content.py b/tests/roadmap/test_roadmap_stage_content.py index 227413f1a..5552d38ab 100644 --- a/tests/roadmap/test_roadmap_stage_content.py +++ b/tests/roadmap/test_roadmap_stage_content.py @@ -235,3 +235,107 @@ def test_a_what_changes_folder_that_a_group_writes_under_is_accepted() -> None: problems = _check(plan) 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) From 447ca8a92239d253371982e3215faf36acea9572 Mon Sep 17 00:00:00 2001 From: "devops-cli-service[bot]" <349441588+devops-cli-service[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:34:39 +0000 Subject: [PATCH 2/2] fix(roadmap): Pass a reader of the staged commit to check_proposal (#3422) --- .../3422-the-plan-check-fails-a-citation/tasks.md | 4 ++-- src/devops_cli/roadmap/stage.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md b/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md index 61e5b6b65..a3636cec9 100644 --- a/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md +++ b/openspec/changes/3422-the-plan-check-fails-a-citation/tasks.md @@ -13,9 +13,9 @@ Reads: `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage_p 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 diff --git a/src/devops_cli/roadmap/stage.py b/src/devops_cli/roadmap/stage.py index 508115751..093d15b4b 100644 --- a/src/devops_cli/roadmap/stage.py +++ b/src/devops_cli/roadmap/stage.py @@ -136,6 +136,7 @@ NextStep, body_dependencies, collect_context, + git_result, inspect_checkout, recorded_blockers, ) @@ -1019,6 +1020,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) @@ -1450,6 +1454,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