diff --git a/README.md b/README.md index 7c5157283..6fa8b4c72 100644 --- a/README.md +++ b/README.md @@ -380,7 +380,7 @@ and excessive permissions fail it, and every other finding is shown. | | | `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; 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. | -| | `devops roadmap plan-check [OPTIONS] ` | Check a plan pull request: the stage gates without a model, the title, the body, the `Semantic PR title` check and a base spec that moved since the plan. Details: docs/roadmap/plan-check.md. | +| | `devops roadmap plan-check [OPTIONS] ` | Check a plan pull request: the stage gates without a model, the token size of each task group, the title, the body, the `Semantic PR title` check and a base spec that moved since the plan. Details: docs/roadmap/plan-check.md. | | | `devops roadmap plan-recheck [OPTIONS]` | Check each change folder on the default branch again. Each folder gets one result: ok, rewrite, new-plan, delivered or cleanup. The command reads GitHub and git, calls no model and writes nothing. Details: docs/roadmap/plan-recheck.md. | | | `devops roadmap explain [OPTIONS] ` | Say, for one issue, whether the refine job, the stage job and the implement job take it. For each job the answer is yes, or no with the reason of the first rule that fails: the issue is closed or is no item, the filter, the rules of the job (the Status, the owner, the stop on record, the plan and the branch, and the setting roadmap.stage.planned_limit), and the limit of a run. For refine the reason is the wait of the item, such as a question for a person. The reasons are the texts of the 'Left out' list of stage --plan and implement --plan. The command reads GitHub and git, calls no model and writes nothing. It uses the filter and the limit that the Service uses for each job. A group over the step budget shows only when an earlier run recorded it. | | | `devops roadmap inbox [OPTIONS]` | List what waits for a person in each repository of service.repos, or in the one that --repo names. Each line has its number, its age and the step that clears it, and an old wait is marked. It reads GitHub, calls no model and writes nothing; details: docs/roadmap/inbox.md. | diff --git a/docs/CLI_REFERENCE.md b/docs/CLI_REFERENCE.md index f850e1577..917049a89 100644 --- a/docs/CLI_REFERENCE.md +++ b/docs/CLI_REFERENCE.md @@ -6690,7 +6690,7 @@ devops roadmap implement [OPTIONS] ### `devops roadmap plan-check` -**Check a plan pull request: the stage gates without a model, the title, the body, the `Semantic PR title` check and a base spec that moved since the plan. Details: docs/roadmap/plan-check.md.** +**Check a plan pull request: the stage gates without a model, the token size of each task group, the title, the body, the `Semantic PR title` check and a base spec that moved since the plan. Details: docs/roadmap/plan-check.md.** ```bash devops roadmap plan-check [OPTIONS] diff --git a/docs/ROADMAP_WORKFLOW.md b/docs/ROADMAP_WORKFLOW.md index 285daf027..3d67b571e 100644 --- a/docs/ROADMAP_WORKFLOW.md +++ b/docs/ROADMAP_WORKFLOW.md @@ -114,7 +114,7 @@ These are all the keys of the job record, by the job that writes them: | Stage | `stage.needs_fix` | The plan pull request that the Service told about the label `plan/needs-fix` | | Stage | `stage.failed_at`, `stage.failed_error` | When the last plan or write of the Item failed and the class of its error. The job leaves the Item out for `roadmap.stage.retry_hours` hours; a staged Item or an Item stopped on the budget clears both | | Implement | `implement.group`, `implement.sha`, `implement.branch`, `implement.pr` | The last group that passed, the last commit, the branch and the pull request | -| Implement | `implement.stopped`, `implement.failure`, `implement.spend`, `implement.trace` | The step that stopped the Item, the class of its failure, the spend of its steps, and the trace of its last run | +| Implement | `implement.stopped`, `implement.failure`, `implement.spend`, `implement.trace` | The step that stopped the Item, the class of its failure, the spend of its steps with the input tokens of the first request of each step (`steps`, kept apart from the trace, which drops its oldest events), and the trace of its last run | | Implement | `implement.check`, `implement.check_log` | The check that failed in the stop, and the masked end of its log (at most 2000 characters). A retry and a clear rule of the Service remove both with the stop | | Implement | `implement.wip` | The work in progress that a step kept: its group, the tasks done, the tasks of the group and the last commit as JSON. The job removes it when the group passes | | Implement | `implement.job`, `implement.job_state`, `implement.job_digest` | The runner Job that the Service made, its state after a round reported it, and the digest of the runner image that the Job ran | diff --git a/docs/commands/roadmap.md b/docs/commands/roadmap.md index 67fd50f20..3f7170c0c 100644 --- a/docs/commands/roadmap.md +++ b/docs/commands/roadmap.md @@ -166,7 +166,7 @@ devops roadmap implement [OPTIONS] ## `devops roadmap plan-check` -**Check a plan pull request: the stage gates without a model, the title, the body, the `Semantic PR title` check and a base spec that moved since the plan. Details: docs/roadmap/plan-check.md.** +**Check a plan pull request: the stage gates without a model, the token size of each task group, the title, the body, the `Semantic PR title` check and a base spec that moved since the plan. Details: docs/roadmap/plan-check.md.** ```bash devops roadmap plan-check [OPTIONS] diff --git a/docs/roadmap/plan-check.md b/docs/roadmap/plan-check.md index 550b2d78b..3bf88532f 100644 --- a/docs/roadmap/plan-check.md +++ b/docs/roadmap/plan-check.md @@ -13,7 +13,13 @@ the issue in the `Plans #` line of the body. A pull request with no such more than one, is a problem. The GitHub client gives the title, the body, the base and the check runs. The command reads the -issue that the body plans, for its acceptance criteria. It builds no model client. +issue that the body plans, for its acceptance criteria and for the text of the item that the size +measure counts. The command takes the body of the issue with no record block of the jobs, as the +stage job reads it. It builds no model client. + +To measure the groups, the command makes a detached worktree of the head commit in a temporary +folder and removes it when the measure ends. The files that the tasks name are the files of the head +commit, whatever the working tree of the checkout holds. ## What it checks @@ -22,6 +28,15 @@ issue that the body plans, for its acceptance criteria. It builds no model clien - The read: the reader reports each change file that it cannot read, each task group that the model refuses and each `Verification:` command that the implement step cannot run, as a `read` problem. It never ends in a traceback. +- The size of each task group (#3499): the command measures each group with `measure_groups`, the + function of the stage job, on the tracked files of the head commit. The item and the files of the + change folder are the shared text, as in the stage job. It reports a `group-over-budget` problem + for a group above the setting `roadmap.stage.step_tokens`, and a `group-no-files` problem for a + group that changes a file that exists and packs no file, because no code span in its tasks names + one. A group that only creates files packs none by nature, and the command accepts it. The plan + holds no estimate of the lines of a created file, so the command counts one line for each. For a + group that creates files, its figure is lower than the stage figure by 32 tokens for each + estimated line above the first. For any other group, the two figures are equal. - The title: `docs(openspec): plan # `, with at most `SUMMARY_MAX_CHARS` characters after the colon. - The body: at most `BODY_MAX_CHARS` characters, one `Plans #` line, and no closing keyword diff --git a/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/.openspec.yaml b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/.openspec.yaml new file mode 100644 index 000000000..1491b921e --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-05 diff --git a/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/design.md b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/design.md new file mode 100644 index 000000000..218bda7b4 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/design.md @@ -0,0 +1,25 @@ +# Design + +## Decisions + +- **Call `measure_groups` from `plan_check` with the groups that `read_change` gives.**: The stage job measures with this function, so one function gives one figure. The test compares the figure of `plan-check` with a call of `measure_groups` on the same plan. +- **Build the shared text with one function, `shared_texts`, that the stage job and `plan-check` both call.**: The shared text is the title and body of the issue and the files of the change folder. Two copies of that list can drift, and the figures would then differ with no test to show it. +- **Measure on the head commit of the pull request, in a detached worktree of `checkout_at`.**: `named_files`, `packed_file` and `rules_sections` read a directory, and the working tree of the caller can be at any commit or hold edits. The head commit is the tree that the plan names, and it is the same tree on each run. +- **Move `checkout_at` to `src/devops_cli/roadmap/clone_git.py` and give it the `operation` of its caller.**: `clone_git.py` already holds `git`. `plan_check.py` must not import an eval module for a git helper, and `stage_eval_run.py` and `stage_verify_eval.py` keep the same behavior. +- **Add `files` to `MeasuredGroup`.**: The measure already finds the files that the step packs. A second call of `named_files` in `plan_check.py` would repeat that work and could differ from it. +- **Read the budget with `load_settings().roadmap.stage.step_tokens`, and add no option.**: The issue names this setting. A person who needs another budget sets `DEVOPS_CLI_ROADMAP_STAGE_STEP_TOKENS`. +- **Report each finding as a problem with the rules `group-over-budget` and `group-no-files`, on the path `tasks.md` of the change folder.**: A problem has a path, a rule and a message, and the output and the exit code already follow from that. +- **Report `group-no-files` only for a group that changes a file that exists.**: A group whose `changes` are all in `creates` has no file to pack, and the plan is right. The sample plan of `tests/roadmap/test_roadmap_plan_check.py` has such a group, and a valid plan prints no problem. +- **Keep the one-line estimate of a created file.**: `tasks.md` holds no estimate, and the plan reader gives each created path one line. The figure of `plan-check` is then a lower bound for a group that creates files, and equal to the stage figure for any other group. +- **Record the size of the first request in `implement.spend`, under `steps`, from the `usage` of the first reply of the step.**: The reply reports the input tokens of the request that it answers, so the first reply gives the size of the first request. `implement.spend` is written apart from `implement.trace`, so the trace limit does not touch it. +- **Take the first request of the first turn of the first tier only.**: A later turn or tier continues a conversation, and its first request is not the first request of the step. + +## Rejected alternatives + +- **Measure on the working tree of the checkout that runs `plan-check`.**: The working tree is at any commit and can hold edits, so two runs on one pull request can give two figures. The test of equality with the stage job would then depend on the state of the checkout. +- **Check out only the named files and the routed documents.**: `rules_sections` reads the routes from `AGENTS.md` and the documents that they name. A second list of paths to extract can differ from what the step reads, and the full checkout of this repository takes about 1 second. +- **Report every group that packs no file.**: The run over the change folders of `origin/main` found 8 such groups, and 2 of them only create files. A plan with such a group is valid, and the report would give it a false problem. +- **Add the estimate of created lines to `tasks.md`.**: This changes the `Changes:` line that the renderer, the reader, the path gates and the implement step all read, and plans on the default branch hold the old form. The issue does not name it. The limit is stated in the documents instead. +- **Add a `--step-tokens` option to `plan-check`.**: The issue asks for the setting of the stage job, and the variable already overrides it. +- **Put the first request in a `model` event of the trace only.**: `encode` drops the oldest events first, so a long run loses the first event. The record #2636 held 77 events and dropped 154. +- **Add the figure as a column of the table in the job comment.**: The table shows the totals of a step. The figure is a fact for a person who compares plan and model, and the record holds it. diff --git a/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/proposal.md b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/proposal.md new file mode 100644 index 000000000..2704e0d4d --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/proposal.md @@ -0,0 +1,51 @@ +# Proposal + +## Why + +The stage comment gives a token figure for each task group of a plan (#3499). The figure goes stale when the plan or the files that its tasks name change, and no gate measures it again. `devops roadmap plan-check` has no token measure. The implement step refuses a group only above the window minus the turn reserve, so a plan above the stage budget can start a run. The stage figures of four plans were 5150, 5687, 3798 and 31191 tokens. A measure of the same plans now gives about 25778, 12835, 60701 and 43832 tokens. No record holds the size of the first request of a step, so nobody can compare a stage figure with what the model received. + +## What Changes + +- `plan-check` measures each task group of the plan with `measure_groups`, the function of the stage job, on the tracked files of the pull request head. It reports a `group-over-budget` problem for a group above `roadmap.stage.step_tokens`, and a `group-no-files` problem for a group that changes a file that exists and packs no file. A group that only creates files packs none by nature, and the command accepts it. +- One function, `shared_texts` in `src/devops_cli/roadmap/stage_change.py`, builds the text that all groups share: the item and the files of the change folder. The stage job and `plan-check` both call it, so their figures stay equal. +- `MeasuredGroup` gains `files`, the number of files that the step packs for the group, so the stage job and `plan-check` count them in one place. +- `checkout_at` moves from `src/devops_cli/roadmap/stage_eval_run.py` to `src/devops_cli/roadmap/clone_git.py`, so `plan-check` can read the head commit as a checkout without a dependency on the eval. +- A step records the input tokens of its first model request. `implement.spend` holds them for each step under `steps`, apart from the trace, so the trace limit never drops them. +- The change carries a spec delta on `roadmap-plan-check` and on `roadmap-implement-job`, because the command output and the job record are behavior that a consumer observes. +- The change fits one pull request. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `roadmap-plan-check`: Plan-check measures each task group +- `roadmap-implement-job`: The record keeps the size of the first request of each step + +## Impact + +- Code: `src/devops_cli/roadmap/plan_check.py`, `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage.py`, `src/devops_cli/roadmap/clone_git.py`, `src/devops_cli/roadmap/stage_eval_run.py`, `src/devops_cli/roadmap/stage_verify_eval.py`, `src/devops_cli/roadmap/implement_step.py`, `src/devops_cli/roadmap/implement_run.py`, `src/devops_cli/lang/en/messages.py`, `src/devops_cli/lang/en/help.py` +- Documents: `docs/roadmap/plan-check.md`, `docs/ROADMAP_WORKFLOW.md`, `docs/commands/roadmap.md`, `docs/CLI_REFERENCE.md`, `README.md` +- Tests: `tests/roadmap/test_roadmap_plan_check.py`, `tests/roadmap/test_roadmap_stage_eval_run.py`, `tests/roadmap/test_roadmap_implement_step_trace.py`, `tests/roadmap/test_roadmap_implement_job_trace.py` + +## Feasibility + +- Read of `src/devops_cli/roadmap/plan_check.py`: the module has no call to `measure_groups` and no token count. `grep -rn measure_groups src` finds the callers `stage.py` and `implement_run.py` only. The premise of the issue holds. +- Read of `src/devops_cli/roadmap/stage_change.py`: `measure_groups(root, groups, shared=...)` counts the shared text, the text of the group, the files and the rules that the step reads under `root`, and 32 tokens for each estimated line of a created file. The stage job calls it with the item and the rendered files of the change folder. +- Read of `src/devops_cli/roadmap/implement_step.py`: the step refuses inputs above `window - CONST_ROADMAP_IMPLEMENT_TURN_RESERVE_TOKENS`, and no stage budget applies. The statement of the issue holds. +- Read of `src/devops_cli/roadmap/implement_trace.py`: `encode` drops the oldest events first, so the first model event of a long run is the first to go. A figure of the first request needs a place outside the trace. +- Command output: a script ran `read_change` and `measure_groups` over the 56 change folders of `origin/main`. 23 folders read, and 65 groups gave 2 groups above 40000 tokens, none above 64000 (the default of `roadmap.stage.step_tokens`), and 8 groups that pack no file. 6 of the 8 change an existing file that no task names in a code span, and 2 only create files. +- Limit that the plan format sets: `tasks.md` holds no estimate of the lines of a created file. The plan reader gives each created path one line, so the figure of `plan-check` for a group that creates files is lower than the stage figure by 32 tokens for each estimated line above the first. For a group that creates no file the two figures are equal. The proposal keeps the format: a change of the `Changes:` line is a change of the plan format, and the issue does not name it. The docs of the command state the limit. +- The deployed value of `roadmap.stage.step_tokens` is not in this checkout. The command reads the setting of the machine that runs it, so it uses the value that the deployment sets, 40000 or 64000. +- `uv run devops ci test tests/roadmap/test_roadmap_plan_check.py` and `uv run devops ci test tests/roadmap/test_roadmap_implement_job_trace.py` run the new tests and fail before the code and pass after it. + +## Existing Tool Considered + +Project code covers the need: `measure_groups`, `count_tokens`, `named_files`, `packed_file` and `rules_sections` give the figure, and `checkout_at` gives the head commit as a checkout. No outside tool knows the inputs of an implement step. The change reuses all of them and builds no counter. The cost is about 8,000 tokens to build, 1,000 tokens to keep and about 2 hours elapsed. The change deletes the copy of `checkout_at` in `src/devops_cli/roadmap/stage_eval_run.py` by moving it, and deletes no other code. + +## Dependencies + +The plan reader of #3482 reads a plan with no crash, so `plan-check` has a plan to measure. diff --git a/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/specs/roadmap-implement-job/spec.md b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/specs/roadmap-implement-job/spec.md new file mode 100644 index 000000000..c64424e16 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/specs/roadmap-implement-job/spec.md @@ -0,0 +1,12 @@ +## ADDED Requirements + +### Requirement: The record keeps the size of the first request of each step +The implement job SHALL record in `implement.spend`, under `steps`, the input tokens of the first model request of each step, from the usage of the first reply of the step. The job SHALL write the figure apart from the trace, so the limit of the trace never drops it. + +#### Scenario: A step records its first request +- **WHEN** a step makes two model requests +- **THEN** `implement.spend` holds for that step the input tokens of the first request, and they equal `tokens_in` of the first `model` event of the step. + +#### Scenario: A trace over its limit keeps the figure +- **WHEN** a run has more events than `CONST_ROADMAP_IMPLEMENT_TRACE_CHARS` allows +- **THEN** the trace has a `dropped` count above 0, and `implement.spend` still holds the first-request figure of each step. diff --git a/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/specs/roadmap-plan-check/spec.md b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/specs/roadmap-plan-check/spec.md new file mode 100644 index 000000000..ca7240d6e --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/specs/roadmap-plan-check/spec.md @@ -0,0 +1,34 @@ +## ADDED Requirements + +### Requirement: Plan-check measures each task group +`devops roadmap plan-check` SHALL measure each task group of the plan with `measure_groups`, the function of the stage job, on the tracked files of the pull request head, with the body of the issue as the store reads it for the stage job, with no record block. The plan holds no estimate of the lines of a created file, so the measure SHALL count one estimated line for each created file. + +#### Scenario: The figure equals the figure of the stage job +- **WHEN** a plan has a group that estimates one line for each file that it creates, and the budget is one token below the figure that `measure_groups` gives the stage job for that group +- **THEN** the `group-over-budget` problem holds that figure. + +#### Scenario: The record block of the issue is not in the figure +- **WHEN** the body of the issue ends with the record block of the jobs, and the budget is one token below the figure that the stage job gives for the same text with no block +- **THEN** the `group-over-budget` problem holds the figure of the stage job, and a budget equal to that figure gives no problem. + +### Requirement: Plan-check reports a group over the budget +The command SHALL read the budget from the setting `roadmap.stage.step_tokens`, and SHALL report a `group-over-budget` problem for each group whose figure is above it. + +#### Scenario: A group over the budget is reported +- **WHEN** a group measures more tokens than `roadmap.stage.step_tokens` +- **THEN** the output holds a `group-over-budget` problem that names the group, its figure and the budget, and the exit code is 1. + +#### Scenario: A group under the budget is not reported +- **WHEN** each group measures no more tokens than `roadmap.stage.step_tokens` +- **THEN** the output holds no `group-over-budget` problem. + +### Requirement: Plan-check reports a group that packs no file +The command SHALL report a `group-no-files` problem for each group that changes a file that exists and packs no file. A group that only creates files SHALL be no problem. + +#### Scenario: A group that packs no file is reported +- **WHEN** a group changes a file of the pull request head and no code span of its tasks names a file of the head +- **THEN** the output holds a `group-no-files` problem that names the group, and the exit code is 1. + +#### Scenario: A group that only creates files is not reported +- **WHEN** each path that a group changes is a path that it creates +- **THEN** the output holds no `group-no-files` problem for that group. diff --git a/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/tasks.md b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/tasks.md new file mode 100644 index 000000000..e0bfe4642 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-3499-plan-check-measures-each-group/tasks.md @@ -0,0 +1,36 @@ +# Tasks + +## 1. Plan-check measures each task group + +Reads: `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage.py`, `src/devops_cli/roadmap/stage_eval_run.py`, `src/devops_cli/roadmap/stage_verify_eval.py`, `src/devops_cli/roadmap/clone_git.py`, `tests/support/roadmap_stage.py`. Changes: `src/devops_cli/roadmap/plan_check.py`, `src/devops_cli/roadmap/stage_change.py`, `src/devops_cli/roadmap/stage.py`, `src/devops_cli/roadmap/clone_git.py`, `src/devops_cli/roadmap/stage_eval_run.py`, `src/devops_cli/roadmap/stage_verify_eval.py`, `src/devops_cli/lang/en/messages.py`, `src/devops_cli/lang/en/help.py`, `tests/roadmap/test_roadmap_plan_check.py`, `tests/roadmap/test_roadmap_stage_eval_run.py`, `docs/roadmap/plan-check.md`, `docs/commands/roadmap.md`, `docs/CLI_REFERENCE.md`, `README.md`. + +- [x] 1.1 Add to `tests/roadmap/test_roadmap_plan_check.py` a test that the figure of `plan-check` for a group equals the figure that `measure_groups` gives the stage job for the same plan, a test that the record block at the end of the body of the issue is not in the figure, a test that `plan-check` reports a group over the budget, a test that it reports no problem for a group under the budget, a test that it reports a group that changes a file that exists and packs no file, and a test that a group that only creates a file has no problem + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_plan_check.py` fails on the base +- [x] 1.2 Move `checkout_at` to `src/devops_cli/roadmap/clone_git.py` with an `operation` argument and update its two callers, add `files` to `MeasuredGroup` and add `shared_texts` to `src/devops_cli/roadmap/stage_change.py`, call `shared_texts` in `src/devops_cli/roadmap/stage.py`, measure each group in `src/devops_cli/roadmap/plan_check.py` on the head commit with the setting `roadmap.stage.step_tokens`, and add the messages `plan_check_group_over_budget` and `plan_check_group_no_files` to `src/devops_cli/lang/en/messages.py` + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_plan_check.py tests/roadmap/test_roadmap_stage_eval_run.py tests/roadmap/test_roadmap_stage_measure.py` passes +- [x] 1.3 Edit `docs/roadmap/plan-check.md` to say that the command measures each group on the head commit with the budget of `roadmap.stage.step_tokens`, reports a group over it and a group that packs no file, and counts a created file as one line, edit the help text `plan_check` in `src/devops_cli/lang/en/help.py` to name the measure, and run `uv run devops docs generate --sync-readme` + - Verification: `uv run devops ci docs` passes + +## 2. The record keeps the size of the first request of each step + +Reads: `src/devops_cli/roadmap/implement_trace.py`, `tests/support/roadmap_implement.py`, `tests/support/model_script.py`. Changes: `src/devops_cli/roadmap/implement_step.py`, `src/devops_cli/roadmap/implement_run.py`, `tests/roadmap/test_roadmap_implement_step_trace.py`, `tests/roadmap/test_roadmap_implement_job_trace.py`, `docs/ROADMAP_WORKFLOW.md`. + +- [x] 2.1 Add to `tests/roadmap/test_roadmap_implement_step_trace.py` a test that the outcome of a step holds the input tokens of its first model request, and add to `tests/roadmap/test_roadmap_implement_job_trace.py` a test that `implement.spend` holds that figure for each step, and a test that a run with more events than the trace limit keeps the figure of each step while its trace has a `dropped` count above 0 + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_implement_step_trace.py tests/roadmap/test_roadmap_implement_job_trace.py` fails on the base +- [x] 2.2 Add `first_request_tokens` to `_Run` and to `StepOutcome` in `src/devops_cli/roadmap/implement_step.py`, set it from the first reply of the first turn of the step, and write `steps` in `_spend` of `src/devops_cli/roadmap/implement_run.py` + - Verification: `uv run devops ci test tests/roadmap/test_roadmap_implement_step_trace.py tests/roadmap/test_roadmap_implement_job_trace.py` passes +- [x] 2.3 Edit `docs/ROADMAP_WORKFLOW.md` to say that `implement.spend` holds the input tokens of the first request of each step, apart from the trace + - Verification: `uv run devops ci docs` passes + +## Criteria + +- Group 1: `plan-check` calls `measure_groups`. A test holds that the figure of `plan-check` equals the figure of the stage job for the same plan. +- Group 1: A test: the record block at the end of the body of the issue is not in the figure of `plan-check`. +- Group 1: A test: `plan-check` prints each group over the budget. +- Group 1: A test: `plan-check` prints each group with 0 packed files. +- Group 2: A test: the run record of a fixture step holds the size of its first request. +- Group 2: A test: a fixture step with more events than the trace limit keeps the size of its first request, and its trace has a `dropped` count above 0. + +## Workflow follow-up + +- Pending a person: review the diff in tuicr (tuicr -r ..HEAD). diff --git a/openspec/specs/roadmap-implement-job/spec.md b/openspec/specs/roadmap-implement-job/spec.md index edea0eb1c..0c955fb58 100644 --- a/openspec/specs/roadmap-implement-job/spec.md +++ b/openspec/specs/roadmap-implement-job/spec.md @@ -1260,3 +1260,14 @@ The implement selection SHALL leave out a Planned item whose plan check needs th #### Scenario: Git fails to list the default tree - **WHEN** A run judges two Planned items and `git ls-tree` of `origin/` fails. - **THEN** `plan_implement` returns no target, and each item has a left-out reason that says its plan cannot be read from git. + +### Requirement: The record keeps the size of the first request of each step +The implement job SHALL record in `implement.spend`, under `steps`, the input tokens of the first model request of each step, from the usage of the first reply of the step. The job SHALL write the figure apart from the trace, so the limit of the trace never drops it. + +#### Scenario: A step records its first request +- **WHEN** a step makes two model requests +- **THEN** `implement.spend` holds for that step the input tokens of the first request, and they equal `tokens_in` of the first `model` event of the step. + +#### Scenario: A trace over its limit keeps the figure +- **WHEN** a run has more events than `CONST_ROADMAP_IMPLEMENT_TRACE_CHARS` allows +- **THEN** the trace has a `dropped` count above 0, and `implement.spend` still holds the first-request figure of each step. diff --git a/openspec/specs/roadmap-plan-check/spec.md b/openspec/specs/roadmap-plan-check/spec.md index 32a301e78..6ac3adaf0 100644 --- a/openspec/specs/roadmap-plan-check/spec.md +++ b/openspec/specs/roadmap-plan-check/spec.md @@ -161,3 +161,36 @@ The plan reader SHALL return the document work of a group from a documentation t #### Scenario: The document mark matches the new template - **WHEN** the plan reader builds a proposal from a change folder whose group renders a documentation task that changes only `SECURITY.md` - **THEN** the document work of the group is present in the built proposal and reads `Edit `SECURITY.md``. + +### Requirement: Plan-check measures each task group +`devops roadmap plan-check` SHALL measure each task group of the plan with `measure_groups`, the function of the stage job, on the tracked files of the pull request head, with the body of the issue as the store reads it for the stage job, with no record block. The plan holds no estimate of the lines of a created file, so the measure SHALL count one estimated line for each created file. + +#### Scenario: The figure equals the figure of the stage job +- **WHEN** a plan has a group that estimates one line for each file that it creates, and the budget is one token below the figure that `measure_groups` gives the stage job for that group +- **THEN** the `group-over-budget` problem holds that figure. + +#### Scenario: The record block of the issue is not in the figure +- **WHEN** the body of the issue ends with the record block of the jobs, and the budget is one token below the figure that the stage job gives for the same text with no block +- **THEN** the `group-over-budget` problem holds the figure of the stage job, and a budget equal to that figure gives no problem. + +### Requirement: Plan-check reports a group over the budget +The command SHALL read the budget from the setting `roadmap.stage.step_tokens`, and SHALL report a `group-over-budget` problem for each group whose figure is above it. + +#### Scenario: A group over the budget is reported +- **WHEN** a group measures more tokens than `roadmap.stage.step_tokens` +- **THEN** the output holds a `group-over-budget` problem that names the group, its figure and the budget, and the exit code is 1. + +#### Scenario: A group under the budget is not reported +- **WHEN** each group measures no more tokens than `roadmap.stage.step_tokens` +- **THEN** the output holds no `group-over-budget` problem. + +### Requirement: Plan-check reports a group that packs no file +The command SHALL report a `group-no-files` problem for each group that changes a file that exists and packs no file. A group that only creates files SHALL be no problem. + +#### Scenario: A group that packs no file is reported +- **WHEN** a group changes a file of the pull request head and no code span of its tasks names a file of the head +- **THEN** the output holds a `group-no-files` problem that names the group, and the exit code is 1. + +#### Scenario: A group that only creates files is not reported +- **WHEN** each path that a group changes is a path that it creates +- **THEN** the output holds no `group-no-files` problem for that group. diff --git a/src/devops_cli/lang/en/help.py b/src/devops_cli/lang/en/help.py index dc37c1c71..0a7111124 100644 --- a/src/devops_cli/lang/en/help.py +++ b/src/devops_cli/lang/en/help.py @@ -1470,9 +1470,9 @@ class RoadmapCommandHelp: "values a read gives." ) plan_check: str = ( - "Check a plan pull request: the stage gates without a model, the title, the body, the " - "`Semantic PR title` check and a base spec that moved since the plan. Details: " - "docs/roadmap/plan-check.md." + "Check a plan pull request: the stage gates without a model, the token size of each task " + "group, the title, the body, the `Semantic PR title` check and a base spec that moved " + "since the plan. Details: docs/roadmap/plan-check.md." ) plan_check_arg: str = "The number of the plan pull request to check." plan_check_json: str = ( diff --git a/src/devops_cli/lang/en/messages.py b/src/devops_cli/lang/en/messages.py index 645ba6aa7..925cb2187 100644 --- a/src/devops_cli/lang/en/messages.py +++ b/src/devops_cli/lang/en/messages.py @@ -3542,6 +3542,15 @@ class RoadmapMessages: plan_check_moved_missing: str = ( "The base spec of `{capability}` no longer holds the requirement `{requirement}`." ) + plan_check_group_over_budget: str = ( + "Group {number}, `{title}`, measures {tokens} tokens. A step takes at most {budget} " + "tokens (`roadmap.stage.step_tokens`). Split the group." + ) + plan_check_group_no_files: str = ( + "Group {number}, `{title}`, packs no file. No code span in its tasks names a file of the " + "head commit, so the step starts with none of the files that it changes. Name each file " + "that the group changes in a code span of a task." + ) plan_check_changed: str = "The plan check could not read the pull request {number}: {reason}" plan_check_removed: str = "The pull request {number} has no title in GitHub." plan_check_failed: str = "The plan check found {count} problem(s)." diff --git a/src/devops_cli/roadmap/clone_git.py b/src/devops_cli/roadmap/clone_git.py index 5450b1ca1..0d9a23469 100644 --- a/src/devops_cli/roadmap/clone_git.py +++ b/src/devops_cli/roadmap/clone_git.py @@ -6,6 +6,10 @@ from __future__ import annotations +import shutil +import tempfile +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path from devops_cli.config.constants import CONST_GIT_CLI @@ -31,6 +35,22 @@ def git(cwd: Path, *args: str, operation: str) -> str: return proc.stdout.strip() +@contextmanager +def checkout_at(repository: Path, commit: str, *, operation: str) -> Iterator[Path]: + """The tracked files of `commit` in a new detached worktree of `repository`, removed after. + A commit the repository lacks raises `GitOperationError` for `operation`.""" + parent = Path(tempfile.mkdtemp(prefix="roadmap-checkout-")) + tree = parent / "tree" + try: + git(repository, "worktree", "add", "--detach", str(tree), commit, operation=operation) + try: + yield tree + finally: + git(repository, "worktree", "remove", "--force", str(tree), operation=operation) + finally: + shutil.rmtree(parent, ignore_errors=True) + + def default_branch(source: Path, *, operation: str) -> str: """The default branch of the clone, from `refs/remotes/origin/HEAD`.""" ref = git(source, "symbolic-ref", "--short", "refs/remotes/origin/HEAD", operation=operation) diff --git a/src/devops_cli/roadmap/implement_run.py b/src/devops_cli/roadmap/implement_run.py index 984259728..9ef5a842f 100644 --- a/src/devops_cli/roadmap/implement_run.py +++ b/src/devops_cli/roadmap/implement_run.py @@ -360,6 +360,11 @@ def _step_label(group: int) -> str: return MESSAGES.roadmap.implement_step_group.format(group=group) +def _outcome_label(outcome: StepOutcome) -> str: + """The label of the step that left `outcome`: its group, or the fix of a failed check.""" + return _step_label(outcome.group) if outcome.group else MESSAGES.roadmap.implement_step_fix + + @contextmanager def _worktree(source: Path, target: ImplementTarget) -> Iterator[Path]: """The worktree on the staged branch, taken over from `origin`, or for a Planned item on the new @@ -504,7 +509,9 @@ def _item_spend(self) -> ItemSpend: ) def _spend(self) -> str: - """The spend of every step, itemized under the tier that served it (#3060).""" + """The spend of every step, itemized under the tier that served it (#3060), and the input + tokens of the first request of each step (#3499). The trace drops its oldest events when it + is long, so the size of a first request is kept here.""" tiers: dict[str, dict[str, int]] = {} for outcome in self.outcomes: for tier, spend in outcome.tier_spend.items(): @@ -519,6 +526,10 @@ def _spend(self) -> str: "requests": sum(o.requests for o in self.outcomes), "input_tokens": sum(o.input_tokens for o in self.outcomes), "output_tokens": sum(o.output_tokens for o in self.outcomes), + "steps": [ + {"step": _outcome_label(o), "first_request_tokens": o.first_request_tokens} + for o in self.outcomes + ], "tiers": tiers, } ) @@ -1213,7 +1224,7 @@ def _scoped_lines(self, since: _Mark) -> list[str]: def _row(self, outcome: StepOutcome) -> SummaryRow: """The row of the table for a step: its attempts are the first turn and the repair rounds, and its last failing command is in the trace.""" - step = _step_label(outcome.group) if outcome.group else MESSAGES.roadmap.implement_step_fix + step = _outcome_label(outcome) return SummaryRow( step=step, status=outcome.status.value, diff --git a/src/devops_cli/roadmap/implement_step.py b/src/devops_cli/roadmap/implement_step.py index 8e08acdbc..cc99f24c8 100644 --- a/src/devops_cli/roadmap/implement_step.py +++ b/src/devops_cli/roadmap/implement_step.py @@ -390,8 +390,9 @@ class StepOutcome(BaseModel): the step deleted (#3092). `repair_rounds` counts the extra turns that the step used (#3043). `tier` is the last tier that the step ran on, `tiers` names every tier it attempted, and `tier_spend` holds the requests and tokens of each tier (#3060). `failure_class` is the class - of a stopped step, when the step knows its cause (#3476). `planned_requests` are the requests - a dry run lists. `fail_to_pass` is the replay of a behavior group's tests on its base commit, + of a stopped step, when the step knows its cause (#3476). `first_request_tokens` is the input + tokens of the first model request of the step, which is the size of that request (#3499), and + 0 when no request got a reply. `planned_requests` are the requests a dry run lists. `fail_to_pass` is the replay of a behavior group's tests on its base commit, and `verified` holds the verify commands of a group of kind `other`, of a done step (#3042).""" model_config = ConfigDict(frozen=True) @@ -415,6 +416,7 @@ class StepOutcome(BaseModel): tiers: tuple[str, ...] = () tier_spend: dict[str, TierSpend] = {} failure_class: FailureClass | None = None + first_request_tokens: int = 0 dry_run: bool = False planned_requests: tuple[str, ...] = () fail_to_pass: BaseReplay | None = None @@ -571,7 +573,9 @@ class _Run: the request can fix the error that ended it (#2794). `provider_error` says that a failed model call ended it, which moves the step to the next tier (#3060). `failure_class` is the class of `reason`: the output cap, or what the classifier reads in the text of the error (#3476). The - tier decision and the stop of the step read this one class.""" + tier decision and the stop of the step read this one class. `first_request_tokens` is the input + tokens of the first request that got a reply, which is the size of that request (#3499), and 0 + when no request got one.""" reason: str last_message: str @@ -583,16 +587,19 @@ class _Run: resend: Conversation | None = None provider_error: bool = False failure_class: FailureClass | None = None + first_request_tokens: int = 0 def then(self, later: _Run) -> _Run: """This turn followed by `later`, which continued its conversation: the end and the last - text of `later`, and the spend of both.""" + text of `later`, and the spend of both. The first request stays that of this turn, unless + this turn got no reply.""" return _Run( reason=later.reason, last_message=later.last_message or self.last_message, requests=self.requests + later.requests, input_tokens=self.input_tokens + later.input_tokens, output_tokens=self.output_tokens + later.output_tokens, + first_request_tokens=self.first_request_tokens or later.first_request_tokens, tool_calls=self.tool_calls + later.tool_calls, conversation=later.conversation, resend=later.resend, @@ -776,6 +783,7 @@ def _run_turn( requests=len(replies), input_tokens=sum(reply.usage.input_tokens for reply in replies), output_tokens=sum(reply.usage.output_tokens for reply in replies), + first_request_tokens=replies[0].usage.input_tokens if replies else 0, tool_calls=_tool_calls(replies), conversation=kept, resend=resend, @@ -1623,6 +1631,7 @@ def _attempt( number = 1 rounds_spent = False tool_calls = 0 + first_request = 0 total = TierSpend() spent: dict[str, TierSpend] = {} tried: list[str] = [] @@ -1639,6 +1648,7 @@ def _attempt( log.fail_to_pass = None log.verified.clear() run = _turn(agent, tier_model, log, inputs.message) + first_request = first_request or run.first_request_tokens failure = ( _Failure(run.reason, klass=run.failure_class) if run.reason @@ -1683,6 +1693,7 @@ def _attempt( tool_calls=tool_calls, input_tokens=total.input_tokens, output_tokens=total.output_tokens, + first_request_tokens=first_request, removed=tuple(log.removed), repair_rounds=rounds, tier=tried[-1] if tried else "", diff --git a/src/devops_cli/roadmap/plan_check.py b/src/devops_cli/roadmap/plan_check.py index f1eb85ae5..cf871eed1 100644 --- a/src/devops_cli/roadmap/plan_check.py +++ b/src/devops_cli/roadmap/plan_check.py @@ -13,13 +13,14 @@ from __future__ import annotations import re -from collections.abc import Callable, Mapping +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass from pathlib import Path from typing import Final, Protocol from devops_cli.config.constants import CONST_OPENSPEC_CHANGES_DIR, CONST_OPENSPEC_SPECS_DIR from devops_cli.config.defaults import DEFAULT_GH_REST_PER_PAGE +from devops_cli.config.settings import load_settings from devops_cli.dry_run import PlannedRequest from devops_cli.exceptions.git import GitOperationError from devops_cli.github.check_verdict import ( @@ -30,7 +31,7 @@ from devops_cli.github.client import IssueView, PullRequest from devops_cli.lang import MESSAGES from devops_cli.openspec.tool import OpenSpecTool -from devops_cli.roadmap.clone_git import git +from devops_cli.roadmap.clone_git import checkout_at, git from devops_cli.roadmap.plan_read import ( PlanReadError, change_root, @@ -46,9 +47,12 @@ StageProposal, check_proposal, closing_keyword_in, + measure_groups, + shared_texts, ) from devops_cli.roadmap.stage_paths import StagedTree from devops_cli.roadmap.stage_validate import validate_change +from devops_cli.roadmap.store import split_record GIT_OPERATION: Final[str] = "roadmap.plan-check" SEMANTIC_TITLE_CHECK: Final[str] = "Semantic PR title" @@ -349,6 +353,48 @@ def _gate_problems( ] +def _size_problems( + proposal: StageProposal, + root: str, + head: str, + source: Path, + title: str, + body: str, + paths: Sequence[str], +) -> list[PlanProblem]: + """The groups of the plan that the stage measure faults (#3499). `measure_groups`, the function + of the stage job, measures each group on the tracked files of `head`, with the item and the + files `paths` of the change folder as the shared text. `body` is the body of the issue with no + record block, as the store reads it for the stage job. The files are read from the checkout, + not from `git show`, which strips the end of a text, so the figure is the figure of the stage + job. A group above the budget `roadmap.stage.step_tokens` is a problem. A group that changes + a file that exists, and packs no file, is a problem too: the step starts with none of the + files that it changes. A group that only creates files packs none by nature, so it is no + problem. The plan holds no estimate of the lines of a created file, so a created file counts + one line.""" + texts = MESSAGES.roadmap + budget = load_settings().roadmap.stage.step_tokens + with checkout_at(source, head, operation=GIT_OPERATION) as tree: + files = { + path: (tree / path).read_text(encoding="utf-8", errors="replace") for path in paths + } + shared = shared_texts(title, body, files) + measured = measure_groups(tree, proposal.groups, shared=shared) + path = f"{root}/tasks.md" + problems: list[PlanProblem] = [] + for number, item in enumerate(measured, start=1): + title = item.group.title + if item.tokens > budget: + message = texts.plan_check_group_over_budget.format( + number=number, title=title, tokens=item.tokens, budget=budget + ) + problems.append(PlanProblem(path, "group-over-budget", message)) + if item.files == 0 and not set(item.group.changes) <= set(item.group.creates): + message = texts.plan_check_group_no_files.format(number=number, title=title) + problems.append(PlanProblem(path, "group-no-files", message)) + return problems + + def _validate_problems( tool: OpenSpecTool, source: Path, change: str, files: Mapping[str, str] ) -> list[PlanProblem]: @@ -417,25 +463,15 @@ def plan_check( ) ) - problems.extend( - _gate_problems( - proposal, - root, - head, - source, - base_rev, - _issue_body(reads, repo, issue), - ) - ) + planned = reads.get_issue(repo, issue) + body, _ = split_record(planned.body or "", issue=f"#{issue}") + problems.extend(_gate_problems(proposal, root, head, source, base_rev, body)) + problems.extend(_size_problems(proposal, root, head, source, planned.title, body, changed)) problems.extend(_validate_problems(tool, source, root.rsplit("/", 1)[-1], change_files)) problems.extend(_moved_problems(proposal, source, plan_rev, base_rev)) return PlanCheckResult(tuple(problems)) -def _issue_body(client: PlanReads, repo: str, issue: int) -> str: - return client.get_issue(repo, issue).body or "" - - def plan_check_requests(repo: str, number: int) -> tuple[PlannedRequest, ...]: """The reads a plan check makes, as a dry run lists them.""" texts = MESSAGES.roadmap diff --git a/src/devops_cli/roadmap/stage.py b/src/devops_cli/roadmap/stage.py index 508115751..d0fa87e77 100644 --- a/src/devops_cli/roadmap/stage.py +++ b/src/devops_cli/roadmap/stage.py @@ -150,6 +150,7 @@ pull_request_body, render_change, safe_text, + shared_texts, staging_title, write_change_files, ) @@ -1083,7 +1084,7 @@ def _build( ) -> tuple[dict[str, str], list[MeasuredGroup]]: files = render_change(target.change, proposal, self._known, created=self._today) item = target.item - shared = [f"{item.title}\n{item.body}", *files.values()] + shared = shared_texts(item.title, item.body, files) measured = measure_groups(self._source, proposal.groups, shared=shared) return files, measured diff --git a/src/devops_cli/roadmap/stage_change.py b/src/devops_cli/roadmap/stage_change.py index 4979e7b2e..490639e50 100644 --- a/src/devops_cli/roadmap/stage_change.py +++ b/src/devops_cli/roadmap/stage_change.py @@ -951,22 +951,34 @@ def safe_text(text: str) -> str: @dataclass(frozen=True) class MeasuredGroup: - """A task group and the tokens of the inputs of its step.""" + """A task group, the tokens of the inputs of its step, and the number of files that the step + packs for it (#3499).""" group: StageGroup tokens: int + files: int -def _read_tokens(root: Path, text: str) -> int: - """The tokens of what the step of the group `text` reads under `root`: the files that its - tasks name, each packed as the step packs it (`step_files`), and the rules of the checkout - that the step adds (`step_rules`). The step ignores a file that only the `Reads:` line lists, - and a name that holds no file is a file the step creates.""" - return sum( - sum(count_tokens(packed_file(path)) for path in named_files(root, group)) - + sum(count_tokens(section) for section in rules_sections(root, group)) - for group in parse_task_groups(text) - ) +def _reads(root: Path, text: str) -> tuple[int, int]: + """The tokens and the count of what the step of the group `text` reads under `root`: the files + that its tasks name, each packed as the step packs it (`step_files`), and the rules of the + checkout that the step adds (`step_rules`). The count is the count of files, and the tokens + include the rules. The step ignores a file that only the `Reads:` line lists, and a name that + holds no file is a file the step creates.""" + tokens = files = 0 + for group in parse_task_groups(text): + paths = named_files(root, group) + files += len(paths) + tokens += sum(count_tokens(packed_file(path)) for path in paths) + tokens += sum(count_tokens(section) for section in rules_sections(root, group)) + return tokens, files + + +def shared_texts(title: str, body: str, files: Mapping[str, str]) -> list[str]: + """The text that all groups of a change share in their measure: the item, and the files of the + change folder by path. The stage job and `plan-check` both build it here, so their figures for + one plan stay equal (#3499).""" + return [f"{title}\n{body}", *files.values()] def measure_groups( @@ -977,19 +989,21 @@ def measure_groups( ) -> list[MeasuredGroup]: """The measure of each group: the tokens of `shared` (the item and the files of the change folder), of the group's own text, of the files and the rules that its step reads under `root`, - and of the estimated lines of each file that it creates.""" + and of the estimated lines of each file that it creates. It also counts the files that the + step packs.""" shared_tokens = sum(count_tokens(text) for text in shared) - texts = [group_text(index, group) for index, group in enumerate(groups, start=1)] - return [ - MeasuredGroup( - group, + measured: list[MeasuredGroup] = [] + for index, group in enumerate(groups, start=1): + text = group_text(index, group) + read_tokens, files = _reads(root, text) + tokens = ( shared_tokens + count_tokens(text) - + _read_tokens(root, text) - + sum(group.creates_lines.values()) * CREATED_LINE_TOKENS, + + read_tokens + + sum(group.creates_lines.values()) * CREATED_LINE_TOKENS ) - for group, text in zip(groups, texts, strict=True) - ] + measured.append(MeasuredGroup(group, tokens, files)) + return measured def write_change_files(root: Path, files: Mapping[str, str]) -> None: diff --git a/src/devops_cli/roadmap/stage_eval_run.py b/src/devops_cli/roadmap/stage_eval_run.py index 0e243498c..e725d559a 100644 --- a/src/devops_cli/roadmap/stage_eval_run.py +++ b/src/devops_cli/roadmap/stage_eval_run.py @@ -14,10 +14,7 @@ from __future__ import annotations -import shutil -import tempfile -from collections.abc import Iterator, Sequence -from contextlib import contextmanager +from collections.abc import Sequence from dataclasses import dataclass from datetime import UTC, datetime from enum import StrEnum @@ -33,7 +30,7 @@ from devops_cli.exceptions import RoadmapStageError, ToolExecutionError from devops_cli.lang import MESSAGES from devops_cli.openspec.tool import OpenSpecTool -from devops_cli.roadmap.clone_git import git +from devops_cli.roadmap.clone_git import checkout_at from devops_cli.roadmap.refine_investigation import task_window from devops_cli.roadmap.stage import ( OverBudget, @@ -111,22 +108,6 @@ def item_of(case: StageCase) -> Item: ) -@contextmanager -def checkout_at(repository: Path, commit: str) -> Iterator[Path]: - """The tracked files of `commit` in a new detached worktree of `repository`, removed after. - A commit the repository lacks raises `GitOperationError`.""" - parent = Path(tempfile.mkdtemp(prefix="stage-eval-")) - tree = parent / "tree" - try: - git(repository, "worktree", "add", "--detach", str(tree), commit, operation=_OPERATION) - try: - yield tree - finally: - git(repository, "worktree", "remove", "--force", str(tree), operation=_OPERATION) - finally: - shutil.rmtree(parent, ignore_errors=True) - - def _first_reply_problems( case: StageCase, reply: StageProposal, source: Path ) -> dict[GateName, list[str]]: @@ -173,7 +154,7 @@ def run_case( does (#3037), and `client` answers the call that splits the groups. The recorder wraps the toolbox client, so it keeps the first plan the model gave. `openspec` reads the specs for the tool `read_spec`. Without a toolbox, a plan is one call.""" - with checkout_at(repository, case.base_commit) as source: + with checkout_at(repository, case.base_commit, operation=_OPERATION) as source: planner: StageClient = ( client if toolbox is None diff --git a/src/devops_cli/roadmap/stage_verify_eval.py b/src/devops_cli/roadmap/stage_verify_eval.py index 9294f8288..d273dd16e 100644 --- a/src/devops_cli/roadmap/stage_verify_eval.py +++ b/src/devops_cli/roadmap/stage_verify_eval.py @@ -22,12 +22,13 @@ from devops_cli.ai.client.models import AICredentialsError from devops_cli.dry_run import PlannedRequest from devops_cli.lang import MESSAGES +from devops_cli.roadmap.clone_git import checkout_at from devops_cli.roadmap.stage import StageClient from devops_cli.roadmap.stage_eval_cases import Defect, DefectType, Severity, StageCase, Verdict -from devops_cli.roadmap.stage_eval_run import checkout_at from devops_cli.roadmap.stage_verify import Finding, VerificationInput, returning, verify _DATASET_NAME: Final[str] = "stage_verification" +_OPERATION: Final[str] = "roadmap.eval.stage-verify" # The severities from the most serious, and the ones that the serious recall counts. _BY_SEVERITY: Final[tuple[Severity, ...]] = ( Severity.BLOCKER, @@ -166,7 +167,7 @@ def task(name: str) -> CaseVerification: raise rejected[0] case = by_name[name] try: - with checkout_at(repository, case.base_commit) as source: + with checkout_at(repository, case.base_commit, operation=_OPERATION) as source: given = VerificationInput.of( number=case.issue, title=case.title, diff --git a/tests/roadmap/test_roadmap_implement_job_trace.py b/tests/roadmap/test_roadmap_implement_job_trace.py index c3c0d46c1..9983f13bc 100644 --- a/tests/roadmap/test_roadmap_implement_job_trace.py +++ b/tests/roadmap/test_roadmap_implement_job_trace.py @@ -7,6 +7,7 @@ from __future__ import annotations +import json from typing import Any import pytest @@ -106,6 +107,45 @@ def test_a_new_run_clears_the_trace_of_the_last_run_before_its_first_step( assert STALE not in (held.job_record[ImplementRecordKey.TRACE] or "") +def _first_requests(world: JobWorld) -> dict[str, int]: + """The input tokens of the first request of each step, as the spend of the record holds them.""" + held = world.store.item(1) + assert held is not None + spend = json.loads(held.job_record[ImplementRecordKey.SPEND] or "{}") + return {row["step"]: row["first_request_tokens"] for row in spend["steps"]} + + +def test_the_spend_holds_the_size_of_the_first_request_of_each_step(job_world: JobWorld) -> None: + job_world.run(two_groups()) + + events = _trace(job_world).events + firsts = { + step: next( + e.fields["tokens_in"] + for e in events + if e.kind is TraceEventKind.MODEL and e.step == step + ) + for step in ("group 1", "group 2") + } + assert firsts["group 1"] > 0 + assert _first_requests(job_world) == firsts + + +def test_a_run_with_more_events_than_the_bound_keeps_the_size_of_its_first_requests( + job_world: JobWorld, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(implement_trace, "CONST_ROADMAP_IMPLEMENT_TRACE_CHARS", 700) + + job_world.run(two_groups()) + + trace = _trace(job_world) + firsts = _first_requests(job_world) + assert trace.dropped > 0 + assert not any(e.kind is TraceEventKind.MODEL and e.step == "group 1" for e in trace.events) + assert set(firsts) == {"group 1", "group 2"} + assert all(tokens > 0 for tokens in firsts.values()) + + def test_a_run_with_more_events_than_the_bound_leaves_the_newest_and_counts_the_rest( job_world: JobWorld, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/roadmap/test_roadmap_implement_step_trace.py b/tests/roadmap/test_roadmap_implement_step_trace.py index 00168b481..1cd3b8da8 100644 --- a/tests/roadmap/test_roadmap_implement_step_trace.py +++ b/tests/roadmap/test_roadmap_implement_step_trace.py @@ -95,6 +95,20 @@ def test_two_model_replies_give_two_model_events_with_their_tokens( assert {e.step for e in models} == {"group 1"} +def test_the_outcome_holds_the_input_tokens_of_the_first_model_request( + store: InMemoryRoadmapStore, checkout: Path +) -> None: + trace = ImplementTrace(step="group 1") + script = ModelScript(tool_call("read_file", path="src/sample.py"), say("Done.")) + + outcome = _step(store, checkout, script, trace) + + first, second = _of(trace, TraceEventKind.MODEL) + assert outcome.first_request_tokens == first.fields["tokens_in"] > 0 + assert outcome.first_request_tokens < outcome.input_tokens + assert second.fields["tokens_in"] > 0 + + def test_a_refused_command_and_a_retried_edit_are_tool_events( store: InMemoryRoadmapStore, checkout: Path ) -> None: diff --git a/tests/roadmap/test_roadmap_plan_check.py b/tests/roadmap/test_roadmap_plan_check.py index 8d54b2cfb..38bcdb9f1 100644 --- a/tests/roadmap/test_roadmap_plan_check.py +++ b/tests/roadmap/test_roadmap_plan_check.py @@ -12,21 +12,27 @@ import pytest +from devops_cli.config import settings from devops_cli.github.check_verdict import CheckBucket, CheckVerdictSummary, PRCheckItem from devops_cli.github.client import CommitRef, IssueView, PullRequest from devops_cli.lang import MESSAGES from devops_cli.openspec.tool import OpenSpecTool from devops_cli.roadmap.plan_check import PlanCheckResult, plan_check from devops_cli.roadmap.stage_change import ( + StageGroup, StageProposal, + measure_groups, render_change, + shared_texts, ) +from devops_cli.roadmap.store import encode_job_record, with_record from tests.support.builders import make_stage_proposal -from tests.support.roadmap_stage import VALID_REPORT, StageRepo +from tests.support.roadmap_stage import VALID_REPORT, StageRepo, heavy_group ISSUE = 1 PULL = 7 CHANGE = f"{ISSUE}-keep-the-last-comment" +ISSUE_TITLE = "An issue" CREATED = datetime(2026, 11, 4, tzinfo=UTC).date() TITLE = "docs(openspec): plan #1 Stop the job dropping a comment" BODY = "Plans #1\n\nThe job keeps every comment it reads.\n\nChange: `openspec/changes/1-keep-the-last-comment/`" @@ -56,7 +62,7 @@ def get_issue(self, repo: str, number: int) -> IssueView: del repo return IssueView( number=number, - title="An issue", + title=ISSUE_TITLE, state="open", body=self.bodies.get(number), ) @@ -75,14 +81,16 @@ def _checks( return CheckVerdictSummary(items=[item]) -def _facts(repo: StageRepo, *, sha: str, title: str = TITLE, body: str = BODY) -> _Reads: +def _facts( + repo: StageRepo, *, sha: str, title: str = TITLE, body: str = BODY, issue_body: str = "" +) -> _Reads: base = repo.git("rev-parse", "main") pull = PullRequest( title=title, head=CommitRef(sha=sha, ref="plan", repository="example/roadmap"), base=CommitRef(sha=base, ref="main", repository="example/roadmap"), ) - return _Reads(pull=pull, bodies={PULL: body, ISSUE: ""}) + return _Reads(pull=pull, bodies={PULL: body, ISSUE: issue_body}) def _push(repo: StageRepo, files: Mapping[str, str], *, branch: str = "plan") -> str: @@ -284,7 +292,9 @@ def test_a_plan_that_the_model_refuses_is_reported_as_a_read_problem( assert [(p.path, p.rule) for p in result.problems] == [("tasks.md", "read")] -def test_a_group_of_the_kind_other_reads_back(stage_repo: StageRepo) -> None: +def test_a_group_of_the_kind_other_that_only_creates_a_file_has_no_problem( + stage_repo: StageRepo, +) -> None: proposal = make_stage_proposal( skip_specs=True, specs=[], @@ -355,3 +365,143 @@ def test_an_openspec_problem_is_reported(stage_repo: StageRepo) -> None: result = _run(stage_repo, _facts(stage_repo, sha=sha), tool=_Tool(report)) assert [(p.path, p.rule) for p in result.problems] == [(CHANGE, "openspec")] + + +# ── The size of each group (#3499) ──────────────────────────────────────────── + +TASKS = f"openspec/changes/{CHANGE}/tasks.md" +MIN_BUDGET = 1000 + + +def _budget(monkeypatch: pytest.MonkeyPatch, tokens: int) -> None: + """Set the step budget of the stage job, as the Service sets it with its variable.""" + monkeypatch.setenv("DEVOPS_CLI_ROADMAP_STAGE_STEP_TOKENS", str(tokens)) + settings.reset_settings_cache() + + +def _counted(group: StageGroup | None = None) -> StageProposal: + """The sample plan with one line estimated for the file that it creates. The plan holds no + estimate, and the reader gives each created path one line, so the figure of the stage job for + such a plan is the figure that the command can reproduce.""" + held = group or make_stage_proposal().groups[0] + return make_stage_proposal( + groups=[held.model_copy(update={"creates_lines": {"tests/test_sample.py": 1}})] + ) + + +def _stage_figure( + repo: StageRepo, proposal: StageProposal, files: Mapping[str, str], body: str = "" +) -> int: + """The tokens that the stage job counts for the only group of `proposal`: it measures the + group on its clone, with the item and the files of the change folder as the shared text. The + body of the item holds no record block, as the store reads it.""" + (measured,) = measure_groups( + repo.clone, proposal.groups, shared=shared_texts(ISSUE_TITLE, body, files) + ) + return measured.tokens + + +def _recorded(body: str) -> str: + """The body of an issue as GitHub holds it: the text, then the record block that the jobs + write. The block holds many tokens, as the block of an issue that the jobs ran holds.""" + record = encode_job_record({"trace": "step " * 600}, {}) + return with_record(body, record) + + +def test_the_figure_of_a_group_is_the_figure_of_the_stage_job( + stage_repo: StageRepo, monkeypatch: pytest.MonkeyPatch +) -> None: + proposal = _counted(heavy_group(stage_repo, files=1)) + files = _change(proposal) + figure = _stage_figure(stage_repo, proposal, files) + sha = _push(stage_repo, files) + assert figure - 1 >= MIN_BUDGET + _budget(monkeypatch, figure - 1) + + result = _run(stage_repo, _facts(stage_repo, sha=sha)) + + (problem,) = result.problems + assert (problem.path, problem.rule) == (TASKS, "group-over-budget") + assert problem.message == MESSAGES.roadmap.plan_check_group_over_budget.format( + number=1, title=proposal.groups[0].title, tokens=figure, budget=figure - 1 + ) + + +def test_the_record_block_of_the_issue_is_not_in_the_figure( + stage_repo: StageRepo, monkeypatch: pytest.MonkeyPatch +) -> None: + text = "The job keeps every comment it reads." + proposal = _counted(heavy_group(stage_repo, files=1)) + files = _change(proposal) + figure = _stage_figure(stage_repo, proposal, files, text) + sha = _push(stage_repo, files) + _budget(monkeypatch, figure - 1) + + result = _run(stage_repo, _facts(stage_repo, sha=sha, issue_body=_recorded(text))) + + (problem,) = result.problems + assert problem.message == MESSAGES.roadmap.plan_check_group_over_budget.format( + number=1, title=proposal.groups[0].title, tokens=figure, budget=figure - 1 + ) + + +def test_a_group_at_the_budget_is_not_reported_when_the_issue_has_a_record_block( + stage_repo: StageRepo, monkeypatch: pytest.MonkeyPatch +) -> None: + text = "The job keeps every comment it reads." + proposal = _counted(heavy_group(stage_repo, files=1)) + files = _change(proposal) + figure = _stage_figure(stage_repo, proposal, files, text) + sha = _push(stage_repo, files) + _budget(monkeypatch, figure) + + result = _run(stage_repo, _facts(stage_repo, sha=sha, issue_body=_recorded(text))) + + assert result.problems == () + + +def test_a_group_at_the_budget_is_not_reported( + stage_repo: StageRepo, monkeypatch: pytest.MonkeyPatch +) -> None: + proposal = _counted(heavy_group(stage_repo, files=1)) + files = _change(proposal) + figure = _stage_figure(stage_repo, proposal, files) + sha = _push(stage_repo, files) + _budget(monkeypatch, figure) + + result = _run(stage_repo, _facts(stage_repo, sha=sha)) + + assert result.problems == () + + +def test_a_group_with_large_files_is_reported_over_the_budget( + stage_repo: StageRepo, monkeypatch: pytest.MonkeyPatch +) -> None: + proposal = _counted(heavy_group(stage_repo, files=2)) + sha = _push(stage_repo, _change(proposal)) + _budget(monkeypatch, MIN_BUDGET) + + result = _run(stage_repo, _facts(stage_repo, sha=sha)) + + (problem,) = result.problems + assert problem.rule == "group-over-budget" + assert f"{MIN_BUDGET} tokens" in problem.message + + +def test_a_group_that_packs_no_file_is_reported(stage_repo: StageRepo) -> None: + group = make_stage_proposal().groups[0] + unnamed = group.model_copy( + update={ + "test": "Write the test of the last comment", + "implementation": "Keep the last comment", + } + ) + sha = _push(stage_repo, _change(make_stage_proposal(groups=[unnamed]))) + + result = _run(stage_repo, _facts(stage_repo, sha=sha)) + + (problem,) = result.problems + assert (problem.path, problem.rule) == (TASKS, "group-no-files") + assert problem.message == MESSAGES.roadmap.plan_check_group_no_files.format( + number=1, title=group.title + ) diff --git a/tests/roadmap/test_roadmap_stage_eval_run.py b/tests/roadmap/test_roadmap_stage_eval_run.py index a44691f2f..e12a89ac7 100644 --- a/tests/roadmap/test_roadmap_stage_eval_run.py +++ b/tests/roadmap/test_roadmap_stage_eval_run.py @@ -18,6 +18,7 @@ ) from devops_cli.config.constants import CONST_ROADMAP_STAGE_TOOL_MAX_CALLS from devops_cli.exceptions import GitOperationError +from devops_cli.roadmap.clone_git import checkout_at from devops_cli.roadmap.stage import StagedChange, plan_item from devops_cli.roadmap.stage_change import StageProposal from devops_cli.roadmap.stage_eval_cases import ( @@ -32,7 +33,6 @@ CaseRun, Outcome, RecordingClient, - checkout_at, item_of, run_case, run_cases, @@ -261,7 +261,7 @@ def test_a_commit_that_the_repository_lacks_fails_and_leaves_no_worktree_and_no_ with ( pytest.raises(GitOperationError, match="worktree"), - checkout_at(stage_repo.clone, "0" * 40), + checkout_at(stage_repo.clone, "0" * 40, operation="test"), ): pass @@ -275,7 +275,7 @@ def test_a_checkout_is_the_tracked_files_of_the_commit_and_is_removed( sha = stage_repo.git("rev-parse", "HEAD") stage_repo.track({"later.txt": "later"}) - with checkout_at(stage_repo.clone, sha) as tree: + with checkout_at(stage_repo.clone, sha, operation="test") as tree: names = {path.name for path in tree.rglob("*") if path.is_file()} assert "sample.py" in names assert "later.txt" not in names