diff --git a/README.md b/README.md index ac7e8207a..0a03d4b86 100644 --- a/README.md +++ b/README.md @@ -381,7 +381,7 @@ and excessive permissions fail it, and every other finding is shown. | | [**roadmap**](docs/commands/roadmap.md) | `devops roadmap intake [OPTIONS]` | Turn each open issue with no priority label into a roadmap item, or file a new candidate: intake sets the labels, adds a reason comment and names a possible duplicate for a person to close. It writes nothing without --confirm. Details: docs/roadmap/intake.md. | | | `devops roadmap refine [OPTIONS]` | Refine roadmap items to Ready: investigate open questions with tools that read the code and the documentation, then write the design, tasks and acceptance criteria to GitHub. Without --confirm, refine prints its plan only. Details: docs/roadmap/refine.md. | | | `devops roadmap alternatives [OPTIONS]` | Check open items for an open, maintained tool or library that already covers their need. A check posts one comment and a body block, and a person decides what the tool means for the item. Details: docs/roadmap/alternatives.md. | -| | `devops roadmap retitle [OPTIONS]` | Shorten the title summary of open items that is over 60 characters. The job keeps the type and the scope. A Planned item takes the summary of its plan pull request title and needs no model call; another item gets one model call. The job leaves a title that a person changed after the last job comment. Without --confirm, it prints the new titles and writes nothing. | +| | `devops roadmap retitle [OPTIONS]` | Shorten the title summary of open items that is over 60 characters. The job keeps the type and the scope. A Planned item takes the summary of its plan pull request title and needs no model call; another item gets one model call. The job leaves a title that a person changed after the last job comment. An item that the job leaves gets the mark retitle.skipped, and the job does not select it again until its title, body or plan pull request changes. Without --confirm, it prints the new titles and writes nothing. | | | `devops roadmap stage [OPTIONS]` | Make Ready items Staged: for each item, push a branch with its OpenSpec change in one commit and open a draft plan pull request. The Service marks the draft ready when its checks pass. Details: docs/roadmap/stage.md. | | | `devops roadmap run [OPTIONS]` | Run the roadmap jobs that are due, in order: metrics, then intake, refine and the alternatives check, and record each one's last success. Intake is due on an issue reopened, and decides at most 5 candidates a run and keeps a record of those it left beside the schedule; refine is due every 31 minutes, on a first run, and after an intake that placed a critical fix or a P0 item; retitle is due every 61 minutes and on a first run; the alternatives check is due every 719 minutes and on a first run, and checks at most roadmap.alternatives.first_pass_limit items while an item has no check, and roadmap.alternatives.limit items after. Without --confirm, or with --dry-run, prints the due list and runs nothing. | | | `devops roadmap implement [OPTIONS]` | Implement a Staged or Planned item in bounded steps: with --group, run one task group in a checkout; without it, run the job, which takes items, runs their steps, and marks the pull request ready. Details: docs/roadmap/implement.md. | diff --git a/docs/CLI_REFERENCE.md b/docs/CLI_REFERENCE.md index 03ac5afcc..9eed4c5dd 100644 --- a/docs/CLI_REFERENCE.md +++ b/docs/CLI_REFERENCE.md @@ -6721,7 +6721,7 @@ devops roadmap alternatives [OPTIONS] ### `devops roadmap retitle` -**Shorten the title summary of open items that is over 60 characters. The job keeps the type and the scope. A Planned item takes the summary of its plan pull request title and needs no model call; another item gets one model call. The job leaves a title that a person changed after the last job comment. Without --confirm, it prints the new titles and writes nothing.** +**Shorten the title summary of open items that is over 60 characters. The job keeps the type and the scope. A Planned item takes the summary of its plan pull request title and needs no model call; another item gets one model call. The job leaves a title that a person changed after the last job comment. An item that the job leaves gets the mark retitle.skipped, and the job does not select it again until its title, body or plan pull request changes. Without --confirm, it prints the new titles and writes nothing.** ```bash devops roadmap retitle [OPTIONS] diff --git a/docs/ROADMAP_WORKFLOW.md b/docs/ROADMAP_WORKFLOW.md index adf519c5e..e85af5c02 100644 --- a/docs/ROADMAP_WORKFLOW.md +++ b/docs/ROADMAP_WORKFLOW.md @@ -117,6 +117,7 @@ These are all the keys of the job record, by the job that writes them: | Refine | `refine.tries`, `refine.failed_check`, `refine.failure` | The count of rounds whose job failed on the same body, the names of the failed checks, and the last error. `refine.next_step` is then `retry`, and `failed` at 3 tries (`CONST_ROADMAP_REFINE_MAX_TRIES`). Only a decision marked `(a person decides)` makes refine wait on a person | | Refine | `refine.comments_seen`, `refine.commented_at`, `refine.wait_refs` | What the last read of the comments of a waiting item gave: the count of comments of the issue listing, the time of refine's last comment, and the issues and paths that the wait text names. A round reads the comments again only when the count moves | | Alternatives | `alternatives.checked_at`, `alternatives.body_hash`, `alternatives.verdict`, `alternatives.tool`, `alternatives.failed_at`, `alternatives.failed_error` | When the check ran, the text it read, its verdict and the tool it found; when the last check failed and the class of its error, until a check is written | +| Retitle | `retitle.skipped` | A hash of the title, the body and the `stage.pr` mark that the retitle job read when it left an Item with a long title (a person's edit, no plan title, or no usable reply of the model). The job writes it once, and does not select the Item again while the hash holds. A change to the title, the body or `stage.pr` selects the Item again; a comment does not | | Stage | `stage.branch`, `stage.pr`, `stage.sha` | The staged branch, its draft pull request and its last commit | | Stage | `stage.groups`, `stage.needs_split`, `stage.file_cap` | The task groups with their size, the group over the budget, and the cap of that measure | | Stage | `stage.ready` | The plan pull request that the Service has seen leave the draft state | diff --git a/docs/commands/roadmap.md b/docs/commands/roadmap.md index 1086e5bf5..68bf861ce 100644 --- a/docs/commands/roadmap.md +++ b/docs/commands/roadmap.md @@ -74,7 +74,7 @@ devops roadmap alternatives [OPTIONS] ## `devops roadmap retitle` -**Shorten the title summary of open items that is over 60 characters. The job keeps the type and the scope. A Planned item takes the summary of its plan pull request title and needs no model call; another item gets one model call. The job leaves a title that a person changed after the last job comment. Without --confirm, it prints the new titles and writes nothing.** +**Shorten the title summary of open items that is over 60 characters. The job keeps the type and the scope. A Planned item takes the summary of its plan pull request title and needs no model call; another item gets one model call. The job leaves a title that a person changed after the last job comment. An item that the job leaves gets the mark retitle.skipped, and the job does not select it again until its title, body or plan pull request changes. Without --confirm, it prints the new titles and writes nothing.** ```bash devops roadmap retitle [OPTIONS] diff --git a/docs/roadmap/intake.md b/docs/roadmap/intake.md index 36714b15f..a05cbb3da 100644 --- a/docs/roadmap/intake.md +++ b/docs/roadmap/intake.md @@ -56,6 +56,7 @@ devops gh issues close --reason not-planned - A Planned item takes the summary of the title of its plan pull request (`plan # `), which its job record names. The job makes no model call for it. When that title gives no valid summary, the job leaves the item and comments the reason once. - Another item gets one request to the `triage` model, and one more with the faults of the first reply. A reply that fails twice leaves the item. - It leaves a title that someone changed after the last comment of a job. It reads the title changes on the timeline of the item. +- When it leaves an item, it writes the mark `retitle.skipped` once. The value is a hash of the title, the body and the `stage.pr` mark. The selection leaves the item out while the hash holds, so a later round reads nothing of it and makes no model request for it. A change to the title, the body or `stage.pr` selects it again. A comment does not. - Without `--confirm` it prints the new titles and writes nothing. With `--confirm` it writes each title and one comment with the marker ``. `--dry-run` makes no request. - The Service runs the job as the row `retitle` of the model lane. The row is due on a first run and every 61 minutes. It runs after `refine` and before `stage`, so the stage job reads the new titles in the same round. Each run takes at most 25 items and writes as `--confirm` does. The row needs no clone. diff --git a/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/.openspec.yaml b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/.openspec.yaml new file mode 100644 index 000000000..e582ee4a2 --- /dev/null +++ b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-07 diff --git a/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/design.md b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/design.md new file mode 100644 index 000000000..2a474f2f4 --- /dev/null +++ b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/design.md @@ -0,0 +1,18 @@ +# Design + +## Decisions + +- **Hash the whole body of the item, as `Item.body` holds it.**: The issue says "the body". `Item.body` has the record block taken out, so the write of the mark does not change the hash. A rewrite of the refine section or of the alternatives block returns the item to the selection once. That costs one decision and keeps one rule that a reader can state. +- **Record a model failure as a skip.**: The model gave no title that passes the form in two tries. A third try on the same text costs 2 model calls and gives the same result in most cases. The item returns when the title, the body or `stage.pr` changes. +- **Record the mark for a person's edit too.**: The issue asks for it. The decision reads the comments and the title changes, and neither changes while the title and body stay the same. +- **Write the mark in `apply_retitle`, once for each `TitleSkip`, with `store.set_marks`.**: `apply_retitle` is the one write point of the job, and `plan_retitle` writes nothing. A `ModelGatewayUnreachableError` raises in the plan, before the apply step, so a gateway fault records no mark and the item stays in the selection. +- **Make `long_title_items` do the check.**: The selection is where the item holds a place of the limit. A check inside `_decide` would still count the item against the limit. +- **Use `StageRecordKey.PR` for the plan input.**: The plan pull request is the only source of a title for a Planned item, so its number is the one mark that the decision reads. +- **Add the key family before the failure-class issues of #3027.**: Those issues extend the job record, and this change gives them the union that they extend. + +## Rejected alternatives + +- **Hash `text_of_a_person(item.body)`.**: That text leaves out the refine section and the alternatives block, so a round of those jobs would not return the item. The model reads the first characters of the whole body (`_request`), so a change in a section can change the reply. The issue names "the body". +- **Keep the skips in memory for the run.**: The job runs every 61 minutes as a new process, so a memory set is empty at the next round. The mark must outlive the run. +- **A timestamp or a retry count in the mark.**: The issue asks for a hash of the inputs. A timestamp returns the item with no change of its inputs and costs the reads again. +- **Skip the mark for a plan pull request with no usable title.**: The reason changes only when the plan pull request or its title changes. The item already carries a comment for it. A mark ends the repeated reads. diff --git a/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/proposal.md b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/proposal.md new file mode 100644 index 000000000..5d86d6f96 --- /dev/null +++ b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/proposal.md @@ -0,0 +1,53 @@ +# Proposal + +## Why + +`devops roadmap retitle` records no mark for an item that it leaves (#4038). `long_title_items` selects every open item with a long title summary, the lowest number first, and `plan_retitle` takes the first 25. An item that the job cannot shorten is taken again in every round, which runs every 61 minutes. Each round reads the comments and the title changes of the item, and for an item that is not Planned it makes up to 2 model calls. These items hold the places of items with higher numbers. #3630 found 34 Ready items that the title rule left out, so this row blocks the stage job. + +The job leaves an item for one of three reasons: a person changed the title, the plan pull request has no usable title, or the model gave no summary that passes the title form in two tries. + +## What Changes + +- Add the record key family `RetitleRecordKey` with the key `retitle.skipped` to `src/devops_cli/roadmap/stores/store.py`. Add it to the `JobRecord` union, to the parsed families `_JOB_RECORD_KEYS` and to `__all__`. +- Add `skip_hash(item)` to `src/devops_cli/roadmap/retitle.py`. It is the SHA-256 hash (`hash_text` of `body_hash.py`) of the title, the body and the `stage.pr` mark of the item. +- Make `long_title_items` leave out an item whose `retitle.skipped` equals `skip_hash(item)`. The item returns to the selection when its title, its body or its plan pull request changes. +- Make `apply_retitle` write the mark once for each `TitleSkip`, with one `set_marks` call. This covers all three reasons, a person's edit included. A model call that cannot reach the gateway raises before the apply step, so it records no mark. +- Add the write of the mark to `retitle_requests` in `src/devops_cli/roadmap/request_plan.py`, with its condition text in `src/devops_cli/lang/`, so that the dry run lists it. +- Add the key family to the table of record keys in `docs/ROADMAP_WORKFLOW.md`. +- The mark is a hash only. It has no link to the failure class table. +- The change alters the output of `devops roadmap retitle --dry-run` and the selection of the job, which a consumer observes, so it carries a spec delta (`roadmap-retitle`) and `skip_specs` is false. +- The change fits one pull request. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `roadmap-retitle`: Skip mark, Skipped item returns when its inputs change, Cost of a skipped item + +## Impact + +- Code: `src/devops_cli/roadmap/stores/store.py`, `src/devops_cli/roadmap/retitle.py`, `src/devops_cli/roadmap/request_plan.py`, `src/devops_cli/lang/en/messages.py` +- Documents: `docs/ROADMAP_WORKFLOW.md` +- Tests: `tests/roadmap/test_roadmap_retitle_skip.py` (new) + +## Feasibility + +- Read at origin/main, in the worktree: `retitle.py` has `long_title_items`, which takes every open item with a long summary, and `plan_retitle`, which takes the first `DEFAULT_ROADMAP_RETITLE_LIMIT` (25) of them. `_decide` reads `comments_on` and `last_renamed_at` for each of them, and `_model_title` makes up to `_ATTEMPTS` (2) model calls. A `TitleSkip` is only reported, and commented when `comment=True`. Nothing records it, so the premise of the issue holds. +- `stores/store.py` lists `ItemField | RefineRecordKey | AlternativesRecordKey | StageRecordKey | ImplementRecordKey` in `JobRecord`, and the same families in `_JOB_RECORD_KEYS`. No `RetitleRecordKey` exists. +- `RoadmapStore.set_marks(item, marks)` writes marks in one call and changes no field. `open_items()` returns each item with its `job_record` and its `body` with the record block taken out, so the check needs no extra read and the write of the mark does not change the body. +- `body_hash.hash_text` gives the SHA-256 hash, so no new library is needed. +- No merged or open pull request and no newer issue or ADR supersedes the item: #3535, #3630 and #3811 added the row and the schedule only, and no open pull request touches `retitle.py`. +- Check of the new tests against the real code: `uv run devops ci test tests/roadmap/test_roadmap_retitle_skip.py` fails on the unchanged selection (15 of 17 tests fail on the call; the 2 that pass are controls that hold with no mark). `uv run devops ci fail-to-pass --base origin/main` is the proof for the pull request. +- Risks settled in `design.md`: the hash covers the whole body, a model failure is recorded as a skip, and the key family lands before the failure-class issues of #3027. + +## Existing Tool Considered + +The record marks of the item job record and `hash_text` of `body_hash.py` cover the need: the job record is the open, in-repository store for per-item state, and refine, alternatives and stage use it the same way. No library is needed. The change builds one small function (`skip_hash`) and one key family. Cost: about 35k tokens and 2 h to build, about 2k tokens per quarter to keep, and about 8k tokens and 0.5 h to fix. The change deletes no internal code. + +## Dependencies + +None. diff --git a/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/specs/roadmap-retitle/spec.md b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/specs/roadmap-retitle/spec.md new file mode 100644 index 000000000..7efdf8186 --- /dev/null +++ b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/specs/roadmap-retitle/spec.md @@ -0,0 +1,41 @@ +## ADDED Requirements + +### Requirement: Skip mark +When the repair pass leaves an item with a long title, it SHALL write the mark `retitle.skipped` in the job record of the item, once, with one write. The value SHALL be a hash of the title, the body and the `stage.pr` mark that the decision read. This covers an item whose title a person changed, a Planned item without a usable plan title, and an item for which the model gave no summary that passes `title_problems` in two tries. The pass SHALL write no mark for an item that it retitles. + +#### Scenario: The model gives no usable summary +- **WHEN** both replies of the model fail `title_problems` and the pass runs with `--confirm` +- **THEN** the job record of the item holds `retitle.skipped`, and the pass wrote that mark once + +#### Scenario: A person changed the title +- **WHEN** a person renamed an item after the last job comment and the pass runs with `--confirm` +- **THEN** the job record of the item holds `retitle.skipped` + +#### Scenario: The title is changed +- **WHEN** the pass retitles an item +- **THEN** the pass writes no `retitle.skipped` for it + +### Requirement: A skipped item is not selected again +The selection of the repair pass SHALL leave out an open item whose `retitle.skipped` equals the hash of its current title, body and `stage.pr` mark. It SHALL select the item again when its title, its body or its `stage.pr` mark changes. A comment on the item SHALL NOT select it again. A skipped item SHALL NOT hold a place of `--limit`. + +#### Scenario: Next round after a skip +- **WHEN** the pass skipped an item and runs again with no change to the item +- **THEN** the selection does not hold the item, and a later item with a long title takes the place of the limit + +#### Scenario: The item changes +- **WHEN** the title, the body or the `stage.pr` mark of a skipped item changes +- **THEN** the selection holds the item again + +### Requirement: A skipped item costs one write once +A skipped item SHALL cost one write in the round that skips it. In a later round with no change to the item, it SHALL cost no read of its comments, no read of its title changes, no read of a pull request title and no model request. + +#### Scenario: Two rounds +- **WHEN** the pass runs twice with `--confirm` on one item that it cannot shorten +- **THEN** the first round makes one write of the mark and the second round makes no write, no read of the item and no model request + +### Requirement: Dry run lists the write of the mark +The dry run of the repair pass SHALL list the write of the mark `retitle.skipped` with the condition under which it runs. + +#### Scenario: Preview of the writes +- **WHEN** the repair pass runs with `--dry-run` +- **THEN** the planned writes hold one write of the body of an item with a condition for a skipped item diff --git a/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/tasks.md b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/tasks.md new file mode 100644 index 000000000..ba8663d74 --- /dev/null +++ b/openspec/changes/archive/2026-11-07-4038-retitle-records-a-skip-mark/tasks.md @@ -0,0 +1,32 @@ +# Tasks + +## 1. Record and honor the skip mark + +Reads: `src/devops_cli/roadmap/retitle.py`, `src/devops_cli/roadmap/stores/store.py`, `src/devops_cli/roadmap/body_hash.py`, `tests/roadmap/test_roadmap_retitle.py`, `tests/support/fakes/roadmap_store.py`. Changes: `src/devops_cli/roadmap/stores/store.py`, `src/devops_cli/roadmap/retitle.py`, `tests/roadmap/test_roadmap_retitle_skip.py` (new). + +- [x] 1.1 Add `tests/roadmap/test_roadmap_retitle_skip.py` for the mark, the return of an item whose title, body or `stage.pr` changes, and the counts of writes, reads and model requests in the next round. Run it and see it fail + - Verification: `$ uv run devops ci fail-to-pass --base origin/main` shows the new tests fail on the base +- [x] 1.2 In `store.py`, add `RetitleRecordKey` with `SKIPPED = "retitle.skipped"`, add it to the `JobRecord` union, to `_JOB_RECORD_KEYS` and to `__all__` + - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_retitle_skip.py` +- [x] 1.3 In `retitle.py`, add `skip_hash(item)`, the `hash_text` of the title, the body and the `stage.pr` mark. Make `long_title_items` leave out an item whose `retitle.skipped` equals it. Make `apply_retitle` write `{RetitleRecordKey.SKIPPED: skip_hash(item)}` with one `store.set_marks` call for each `TitleSkip`. Update the module document of `retitle.py` for the mark + - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_retitle_skip.py tests/roadmap/test_roadmap_retitle.py` + +## 2. List the write of the mark in the dry run + +Reads: `src/devops_cli/roadmap/request_plan.py`, `src/devops_cli/lang/en/messages.py`. Changes: `src/devops_cli/roadmap/request_plan.py`, `src/devops_cli/lang/en/messages.py`. + +- [x] 2.1 In `retitle_requests`, add the `PATCH` of the body with the record (`StoreRequests.set_marks`) under a new condition text in `plan_conditions`, and update its docstring. The test `test_request_count_the_dry_run_plan_lists_the_write_of_the_mark` of group 1 is the failing test + - Verification: `$ uv run devops ci test tests/roadmap/test_roadmap_retitle_skip.py tests/roadmap/test_roadmap_dry_runs.py` + +## 3. Documents + +Reads: `docs/ROADMAP_WORKFLOW.md`. Changes: `docs/ROADMAP_WORKFLOW.md`. + +- [x] 3.1 Add a Retitle row for `retitle.skipped` to the table of record keys in `docs/ROADMAP_WORKFLOW.md`: the hash of the title, the body and the `stage.pr` mark, and the rule that the job selects the item again when the hash changes + - Verification: `$ uv run devops ci docs` +- [x] 3.2 Run `uv run devops docs generate --sync-readme` and commit the generated documents if the message change alters them + - Verification: `$ uv run devops ci docs` + +## Workflow follow-up + +- Pending a person: review the diff in tuicr (tuicr -r ..HEAD). diff --git a/openspec/specs/roadmap-retitle/spec.md b/openspec/specs/roadmap-retitle/spec.md index 2d10ddf65..1f549017f 100644 --- a/openspec/specs/roadmap-retitle/spec.md +++ b/openspec/specs/roadmap-retitle/spec.md @@ -117,3 +117,43 @@ The `retitle` row SHALL call `plan_retitle` with the model of the retitle job an #### Scenario: Default stub table - **WHEN** `build_stub_table()` runs with no `retitle` runner. - **THEN** the table has no `retitle` row. + +### Requirement: Skip mark +When the repair pass leaves an item with a long title, it SHALL write the mark `retitle.skipped` in the job record of the item, once, with one write. The value SHALL be a hash of the title, the body and the `stage.pr` mark that the decision read. This covers an item whose title a person changed, a Planned item without a usable plan title, and an item for which the model gave no summary that passes `title_problems` in two tries. The pass SHALL write no mark for an item that it retitles. + +#### Scenario: The model gives no usable summary +- **WHEN** both replies of the model fail `title_problems` and the pass runs with `--confirm` +- **THEN** the job record of the item holds `retitle.skipped`, and the pass wrote that mark once + +#### Scenario: A person changed the title +- **WHEN** a person renamed an item after the last job comment and the pass runs with `--confirm` +- **THEN** the job record of the item holds `retitle.skipped` + +#### Scenario: The title is changed +- **WHEN** the pass retitles an item +- **THEN** the pass writes no `retitle.skipped` for it + +### Requirement: A skipped item is not selected again +The selection of the repair pass SHALL leave out an open item whose `retitle.skipped` equals the hash of its current title, body and `stage.pr` mark. It SHALL select the item again when its title, its body or its `stage.pr` mark changes. A comment on the item SHALL NOT select it again. A skipped item SHALL NOT hold a place of `--limit`. + +#### Scenario: Next round after a skip +- **WHEN** the pass skipped an item and runs again with no change to the item +- **THEN** the selection does not hold the item, and a later item with a long title takes the place of the limit + +#### Scenario: The item changes +- **WHEN** the title, the body or the `stage.pr` mark of a skipped item changes +- **THEN** the selection holds the item again + +### Requirement: A skipped item costs one write once +A skipped item SHALL cost one write in the round that skips it. In a later round with no change to the item, it SHALL cost no read of its comments, no read of its title changes, no read of a pull request title and no model request. + +#### Scenario: Two rounds +- **WHEN** the pass runs twice with `--confirm` on one item that it cannot shorten +- **THEN** the first round makes one write of the mark and the second round makes no write, no read of the item and no model request + +### Requirement: Dry run lists the write of the mark +The dry run of the repair pass SHALL list the write of the mark `retitle.skipped` with the condition under which it runs. + +#### Scenario: Preview of the writes +- **WHEN** the repair pass runs with `--dry-run` +- **THEN** the planned writes hold one write of the body of an item with a condition for a skipped item diff --git a/src/devops_cli/lang/en/help.py b/src/devops_cli/lang/en/help.py index 99599cd00..23c31c84b 100644 --- a/src/devops_cli/lang/en/help.py +++ b/src/devops_cli/lang/en/help.py @@ -1384,7 +1384,9 @@ class RoadmapCommandHelp: "Shorten the title summary of open items that is over 60 characters. The job keeps the type " "and the scope. A Planned item takes the summary of its plan pull request title and needs no " "model call; another item gets one model call. The job leaves a title that a person changed " - "after the last job comment. Without --confirm, it prints the new titles and writes nothing." + "after the last job comment. An item that the job leaves gets the mark retitle.skipped, and " + "the job does not select it again until its title, body or plan pull request changes. " + "Without --confirm, it prints the new titles and writes nothing." ) retitle_item: str = ( "Specific issue number to examine instead of selecting. Retitle examines it when it is " diff --git a/src/devops_cli/lang/en/messages.py b/src/devops_cli/lang/en/messages.py index abf98fab8..44d961a56 100644 --- a/src/devops_cli/lang/en/messages.py +++ b/src/devops_cli/lang/en/messages.py @@ -4207,6 +4207,10 @@ class RoadmapMessages: "planned": "the item is Planned", "unplanned": "the item is not Planned", "retitled": "a person did not change the title since the last job comment", + "retitle_skipped": ( + "the run leaves the item: a person changed its title, its plan pull request has " + "no usable title, or the model gave no summary that passes the title form" + ), } ) plan_repeat: dict[str, str] = field( diff --git a/src/devops_cli/roadmap/request_plan.py b/src/devops_cli/roadmap/request_plan.py index 7362e3539..f46218218 100644 --- a/src/devops_cli/roadmap/request_plan.py +++ b/src/devops_cli/roadmap/request_plan.py @@ -538,8 +538,9 @@ def retitle_requests( """What `devops roadmap retitle` reads, in order, and the writes `--confirm` makes after them (#3486). The run reads every issue. For each item with a long title it reads the comments and the title changes of the item, then the title of its plan pull request when the item is - Planned, or runs the model when it is not. `--item` names the item, and `limit` caps the - items of a run.""" + Planned, or runs the model when it is not. An item that the run leaves gets one write of the + body with the skip mark (#4038), and the next run does not read it again. `--item` names the + item, and `limit` caps the items of a run.""" del limit store = StoreRequests(repo) texts = MESSAGES.roadmap @@ -559,9 +560,10 @@ def retitle_requests( *within(store.pull_title(subject, _p("pull")), cond["planned"], each_item), *within([model], cond["unplanned"], each_item), ] - writes = within( - [*store.set_title(subject), *store.comment(subject)], cond["retitled"], each_item - ) + writes = [ + *within([*store.set_title(subject), *store.comment(subject)], cond["retitled"], each_item), + *within(store.set_marks(subject), cond["retitle_skipped"], each_item), + ] return tuple(reads), tuple(writes) diff --git a/src/devops_cli/roadmap/retitle.py b/src/devops_cli/roadmap/retitle.py index df381a8e3..5e0e59637 100644 --- a/src/devops_cli/roadmap/retitle.py +++ b/src/devops_cli/roadmap/retitle.py @@ -8,6 +8,13 @@ **Selection.** An open Item whose title has a Conventional Commit type and a summary over the limit, the lowest number first, at most `--limit` a run. `--item` names the one item to examine. +**The skip mark (#4038).** When the job leaves an item (a person's edit, no plan title, no usable +reply of the model), `apply_retitle` writes `retitle.skipped` in the job record of the item, once. +The value is `skip_hash`: a hash of the title, the body and the `stage.pr` mark. The selection +leaves out an item while the mark equals the current hash, so a later round reads no comment of +it, makes no model request for it, and does not let it hold a place of `--limit`. A change to the +title, the body or the `stage.pr` mark selects the item again. A comment does not. + **The source of the new summary.** The job keeps the type and the scope, and writes only the summary (`build_title`), so `title_problems` holds for the new title. @@ -53,10 +60,17 @@ from devops_cli.exceptions.base import DevOpsCLIError from devops_cli.exceptions.validation import ValidationError from devops_cli.lang import MESSAGES +from devops_cli.roadmap.body_hash import hash_text from devops_cli.roadmap.commit_title import commit_summary, commit_type from devops_cli.roadmap.intake.intake_model import gateway_address from devops_cli.roadmap.pull_form import SUMMARY_MAX_CHARS, build_title, summary_length -from devops_cli.roadmap.stores.store import IssueComment, Item, RoadmapStore, StageRecordKey +from devops_cli.roadmap.stores.store import ( + IssueComment, + Item, + RetitleRecordKey, + RoadmapStore, + StageRecordKey, +) logger = logging.getLogger(__name__) @@ -175,15 +189,28 @@ def has_writes(self) -> bool: return bool(self.changes) or any(skip.comment for skip in self.skips) +def skip_hash(item: Item) -> str: + """The hash of what the decision for `item` reads: its title, its body and its `stage.pr` + mark. The body of an Item holds no record block, so the write of the mark leaves it as it is.""" + recorded = item.job_record.get(StageRecordKey.PR) or "" + return hash_text("\0".join((item.title, item.body, recorded))) + + +def _is_skipped(item: Item) -> bool: + return item.job_record.get(RetitleRecordKey.SKIPPED) == skip_hash(item) + + def long_title_items(store: RoadmapStore, item_number: int | None = None) -> list[Item]: """The open Items with a Conventional Commit type and a summary over the limit, the lowest - number first. With `item_number`, only that item, when it is one of them.""" + number first, without an item whose skip mark holds. With `item_number`, only that item, + when it is one of them.""" found = [ item for item in store.open_items() if commit_type(item.title) is not None and summary_length(commit_summary(item.title)) > SUMMARY_MAX_CHARS and item_number in (None, item.number) + and not _is_skipped(item) ] return sorted(found, key=lambda item: item.number) @@ -326,8 +353,9 @@ def _changed_since_read(store: RoadmapStore, change: TitleChange) -> bool: def apply_retitle(store: RoadmapStore, plan: RetitlePlan) -> int: """Write each new title and one comment on its item, and one comment for each skip that says - so, unless the item already holds that comment. An item whose title changed since the plan - read it is left. Returns the number of titles written.""" + so, unless the item already holds that comment. Each skip also gets the skip mark, in one write. + An item whose title changed since the plan read it is left. Returns the number of titles + written.""" written = 0 for change in plan.changes: if _changed_since_read(store, change): @@ -343,6 +371,7 @@ def apply_retitle(store: RoadmapStore, plan: RetitlePlan) -> int: body = _skip_comment_of(skip) if skip.comment and all(c.body != body for c in store.comments_on(skip.item.number)): store.comment(skip.item.number, body) + store.set_marks(skip.item, {RetitleRecordKey.SKIPPED: skip_hash(skip.item)}) return written @@ -396,4 +425,5 @@ def render_retitle_plan(plan: RetitlePlan) -> str: "person_changed_title", "plan_retitle", "render_retitle_plan", + "skip_hash", ] diff --git a/src/devops_cli/roadmap/stores/store.py b/src/devops_cli/roadmap/stores/store.py index 226f88549..a2365da1b 100644 --- a/src/devops_cli/roadmap/stores/store.py +++ b/src/devops_cli/roadmap/stores/store.py @@ -272,8 +272,21 @@ class ImplementRecordKey(StrEnum): CONTINUATIONS = "implement.continuations" +class RetitleRecordKey(StrEnum): + """What the retitle job records in an Item's job record (#4038): `retitle.skipped` holds a + hash of the title, the body and the `stage.pr` mark that the job read when it left the item + with a long title. The selection leaves out the item while the hash holds.""" + + SKIPPED = "retitle.skipped" + + JobRecord = dict[ - ItemField | RefineRecordKey | AlternativesRecordKey | StageRecordKey | ImplementRecordKey, + ItemField + | RefineRecordKey + | AlternativesRecordKey + | StageRecordKey + | ImplementRecordKey + | RetitleRecordKey, str | None, ] @@ -583,6 +596,7 @@ def require_value(field: ItemField, value: str | None, operation: str) -> None: *AlternativesRecordKey, *StageRecordKey, *ImplementRecordKey, + *RetitleRecordKey, ) ) @@ -1043,6 +1057,7 @@ def get_roadmap_store(repo: str, *, runner: GhRunner | None = None) -> RoadmapSt "Milestone", "RefineRecordKey", "RepositoryFacts", + "RetitleRecordKey", "RoadmapStore", "StageRecordKey", "Verdict", diff --git a/tests/roadmap/test_roadmap_alternatives.py b/tests/roadmap/test_roadmap_alternatives.py index d5831e7c6..4d043f189 100644 --- a/tests/roadmap/test_roadmap_alternatives.py +++ b/tests/roadmap/test_roadmap_alternatives.py @@ -667,7 +667,10 @@ def broken(full_name: str) -> RepositoryFacts: with caplog.at_level(logging.WARNING, logger="devops_cli.roadmap.alternatives"): answer = tools.github_repository("example/scanner") - assert (answer, [r.getMessage() for r in caplog.records]) == ( + assert ( + answer, + [r.getMessage() for r in caplog.records if r.name == "devops_cli.roadmap.alternatives"], + ) == ( "The read failed: GitHubOperationError.", ["A repository read of the alternatives check failed: GitHubOperationError."], ) @@ -809,7 +812,10 @@ def test_a_pypi_package_that_does_not_exist_or_fails_to_read_is_answered_as_text with caplog.at_level(logging.WARNING, logger="devops_cli.roadmap.alternatives"): answers = [tools.pypi_package(name) for name in ("none-such", "broken", "../x", "a b")] - assert (answers, [r.getMessage() for r in caplog.records]) == ( + assert ( + answers, + [r.getMessage() for r in caplog.records if r.name == "devops_cli.roadmap.alternatives"], + ) == ( [ "PyPI has no package none-such.", "The read failed: HTTPStatusError.", @@ -957,7 +963,11 @@ def test_the_decisions_are_cut_by_whole_entries_the_oldest_go_first_and_a_warnin decisions.startswith("ADR 0009: Newest decision\n"), decisions.endswith("Nine 499**: rejected, for a reason."), "ADR 0008" in decisions or "ADR 0007" in decisions, - [record.getMessage() for record in caplog.records], + [ + record.getMessage() + for record in caplog.records + if record.name == "devops_cli.roadmap.adr" + ], ) == ( True, True, diff --git a/tests/roadmap/test_roadmap_retitle_skip.py b/tests/roadmap/test_roadmap_retitle_skip.py new file mode 100644 index 000000000..deafc9b61 --- /dev/null +++ b/tests/roadmap/test_roadmap_retitle_skip.py @@ -0,0 +1,310 @@ +"""`devops roadmap retitle` records a skip mark for an item it leaves (#4038). The mark holds a +hash of what the decision read: the title, the body and the `stage.pr` mark. The selection leaves +out an item whose recorded hash is the current one, so a round does not read the comments of the +item again, does not call the model for it, and does not let it hold a place of the limit.""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from typing import Any + +import pytest + +from devops_cli.roadmap.retitle import RetitleReply, apply_retitle, long_title_items, plan_retitle +from devops_cli.roadmap.stores.store import ItemField, StageRecordKey +from tests.support.fakes.roadmap_store import InMemoryRoadmapStore, JobWrite + +REPO = "example/roadmap" +LONG = "Shorten the title of an item that a job leaves out because it is too long" +OLD = f"fix(roadmap): {LONG}" +OTHER_LONG = "Shorten the title of an item that a person edited after the job left it" +NEW_SUMMARY = "Shorten the long title of an item" +# The key of `RetitleRecordKey.SKIPPED`. The tests name it as text, so that they fail in the call +# phase, and not at the import, while the key family does not exist. +SKIPPED = "retitle.skipped" +JOB_COMMENT = "\nPlaced." + + +class Clock: + """A clock that a test moves, so that the order of events is exact.""" + + def __init__(self) -> None: + self.now = datetime(2026, 11, 4, 12, 0, tzinfo=UTC) + + def __call__(self) -> datetime: + self.now += timedelta(minutes=1) + return self.now + + +class FakeModel: + """Records each request and answers from a list of summaries, the last one repeated. A + summary over the limit fails the title form, so the item is left.""" + + def __init__(self, *summaries: str) -> None: + self.requests: list[str] = [] + self._summaries = list(summaries) or [LONG] + + def summarize(self, request: str) -> RetitleReply: + self.requests.append(request) + index = min(len(self.requests), len(self._summaries)) - 1 + return RetitleReply(summary=self._summaries[index]) + + +class CountingStore(InMemoryRoadmapStore): + """The reads that the decision of one item costs: its comments, its title changes and the + title of its plan pull request. `open_items` is the one read that every round makes.""" + + def __init__(self) -> None: + super().__init__(repo=REPO, clock=Clock()) + self.decision_reads: list[str] = [] + + def comments_on(self, number: int) -> Any: + self.decision_reads.append("comments_on") + return super().comments_on(number) + + def last_renamed_at(self, number: int) -> Any: + self.decision_reads.append("last_renamed_at") + return super().last_renamed_at(number) + + def pull_title(self, number: int) -> Any: + self.decision_reads.append("pull_title") + return super().pull_title(number) + + +@pytest.fixture +def store() -> CountingStore: + return CountingStore() + + +def _item(store: InMemoryRoadmapStore, title: str = OLD, status: str = "Ready") -> int: + return store.seed_issue( + title, + body="It asks for one change.", + fields={ItemField.PRIORITY: "P2-Medium", ItemField.STATUS: status}, + ) + + +def _planned_without_title(store: InMemoryRoadmapStore) -> int: + number = _item(store, status="Planned") + pull = store.seed_pull("docs(openspec): not a plan title", head_ref=f"docs/{number}-plan") + item = store.item(number) + assert item is not None + store.set_marks(item, {StageRecordKey.PR: str(pull)}) + return number + + +def _person_edited(store: InMemoryRoadmapStore) -> int: + number = _item(store) + store.comment(number, JOB_COMMENT) + store.rename_by_hand(number, f"fix(roadmap): {OTHER_LONG}") + return number + + +def _round(store: InMemoryRoadmapStore, model: FakeModel, *, limit: int | None = None) -> Any: + """One round of the job: the plan, then the writes.""" + plan = plan_retitle(store, repo=REPO, model=model, limit=limit) + apply_retitle(store, plan) + return plan + + +def _skip_marks(store: InMemoryRoadmapStore) -> list[JobWrite]: + return [w for w in store.job_writes() if w.key == SKIPPED] + + +def _numbers(store: InMemoryRoadmapStore) -> list[int]: + return [item.number for item in long_title_items(store)] + + +# The three reasons for a skip each leave one mark, and the next round does not take the item. + + +@pytest.mark.parametrize("reason", ["model_failed", "person_edit", "no_plan_title"]) +def test_skip_mark_leaves_an_item_that_fails_to_shorten_out_of_the_next_round( + store: CountingStore, reason: str +) -> None: + number = { + "model_failed": lambda: _item(store), + "person_edit": lambda: _person_edited(store), + "no_plan_title": lambda: _planned_without_title(store), + }[reason]() + model = FakeModel() + + first = _round(store, model) + second = _round(store, model) + + assert [skip.item.number for skip in first.skips] == [number] + assert [w.number for w in _skip_marks(store)] == [number] + assert _numbers(store) == [] + assert (second.changes, second.skips) == ((), ()) + + +def test_skip_mark_holds_a_hash_of_the_inputs_and_not_a_link_to_a_failure_class( + store: CountingStore, +) -> None: + _item(store) + + _round(store, FakeModel()) + + (mark,) = _skip_marks(store) + assert mark.value is not None + assert len(mark.value) == 64 + assert int(mark.value, 16) >= 0 + assert OLD not in mark.value + + +def test_skip_mark_is_written_for_a_skip_only_and_never_for_a_new_title( + store: CountingStore, +) -> None: + _item(store) + + _round(store, FakeModel(NEW_SUMMARY)) + + assert _skip_marks(store) == [] + + +def test_skip_mark_does_not_change_the_hash_that_it_holds(store: CountingStore) -> None: + """The mark rides in the record block of the body, which the body of an item leaves out, so + the write does not return the item to the selection.""" + number = _item(store) + _round(store, FakeModel()) + + item = store.item(number) + assert item is not None + assert {str(k): v for k, v in item.job_record.items()}.get(SKIPPED) + assert "retitle.skipped" not in item.body + assert _numbers(store) == [] + + +def test_skip_mark_frees_the_place_of_the_limit_for_the_next_item(store: CountingStore) -> None: + first = _item(store) + second = _item(store, f"fix(roadmap): {OTHER_LONG}") + model = FakeModel(LONG, LONG, NEW_SUMMARY) + + one = _round(store, model, limit=1) + two = _round(store, model, limit=1) + + assert [skip.item.number for skip in one.skips] == [first] + assert [change.item.number for change in two.changes] == [second] + + +# A change to what the decision read returns the item to the selection. + + +def _change_title(store: InMemoryRoadmapStore, number: int) -> None: + store.rename_by_hand(number, f"fix(roadmap): {OTHER_LONG}") + + +def _change_body(store: InMemoryRoadmapStore, number: int) -> None: + store.write_issue_body(number, "It asks for one change, and now it says why.") + + +def _change_plan_pull(store: InMemoryRoadmapStore, number: int) -> None: + pull = store.seed_pull("docs(openspec): plan #1 Add a title repair", head_ref="docs/plan") + item = store.item(number) + assert item is not None + store.set_marks(item, {StageRecordKey.PR: str(pull)}) + + +@pytest.mark.parametrize( + "change", [_change_title, _change_body, _change_plan_pull], ids=["title", "body", "stage_pr"] +) +def test_returns_when_changed_the_title_the_body_or_the_plan_pull( + store: CountingStore, change: Any +) -> None: + number = _item(store) + _round(store, FakeModel()) + assert _numbers(store) == [] + + change(store, number) + + assert _numbers(store) == [number] + + +def test_returns_when_changed_and_the_new_round_decides_the_item_again( + store: CountingStore, +) -> None: + number = _item(store) + _round(store, FakeModel()) + _change_body(store, number) + model = FakeModel(NEW_SUMMARY) + + plan = _round(store, model) + + assert [c.item.number for c in plan.changes] == [number] + assert len(model.requests) == 1 + + +def test_returns_when_changed_a_mark_that_is_not_the_current_hash_selects_the_item( + store: CountingStore, +) -> None: + from devops_cli.roadmap.stores.store import RetitleRecordKey + + number = _item(store) + item = store.item(number) + assert item is not None + store.set_marks(item, {RetitleRecordKey.SKIPPED: "0" * 64}) + + assert _numbers(store) == [number] + + +def test_returns_when_changed_a_comment_leaves_the_item_out(store: CountingStore) -> None: + number = _item(store) + _round(store, FakeModel()) + + store.comment(number, "A person asks about the title.") + + assert _numbers(store) == [] + + +# The cost: one write once, and no read and no model call after it. + + +def test_request_count_a_skipped_item_costs_one_write_once(store: CountingStore) -> None: + _item(store) + model = FakeModel() + + _round(store, model) + writes_after_first = len(_skip_marks(store)) + _round(store, model) + _round(store, model) + + assert (writes_after_first, len(_skip_marks(store))) == (1, 1) + + +def test_request_count_a_skipped_item_costs_no_read_and_no_model_call_in_the_next_round( + store: CountingStore, +) -> None: + _item(store) + model = FakeModel() + _round(store, model) + reads, requests = list(store.decision_reads), len(model.requests) + assert (reads, requests) == (["comments_on", "last_renamed_at"], 2) + + _round(store, model) + + assert store.decision_reads == reads + assert len(model.requests) == requests + + +def test_request_count_a_skip_with_a_comment_adds_the_mark_to_the_one_comment( + store: CountingStore, +) -> None: + number = _planned_without_title(store) + model = FakeModel() + + _round(store, model) + _round(store, model) + + assert [w.number for w in _skip_marks(store)] == [number] + assert len(store.comments_on(number)) == 1 + assert model.requests == [] + + +def test_request_count_the_dry_run_plan_lists_the_write_of_the_mark() -> None: + from devops_cli.roadmap.request_plan import retitle_requests + + _, writes = retitle_requests(REPO) + + bodies = [w for w in writes if w.stdin and '"body"' in w.stdin] + assert len(bodies) == 1 + assert bodies[0].condition