Skip to content

Commit ee5b77b

Browse files
committed
fix skill
1 parent eabd751 commit ee5b77b

10 files changed

Lines changed: 181 additions & 20 deletions

File tree

.claude/skills/add-model/SKILL.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@ Read these first to understand the patterns:
6767
- **CLI creation:** `problemreductions-cli/src/commands/create.rs`
6868
- **Canonical model examples:** `src/example_db/model_builders.rs`
6969

70+
## Pre-review Checklist
71+
72+
Before implementing, make sure the plan explicitly covers these items that structural review checks later:
73+
- `ProblemSchemaEntry` metadata is complete for the current schema shape (`display_name`, `aliases`, `dimensions`, and constructor-facing `fields`)
74+
- `declare_variants!` is present with the correct `opt`/`sat` marker and exactly one `default` variant when multiple concrete variants exist
75+
- CLI discovery and `pred create <ProblemName>` support are included where applicable
76+
- A canonical model example is registered for example-db / `pred create --example`
77+
- `docs/paper/reductions.typ` adds both the display-name dictionary entry and the `problem-def(...)`
78+
- `src/unit_tests/trait_consistency.rs` is updated
79+
7080
## Step 1: Determine the category
7181

7282
Choose the appropriate sub-module under `src/models/`:
@@ -107,6 +117,7 @@ Create `src/models/<category>/<name>.rs`:
107117
```
108118

109119
Key decisions:
120+
- **Schema metadata:** `ProblemSchemaEntry` must reflect the current registry schema shape, including `display_name`, `aliases`, `dimensions`, and constructor-facing `fields`
110121
- **Optimization problems:** `type Metric = SolutionSize<W::Sum>`, implement `OptimizationProblem` with `direction()`
111122
- **Satisfaction problems:** `type Metric = bool`, implement `SatisfactionProblem` (marker trait)
112123
- **Weight management:** use inherent methods (`weights()`, `set_weights()`, `is_weighted()`), NOT traits

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,18 @@ EOF
223223
python3 scripts/pipeline_pr.py comment --repo "$REPO" --pr "$PR" --body-file "$COMMENT_FILE"
224224
rm -f "$COMMENT_FILE"
225225

226+
# Repo verification may regenerate tracked exports (notably after `make paper`).
227+
# Inspect the tree once more before pushing.
228+
git status --short
229+
230+
# If expected generated exports changed, stage them before push.
231+
git add docs/src/reductions/problem_schemas.json docs/src/reductions/reduction_graph.json
232+
233+
# The issue plan file must be gone before push.
234+
test ! -e docs/plans/<plan-file>.md
235+
226236
git push
227-
make copilot-review
237+
gh copilot-review "$PR"
228238
```
229239

230240
#### 7e. Done
@@ -280,3 +290,5 @@ Run /review-pipeline to process Copilot comments, fix CI, and run agentic tests.
280290
| Dirty working tree | Use `pipeline_worktree.py prepare-issue-branch` — it stops before branching if the worktree is dirty |
281291
| Bundling model + rule in one PR | Each PR must contain exactly one model or one rule — STOP and block if model is missing (Step 3.5) |
282292
| Plan files left in PR | Delete plan files before final push (Step 7c) |
293+
| `make paper` or export steps changed tracked JSON after verification | Run `git status --short`, stage expected generated exports, and STOP if unexpected files remain before push |
294+
| `make copilot-review` cannot find a resumed PR branch | Request review with `gh copilot-review "$PR"` so resumed local branch names do not matter |

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,17 @@ WORKTREE_DIR=$(printf '%s\n' "$WORKTREE" | python3 -c "import sys,json; print(js
147147
cd "$WORKTREE_DIR"
148148
```
149149

150+
If `worktree-for-issue` fails during the GraphQL-backed open-PR lookup, fall back to a REST search instead of creating a fresh branch blindly:
151+
152+
```bash
153+
PR_JSON=$(gh api "search/issues?q=repo:$REPO+is:pr+is:open+%22Fix+%23$ISSUE%22")
154+
PR_NUMBER=$(printf '%s\n' "$PR_JSON" | python3 -c "import sys,json; items=json.load(sys.stdin).get('items', []); print(items[0]['number'] if items else '')")
155+
WORKTREE=$(python3 scripts/pipeline_worktree.py checkout-pr \
156+
--repo "$REPO" --pr "$PR_NUMBER" --repo-root . --format json)
157+
```
158+
150159
- `action == "resume-pr"`: existing PR checked out — `issue-to-pr` will skip plan creation and jump to execution
160+
- If the resumed branch later proves stale against `main` (for example, merge conflicts span registry/CLI/skill wiring or a merge helper reports `likely_complex: true`), STOP autonomous repair and move the issue to `Final review` for human triage instead of forcing a large migration inside project-pipeline
151161
- `action == "create-worktree"`: fresh branch from `origin/main`
152162

153163
All subsequent steps run inside the worktree. This ensures the user's main checkout is never modified.
@@ -246,3 +256,4 @@ Completed: 2/4 | Review pool: 3 | Returned to Ready: 1
246256
| Working in main checkout | All work happens in `.worktrees/` — never modify the main checkout |
247257
| Missing items from project board | `gh project item-list` defaults to 30 items — always use `--limit 500` |
248258
| Creating a fresh branch when PR exists | Check `issue-context` action field first — use `checkout-pr` for existing PRs instead of `create-issue` |
259+
| Forcing a very stale resume PR through autonomous conflict repair | If a resumed PR has broad merge conflicts (`likely_complex: true`, registry/CLI/skill churn, etc.), stop and move it to `Final review` |

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ The report is the Step 0 packet. It should already include:
9999
- Comment Summary
100100
- CI / Coverage
101101
- Merge Prep
102+
- PR head branch
102103
- Linked Issue Context
103104

104105
Branch from the report:
@@ -118,7 +119,7 @@ The bundle already handled the mechanical claim step:
118119
- the selected reviewed PR is claimed through the review queue
119120
- explicit `--pr` matches on ambiguous cards are treated as deterministic disambiguation and claimed automatically
120121

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.
122+
When you need to take actions later, use the identifiers already printed in the report (`Board item`, `PR`, worktree path, `PR head branch`). 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.
122123

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

@@ -132,7 +133,7 @@ All subsequent steps run inside the prepared worktree and should read facts from
132133

133134
Read the merge result from the report's `Merge Prep` section.
134135

135-
- If the report says the merge status is `clean`: push the merge commit and continue.
136+
- If the report says the merge status is `clean`: push the merge commit to the packet's `PR head branch` and continue.
136137
- If there are conflicts:
137138
1. Inspect the conflicting files listed in the report.
138139
2. Compare the current skill versions on main vs the PR branch to understand which patterns are current.
@@ -146,11 +147,13 @@ Use the report as the primary mechanical context:
146147
- `Comment Summary`
147148
- `CI / Coverage`
148149
- `Linked Issue Context`
150+
- `Merge Prep` (`PR head branch`)
149151

150152
Inspect the report's Copilot comment count and linked issue context. If there are actionable comments: invoke `/fix-pr` to address them, then push:
151153

152154
```bash
153-
git push
155+
HEAD_REF_NAME=<from the review-pipeline packet>
156+
git push origin HEAD:"$HEAD_REF_NAME"
154157
```
155158

156159
If Copilot approved with no actionable comments: skip to next step.
@@ -196,7 +199,9 @@ Run agentic feature tests on the modified feature:
196199
- `[Model]` PRs: the new problem model name
197200
- `[Rule]` PRs: the new reduction rule (source -> target)
198201

199-
2. **Invoke `/agentic-tests:test-feature`** with the identified feature. This simulates a downstream user exercising the feature from docs and examples. You MUST use the Skill tool to invoke `agentic-tests:test-feature`.
202+
2. **Invoke `/agentic-tests:test-feature`** with the identified feature. This simulates a downstream user exercising the feature from docs and examples.
203+
- In environments with a Skill tool, use that tool.
204+
- In Codex, open `~/.claude/commands/agentic-tests:test-feature.md` directly and follow it instead of assuming slash-command support.
200205

201206
3. **If test-feature reports issues:** treat every reported issue as real until you have checked it in the **current PR worktree/branch**.
202207
- Reproduce each issue from the current PR branch/worktree before acting. If it does not reproduce there, classify it as `not reproducible in current worktree`.
@@ -238,7 +243,8 @@ print(mapping.get(state, 'FAILED'))
238243

239244
3. **Push fixes:**
240245
```bash
241-
git push
246+
HEAD_REF_NAME=<from the review-pipeline packet>
247+
git push origin HEAD:"$HEAD_REF_NAME"
242248
```
243249

244250
4. Increment retry counter. If `< 3`, go back to step 1. If `= 3`, give up.

scripts/pipeline_board.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1232,7 +1232,10 @@ def parse_args(argv: list[str]) -> argparse.Namespace:
12321232
ack_parser.add_argument("item_id")
12331233

12341234
list_parser = subparsers.add_parser("list")
1235-
list_parser.add_argument("mode", choices=["ready", "in-progress", "review-pool", "review"])
1235+
list_parser.add_argument(
1236+
"mode",
1237+
choices=["ready", "in-progress", "review-pool", "review", "final-review"],
1238+
)
12361239
list_parser.add_argument("--repo")
12371240
list_parser.add_argument("--owner", default="CodingThrust")
12381241
list_parser.add_argument("--project-number", type=int, default=8)
@@ -1338,6 +1341,13 @@ def main(argv: list[str] | None = None) -> int:
13381341
batch_pr_fetcher=batch_fetch_prs_with_reviews,
13391342
)
13401343
return print_candidate_list(args.mode, items, fmt=args.format)
1344+
if args.mode == "final-review":
1345+
items = status_items(
1346+
board_data,
1347+
STATUS_FINAL_REVIEW,
1348+
content_types={"Issue", "PullRequest"},
1349+
)
1350+
return print_candidate_list(args.mode, items, fmt=args.format)
13411351
raise SystemExit(f"Unsupported list mode: {args.mode}")
13421352

13431353
if args.mode in {"review", "final-review"} and not args.repo:

scripts/pipeline_checks.py

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -587,19 +587,42 @@ def fetch_issue(repo: str, issue_number: int) -> dict:
587587
)
588588

589589

590+
def fetch_existing_prs_via_search(repo: str, issue_number: int) -> list[dict]:
591+
query = f'repo:{repo} is:pr is:open "Fix #{issue_number}"'
592+
data = run_gh_json("api", "search/issues", "-f", f"q={query}")
593+
items = data.get("items", [])
594+
prs: list[dict] = []
595+
for item in items:
596+
number = item.get("number")
597+
if number is None or "pull_request" not in item:
598+
continue
599+
pr = run_gh_json("api", f"repos/{repo}/pulls/{number}")
600+
prs.append(
601+
{
602+
"number": pr.get("number"),
603+
"headRefName": (pr.get("head") or {}).get("ref"),
604+
"url": pr.get("html_url"),
605+
}
606+
)
607+
return prs
608+
609+
590610
def fetch_existing_prs(repo: str, issue_number: int) -> list[dict]:
591-
data = run_gh_json(
592-
"pr",
593-
"list",
594-
"--repo",
595-
repo,
596-
"--state",
597-
"open",
598-
"--search",
599-
f"Fix #{issue_number}",
600-
"--json",
601-
"number,headRefName,url",
602-
)
611+
try:
612+
data = run_gh_json(
613+
"pr",
614+
"list",
615+
"--repo",
616+
repo,
617+
"--state",
618+
"open",
619+
"--search",
620+
f"Fix #{issue_number}",
621+
"--json",
622+
"number,headRefName,url",
623+
)
624+
except subprocess.CalledProcessError:
625+
return fetch_existing_prs_via_search(repo, issue_number)
603626
if not isinstance(data, list):
604627
raise ValueError(f"Unexpected PR list payload for issue #{issue_number}: {data!r}")
605628
return data

scripts/pipeline_skill_context.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ def render_review_pipeline_text(result: dict) -> str:
225225
)
226226
if checkout.get("worktree_dir"):
227227
lines.append(f"- Worktree: `{checkout['worktree_dir']}`")
228+
if checkout.get("head_ref_name"):
229+
lines.append(f"- PR head branch: `{checkout['head_ref_name']}`")
228230
conflicts = merge.get("conflicts") or []
229231
if conflicts:
230232
lines.append("- Conflicts:")

scripts/test_pipeline_board.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,46 @@ def fake_pr_state_fetcher(repo: str, pr_number: int) -> str:
210210
)
211211
self.assertIsNone(no_item)
212212

213+
def test_parse_args_accepts_final_review_list_mode(self) -> None:
214+
import pipeline_board
215+
216+
args = pipeline_board.parse_args(["list", "final-review", "--repo", "CodingThrust/problem-reductions"])
217+
218+
self.assertEqual(args.command, "list")
219+
self.assertEqual(args.mode, "final-review")
220+
221+
def test_main_lists_final_review_items(self) -> None:
222+
import pipeline_board
223+
224+
board_data = {
225+
"items": [make_issue_item("PVTI_30", 239, status="Final review", title="[Model] BalancedCompleteBipartiteSubgraph")]
226+
}
227+
228+
with (
229+
patch.object(pipeline_board, "fetch_board_items", return_value=board_data) as fetch_board_items,
230+
patch.object(pipeline_board, "print_candidate_list", return_value=0) as print_candidate_list,
231+
):
232+
rc = pipeline_board.main(
233+
["list", "final-review", "--repo", "CodingThrust/problem-reductions", "--format", "json"]
234+
)
235+
236+
self.assertEqual(rc, 0)
237+
fetch_board_items.assert_called_once()
238+
print_candidate_list.assert_called_once_with(
239+
"final-review",
240+
[
241+
{
242+
"number": 239,
243+
"issue_number": 239,
244+
"pr_number": None,
245+
"status": "Final review",
246+
"title": "[Model] BalancedCompleteBipartiteSubgraph",
247+
"item_id": "PVTI_30",
248+
}
249+
],
250+
fmt="json",
251+
)
252+
213253

214254
class ReviewCandidateQueueTests(unittest.TestCase):
215255
def test_select_entry_from_entries_tracks_pending_until_ack(self) -> None:

scripts/test_pipeline_checks.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/usr/bin/env python3
2+
import subprocess
23
import tempfile
34
import unittest
5+
from unittest import mock
46
from pathlib import Path
57

68
import pipeline_checks
@@ -16,6 +18,46 @@
1618

1719

1820
class PipelineChecksTests(unittest.TestCase):
21+
@mock.patch("pipeline_checks.run_gh_json")
22+
def test_fetch_existing_prs_falls_back_to_rest_search_on_pr_list_failure(
23+
self,
24+
run_gh_json: mock.Mock,
25+
) -> None:
26+
run_gh_json.side_effect = [
27+
subprocess.CalledProcessError(1, ["gh", "pr", "list"]),
28+
{
29+
"items": [
30+
{
31+
"number": 223,
32+
"pull_request": {
33+
"url": "https://api.github.com/repos/CodingThrust/problem-reductions/pulls/223"
34+
},
35+
}
36+
]
37+
},
38+
{
39+
"number": 223,
40+
"head": {"ref": "issue-212-multiprocessor-scheduling"},
41+
"html_url": "https://example.test/pull/223",
42+
},
43+
]
44+
45+
prs = pipeline_checks.fetch_existing_prs(
46+
"CodingThrust/problem-reductions",
47+
212,
48+
)
49+
50+
self.assertEqual(
51+
prs,
52+
[
53+
{
54+
"number": 223,
55+
"headRefName": "issue-212-multiprocessor-scheduling",
56+
"url": "https://example.test/pull/223",
57+
}
58+
],
59+
)
60+
1961
def test_detect_scope_reports_model_review_for_new_model_file(self) -> None:
2062
scope = detect_scope_from_paths(
2163
added_files=["src/models/graph/graph_partitioning.rs"],

scripts/test_pipeline_skill_context.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,10 @@ def test_emit_result_prints_review_pipeline_text_report(self) -> None:
178178
},
179179
"prep": {
180180
"ready": False,
181-
"checkout": {"worktree_dir": "/tmp/review-pr-570"},
181+
"checkout": {
182+
"worktree_dir": "/tmp/review-pr-570",
183+
"head_ref_name": "issue-117-graph-partitioning",
184+
},
182185
"merge": {"status": "conflicted", "conflicts": ["src/models/graph_partitioning.rs"]},
183186
},
184187
}
@@ -197,6 +200,7 @@ def test_emit_result_prints_review_pipeline_text_report(self) -> None:
197200
self.assertIn("- CI state: failure", rendered)
198201
self.assertIn("## Merge Prep", rendered)
199202
self.assertIn("- Worktree: `/tmp/review-pr-570`", rendered)
203+
self.assertIn("- PR head branch: `issue-117-graph-partitioning`", rendered)
200204
self.assertIn("## Linked Issue Context", rendered)
201205

202206
def test_emit_result_prints_review_implementation_text_report(self) -> None:

0 commit comments

Comments
 (0)