Skip to content

Commit 27a73bf

Browse files
committed
polish workflow
1 parent 73ece9a commit 27a73bf

12 files changed

Lines changed: 395 additions & 90 deletions

File tree

.claude/CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ These repo-local skills live under `.claude/skills/*/SKILL.md`.
3030
- In Codex, read the relevant `SKILL.md` directly and follow it; do not assume slash-command support exists.
3131
- The Makefile targets `run-plan`, `run-issue`, `run-pipeline`, and `run-review` already translate these workflows into explicit `SKILL.md` prompts for Codex.
3232
- The default Codex model in the Makefile is `gpt-5.4`. Override it with `CODEX_MODEL=<model>` if needed.
33+
- The Step 0/Step 1 packet builders under `scripts/pipeline_skill_context.py` and `scripts/pipeline_checks.py` are expensive GitHub-backed calls. Per top-level skill invocation, generate each packet at most once and reuse the resulting text/JSON for all later steps unless the skill explicitly requires a fresh rerun.
3334

3435
## Commands
3536
```bash

.claude/skills/final-review/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ GitHub Project board IDs (for `gh project item-edit`):
3131
### Step 0: Generate the Final-Review Report
3232

3333
Step 0 should be a single report-generation step. Do not manually unpack board selection, PR metadata, merge prep, or deterministic checks with shell snippets.
34+
The expensive full-context call here is `python3 scripts/pipeline_skill_context.py final-review ...` (backed by `build_final_review_context()`). It is allowed exactly once per top-level `final-review` invocation. After it succeeds, reuse the packet for the rest of the review and do not rerun Step 0 just to fetch the same context in another format.
3435

3536
```bash
3637
REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner)
@@ -59,7 +60,7 @@ Branch from the report:
5960
- `Bundle status: ready` => continue normally (check warnings — a self-review warning means the reviewer is the PR author; flag it but do not block)
6061
- `Bundle status: ready-with-warnings` => continue only with the narrow warning fallback described in the report
6162

62-
When you need to take actions later, use the identifiers already printed in the report (`Board item`, `PR`, URL). If you absolutely need raw structured data for a corner case, rerun the same command with `--format json`, but do not rebuild Step 0 manually.
63+
When you need to take actions later, use the identifiers already printed in the report (`Board item`, `PR`, URL). If you need structured data for a corner case, derive it from the existing packet whenever possible instead of rerunning Step 0.
6364

6465
### Step 1: Push the Merge with Main
6566

.claude/skills/issue-to-pr/SKILL.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ ISSUE_JSON=$(python3 scripts/pipeline_checks.py issue-context \
4949
--format json)
5050
```
5151

52+
This `issue-context` packet is the expensive deterministic preflight call for `issue-to-pr`. It is allowed exactly once per top-level `issue-to-pr` invocation. After it succeeds, reuse `ISSUE_JSON` for all later guards, resume/create decisions, and summaries instead of calling `issue-context` again.
53+
5254
Treat `ISSUE_JSON` as the source of truth for the deterministic preflight data:
5355
- `title`, `body`, `labels`, and `comments` provide the issue summary and comment thread
5456
- `kind`, `source_problem`, and `target_problem` provide parsed issue metadata

.claude/skills/project-pipeline/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ This skill runs **fully autonomously** — no confirmation prompts, no user ques
3939
### 0. Generate the Project-Pipeline Report
4040

4141
Step 0 should be a single report-generation step. Do not manually list Ready items, list In-progress items, grep model declarations, or re-derive blocked rules with separate shell commands.
42+
The expensive full-context call here is `python3 scripts/pipeline_skill_context.py project-pipeline ...` (backed by `build_project_pipeline_context()`). For a single top-level `project-pipeline` invocation, call it once and reuse the packet for scoring, ranking, and choosing the issue. Do not rerun it in the single-issue path after the packet exists.
4243

4344
```bash
4445
set -- python3 scripts/pipeline_skill_context.py project-pipeline --repo CodingThrust/problem-reductions --repo-root . --format text
@@ -117,7 +118,7 @@ The report should already have stopped you before this point if the requested is
117118

118119
After successful validation, extract `ITEM_ID`, `ISSUE`, and `TITLE` from `CLAIM` using the same commands shown below.
119120

120-
**If `--all`:** proceed with all eligible issues in ranked order (highest score first). Models before Rules at same score. Blocked rules are skipped. After each issue is processed, regenerate the report before the next claim, because a just-merged Model may unblock pending rules.
121+
**If `--all`:** proceed with all eligible issues in ranked order (highest score first). Models before Rules at same score. Blocked rules are skipped. After each issue is processed, regenerate the report before the next claim, because a just-merged Model may unblock pending rules. This is the only normal case in this skill where a second full-context packet is expected.
121122

122123
**Otherwise (no args):** score the eligible issues from the report, pick the highest-scored one, and proceed immediately (no confirmation). After picking the issue number, claim it through the scripted bundle:
123124

.claude/skills/review-implementation/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Dispatches two parallel review subagents with fresh context (no implementation h
1919
## Step 1: Generate the Review-Implementation Report
2020

2121
Step 1 should be a single report-generation step. Do not manually rebuild git range detection, `review-context`, current PR lookup, or linked-issue loading with separate shell snippets.
22+
The expensive full-context call here is `python3 scripts/pipeline_skill_context.py review-implementation ...` (backed by `build_review_implementation_context()`). It is allowed exactly once per top-level `review-implementation` invocation. After it succeeds, reuse that packet for both subagents and the consolidated report.
2223

2324
```bash
2425
set -- python3 scripts/pipeline_skill_context.py review-implementation --repo-root . --format text
@@ -41,7 +42,7 @@ The report is the Step 1 packet. It should already include:
4142
- Current PR
4243
- Linked Issue Context
4344

44-
Use the report as the default source of truth for the rest of this skill. If you need structured data for a corner case, rerun the same command with `--format json`, but do not rebuild Step 1 manually.
45+
Use the report as the default source of truth for the rest of this skill. If you need structured data for a corner case, derive it from the existing packet whenever possible instead of rerunning Step 1.
4546

4647
## Step 2: Prepare Subagent Context
4748

.claude/skills/review-pipeline/SKILL.md

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
---
22
name: review-pipeline
3-
description: Pick a PR from the Review pool board column, fix Copilot review comments, check issue/human comments, fix CI, run agentic feature tests, then move to Final review
3+
description: Use when triaging PRs from the Review pool board column and moving them toward Final review or back to Ready
44
---
55

66
# Review Pipeline
77

8-
Pick PRs from the `Review pool` column on the [GitHub Project board](https://github.com/orgs/CodingThrust/projects/8/views/1). For each PR: wait for Copilot review, fix Copilot comments, check and address issue/human comments, fix CI, run agentic feature tests, then move to `Final review`.
8+
Pick PRs from the `Review pool` column on the [GitHub Project board](https://github.com/orgs/CodingThrust/projects/8/views/1). For each item: first decide whether the PR is actually review-ready. If Copilot has not reviewed it yet, inspect the PR body and current diff to understand why. Incomplete PRs go back to `Ready` with a remark. Review-ready PRs without Copilot review metadata get a Copilot review request, then you switch to another item. PRs that already show Copilot review metadata continue through the full review flow, where the actual comment text is fetched later.
99

1010
## Invocation
1111

@@ -34,21 +34,61 @@ GitHub Project board IDs (for `gh project item-edit`):
3434

3535
## Autonomous Mode
3636

37-
This skill runs **fully autonomously** except for one case: if the scripted `review-pipeline` context bundle returns `status == "needs-user-choice"`, STOP and ask the user which PR is the intended target.
37+
This skill runs **fully autonomously** except for one case: if a Review pool card links multiple repo PRs and the intended target is unclear, STOP and ask the user which PR is the intended target.
3838

3939
## Steps
4040

41-
### 0. Generate the Review-Pipeline Report
41+
### 0a. Triage Review Pool Cheaply
4242

43-
Step 0 should be a single report-generation step. Do not manually unpack board selection, worktree prep, or PR context with shell snippets.
43+
Before spending the expensive full-context packet, do one lightweight Review-pool scan:
4444

4545
```bash
4646
REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner)
47+
QUEUE=$(python3 scripts/pipeline_board.py list review --repo "$REPO" --format json)
48+
```
49+
50+
If `PR` was explicitly supplied (for example `/review-pipeline 570`), do **not** pick a different item from the queue. Triage only that PR. The "switch to another item" behavior in this step is for untargeted `/review-pipeline` runs.
51+
52+
Pick one candidate with a lightweight heuristic. Good enough is:
53+
- prefer direct PR cards over issue cards
54+
- prefer `eligible` items over `waiting-for-copilot`
55+
- otherwise any open PR is fine; randomness is acceptable
56+
57+
For the chosen PR:
58+
- If it already shows Copilot review metadata (`eligibility == eligible`), set `PR` to that PR number and continue to Step 0b.
59+
- If it is `waiting-for-copilot`, check out the PR and inspect the current diff plus PR body to decide why Copilot has not reviewed it yet.
60+
61+
When a PR is `waiting-for-copilot`, inspect the checked-out worktree, the PR body, and the current diff. Use the actual code delta as the source of truth; do not assume the PR is review-ready just because it is in `Review pool`.
62+
63+
Decision rule for `waiting-for-copilot` PRs:
64+
- If the PR is not complete enough for review, keep the PR open, post a short remark explaining what is still missing, and move the project item back to `Ready`.
65+
For untargeted runs, then return to Step 0a to pick another item.
66+
For explicit `PR` runs, STOP after reporting what you moved back.
67+
- If the PR looks complete enough for review, run `gh copilot-review <PR_NUMBER>` and leave it in `Review pool`.
68+
For untargeted runs, then return to Step 0a to pick another item. Do **not** wait on that PR in the same invocation.
69+
For explicit `PR` runs, STOP after reporting that Copilot review was requested.
70+
71+
For "not complete enough", rely on the checked-out diff and PR body together. Typical reasons include: the PR body promises work that is not in the diff, the diff is still mostly scaffolding or partial plumbing, key files or tests are missing, or the implementation is obviously not ready for a meaningful review pass.
72+
73+
Use a concrete remark when sending the item back to `Ready`, for example:
74+
75+
```bash
76+
gh pr comment <PR_NUMBER> --body "This PR is not review-ready yet. I checked the current diff and PR body and it still looks incomplete for review: <brief concrete reason>. Moving the linked board item back to Ready so implementation can continue."
77+
python3 scripts/pipeline_board.py move <ITEM_ID> ready
78+
```
79+
80+
If no candidate in the current queue is both open and ready for a full review pass after this triage, STOP with `No Review pool PRs are currently ready for full review-pipeline processing.`
81+
82+
### 0b. Generate the Review-Pipeline Report for the Selected Reviewed PR
83+
84+
Only after Step 0a has identified a reviewed, review-ready PR should you spend the expensive full-context packet. Step 0b should be a single report-generation step for that selected PR. Do not manually unpack board selection, worktree prep, or PR context with shell snippets.
85+
The expensive full-context call here is `python3 scripts/pipeline_skill_context.py review-pipeline ...` (backed by `build_review_pipeline_context()`). It is allowed exactly once per top-level `review-pipeline` invocation. After it succeeds, do not rerun this command or lower-level selection helpers such as `fetch_review_candidates`, `claim_review_entry`, or `pipeline_board.review_candidates` in the same invocation unless no packet was produced at all or the user explicitly asks for a fresh snapshot.
86+
Step 0a only tells you whether Copilot has reviewed the PR at all. Step 0b is where you fetch the full comment summary and actual review context for the selected PR.
87+
88+
```bash
4789
STATE_FILE=/tmp/problemreductions-review-selection.json
48-
set -- python3 scripts/pipeline_skill_context.py review-pipeline --repo "$REPO" --state-file "$STATE_FILE" --format text
49-
if [ -n "${PR:-}" ]; then
50-
set -- "$@" --pr "$PR"
51-
fi
90+
# Step 0a should already have selected the PR number.
91+
set -- python3 scripts/pipeline_skill_context.py review-pipeline --repo "$REPO" --pr "$PR" --state-file "$STATE_FILE" --format text
5292
REPORT=$("$@")
5393
printf '%s\n' "$REPORT"
5494
```
@@ -62,7 +102,7 @@ The report is the Step 0 packet. It should already include:
62102
- Linked Issue Context
63103

64104
Branch from the report:
65-
- `Bundle status: empty` => STOP with `No Review pool PRs are currently eligible for review-pipeline.`
105+
- `Bundle status: empty` => the selected PR is no longer eligible; for untargeted runs return to Step 0a and pick another item, and for explicit `PR` runs STOP and report that the targeted PR is not currently eligible
66106
- `Bundle status: needs-user-choice` => STOP and ask the user which PR is intended
67107
- `Bundle status: ready` => continue with the already-claimed item and prepared worktree
68108

@@ -75,10 +115,10 @@ Review pool card links multiple repo PRs:
75115
```
76116

77117
The bundle already handled the mechanical claim step:
78-
- normal eligible PRs are claimed through the review queue
118+
- the selected reviewed PR is claimed through the review queue
79119
- explicit `--pr` matches on ambiguous cards are treated as deterministic disambiguation and claimed automatically
80120

81-
When you need to take actions later, use the identifiers already printed in the report (`Board item`, `PR`, worktree path). If you absolutely need raw structured data for a corner case, rerun the same command with `--format json`, but do not rebuild Step 0 manually.
121+
When you need to take actions later, use the identifiers already printed in the report (`Board item`, `PR`, worktree path). If you need structured data, prefer parsing the existing packet or ensure Step 0 was generated as JSON once; do not rerun Step 0 just to switch formats inside the same invocation.
82122

83123
All subsequent steps run inside the prepared worktree and should read facts from the report instead of re-fetching them by default.
84124

@@ -301,11 +341,12 @@ Completed: 2/2 | All moved to Final review
301341
| PR not in Review pool column | Verify status before processing; STOP if not Review pool |
302342
| Processing a closed PR from a stale issue card | Require PR state `OPEN`; skip stale closed PRs |
303343
| Guessing on an issue card with multiple linked repo PRs | Stop, show options to the user, and recommend the most likely correct OPEN PR |
304-
| Picking a PR before Copilot has reviewed | Check `pulls/$PR/reviews` for copilot-pull-request-reviewer[bot]; if absent, request with `gh copilot-review <PR>` and wait |
344+
| Picking a PR before Copilot has reviewed | Inspect the checked-out diff and PR body first. If the PR is incomplete, comment and move it back to Ready. If it is review-ready, request Copilot review and switch to another item instead of waiting |
305345
| Missing project scopes | Run `gh auth refresh -s read:project,project` |
306346
| Skipping review-implementation | Always run structural completeness check in Step 2b — it catches gaps Copilot misses (paper entries, CLI registration, trait_consistency) |
307347
| Skipping agentic tests | Always run test-feature even if CI is green |
308348
| Not checking out the right branch | Use `gh pr view` to get the exact branch name |
349+
| Waiting idle for Copilot | Request the review, leave the PR in Review pool, and keep triaging other items in the same run |
309350
| Worktree left behind on failure | Always clean up with `git worktree remove` in Step 5 |
310351
| Working in main checkout | All work happens in `.worktrees/` — never modify the main checkout |
311352
| Skipping merge with main | Always merge origin/main in Step 1a to catch conflicts before fixing comments |

scripts/make_helpers.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,8 @@ watch_and_dispatch() {
243243
repo=${4-}
244244
interval=${POLL_INTERVAL:-600}
245245

246-
state_file=$(mktemp /tmp/problemreductions-${mode}-state.XXXXXX)
247-
board_cache="/tmp/problemreductions-${mode}-board-cache.json"
248-
trap 'rm -f "$state_file" "$board_cache"' EXIT INT TERM
246+
state_file=${STATE_FILE:-/tmp/problemreductions-${mode}-forever-state.json}
247+
board_cache="/tmp/problemreductions-${mode}-forever-board-cache.json"
249248

250249
echo "Watching for new ${label} (polling every $((interval / 60))m)..."
251250
while true; do
@@ -265,6 +264,8 @@ watch_and_dispatch() {
265264
echo "$(date '+%Y-%m-%d %H:%M:%S') New ${label}: item $number ($item_id)"
266265
if ${MAKE:-make} "$make_target" N="$number"; then
267266
ack_polled_item "$state_file" "$item_id" || exit $?
267+
echo "$(date '+%Y-%m-%d %H:%M:%S') Processed ${label} item $number; sleeping $((interval / 60))m..."
268+
sleep "$interval"
268269
else
269270
dispatch_status=$?
270271
echo "$(date '+%Y-%m-%d %H:%M:%S') Dispatch failed for ${label} item $number; will retry after sleep." >&2

0 commit comments

Comments
 (0)