Skip to content

Commit 83618c4

Browse files
GiggleLiuclaude
andcommitted
Improve review pipeline: whitelist scoping, CI paper check, complexity reporting
- Fix whitelist false positives: use origin/main (not local main) as merge-base in review-implementation, so after merge-with-main the diff only shows PR files - Add make paper to CI (install typst, run after tests) to catch paper regressions on main - Fix pre-existing paper error: SequencingToMinimizeWeightedCompletionTime used removed x.optimal API - Extract and report complexity strings in model completeness check so reviewers can compare against issue specification - Document canonical_model_example_specs() requirement in add-model skill Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3618171 commit 83618c4

6 files changed

Lines changed: 50 additions & 9 deletions

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,11 @@ Update `problemreductions-cli/src/commands/create.rs` so `pred create <ProblemNa
186186

187187
Add a builder function in `src/example_db/model_builders.rs` that constructs a small, canonical instance for this model. Register it in `build_model_examples()`.
188188

189+
Also add `canonical_model_example_specs()` **in the model file itself** (gated by `#[cfg(feature = "example-db")]`), and register it in the category `mod.rs` example chain (e.g., `specs.extend(<module>::canonical_model_example_specs());`). See any existing model in `src/models/graph/` for the pattern.
190+
189191
This example is now the canonical source for:
190192
- `pred create --example <PROBLEM_SPEC>`
191-
- paper/example exports
193+
- paper/example exports via `load-model-example()` in `reductions.typ`
192194
- example-db invariants tested in `src/unit_tests/example_db.rs`
193195

194196
## Step 5: Write unit tests

.claude/skills/fix-issue/SKILL.md

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@ Fix errors and warnings from a `check-issue` report. Auto-fixes mechanical issue
1010
## Invocation
1111

1212
```
13-
/fix-issue <model|rule>
13+
/fix-issue <model|rule|issue-number>
1414
```
1515

16+
- `/fix-issue model` or `/fix-issue rule` — pick next from Backlog
17+
- `/fix-issue 207` — fix a specific issue by number (skip Step 1a/1b, go directly to 1c)
18+
1619
## Constants
1720

1821
GitHub Project board IDs:
@@ -59,7 +62,10 @@ digraph fix_issue {
5962

6063
## Step 1: Pick Next Issue from Backlog
6164

62-
The argument is `model` or `rule` — determines which issue type (`[Model]` or `[Rule]`) to process.
65+
The argument is `model`, `rule`, or a specific issue number.
66+
67+
- If a **number** is given, skip to Step 1c with that issue.
68+
- If `model` or `rule`, pick from the Backlog as below.
6369

6470
### 1a: Fetch candidate list from project board
6571

@@ -83,6 +89,8 @@ Returns all Backlog issues of the requested type, sorted by `Good` label first t
8389

8490
Pick the first item from the list. If the list is empty, STOP with message: "No `[Model]`/`[Rule]` issues in Backlog."
8591

92+
If the top issue already has the `Good` label and its check report has **0 failures and 0 warnings**, skip to Step 8 (just move it to Ready — no edits needed). If it has warnings, proceed normally.
93+
8694
### 1c: Fetch the chosen issue
8795

8896
```bash
@@ -136,8 +144,8 @@ Tag each issue as:
136144
| Missing type dependencies | Architectural decision about codebase |
137145
| Incorrect mathematical claims | Domain expertise needed |
138146
| Incomplete reduction algorithm | Core technical content |
139-
| Incomplete or trivial example | Needs meaningful design, provide 3 options for the human to choose from |
140-
| Decision vs optimization framing | Check associated `[Rule]` issues first — if a rule targets the decision version, implement that; if it targets optimization, implement that; if both exist, split into two separate model issues. Problem modeling choice |
147+
| Incomplete or trivial example | Present **3 concrete example options** with pros/cons (use `AskUserQuestion` with previews showing vertex/edge counts, optimal values, and suboptimal cases). Prefer examples that match the model issue's example when a companion model exists. |
148+
| Decision vs optimization framing | **Default to optimization** unless evidence points otherwise. The project prefers `OptimizationProblem` (like MIS, SpinGlass, TSP) because optimization subsumes decision. Check associated `[Rule]` issues (`gh issue list --search "<ProblemName> in:title label:rule"`) to see how rules use the model — if rules only need the decision version (e.g., reducing to SAT with a bound), optimization still works since you can extract the bound from the optimal value. Only use `SatisfactionProblem` for inherently decision/feasibility problems (SAT, KColoring) where there is no natural optimization objective. If switching to optimization, add the appropriate `Minimum`/`Maximum` prefix per codebase conventions. |
141149
| Ambiguous overhead expressions | Requires understanding the reduction |
142150

143151
---
@@ -232,14 +240,28 @@ Apply the requested changes to the draft issue body, re-check locally (Step 6),
232240

233241
Only reached when the human approves. Now push everything to GitHub.
234242

235-
### 8a: Edit the issue body
243+
### 8a: Edit the issue body and title
236244

237245
Use the Write tool to save the updated body to `/tmp/fix_issue_body.md`, then:
238246

239247
```bash
240248
gh issue edit <NUMBER> --body-file /tmp/fix_issue_body.md
241249
```
242250

251+
If the problem name was changed (e.g., renamed to add `Minimum`/`Maximum` prefix), also update the issue **title**:
252+
253+
```bash
254+
gh issue edit <NUMBER> --title "[Model] NewProblemName"
255+
```
256+
257+
Then find and update **all related issues** that reference the old name in their title:
258+
259+
```bash
260+
gh issue list --search "OldName in:title" --state open --json number,title
261+
# For each related issue, update the title:
262+
gh issue edit <RELATED_NUMBER> --title "<updated title>"
263+
```
264+
243265
### 8b: Comment on the issue with a changelog
244266

245267
Post a comment summarizing what was changed, so reviewers can see the diff at a glance:
@@ -295,3 +317,4 @@ Done! Issue #<NUMBER>:
295317
| Closing the issue | Never close. Labels and board status only |
296318
| Force-pushing or modifying git | This skill only edits GitHub issues via `gh`. No git operations |
297319
| Inventing `pipeline_board.py` subcommands | Only `next`, `claim-next`, `ack`, `list`, `move`, `backlog` exist |
320+
| Forgetting to update the issue title | If the problem name changed, update the title with `gh issue edit <N> --title "..."` and find all related issues referencing the old name |

.github/workflows/ci.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ jobs:
4242
- name: Run doc tests
4343
run: cargo test --doc --features ilp-highs --verbose
4444

45+
- name: Install typst
46+
uses: typst-community/setup-typst@v4
47+
48+
- name: Build paper
49+
run: make paper
50+
4551
# Code coverage
4652
coverage:
4753
name: Code Coverage

docs/paper/reductions.typ

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3723,7 +3723,7 @@ A classical NP-complete problem from Garey and Johnson @garey1979[Ch.~3, p.~76],
37233723
let weights = x.instance.weights
37243724
let precs = x.instance.precedences
37253725
let ntasks = lengths.len()
3726-
let sol = x.optimal.at(0)
3726+
let sol = (config: x.optimal_config, metric: x.optimal_value)
37273727
let opt = sol.metric.Valid
37283728
let lehmer = sol.config
37293729
let schedule = {

scripts/pipeline_checks.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,15 @@ def check_entry(
183183
}
184184

185185

186+
def _extract_complexity_strings(model_text: str) -> str:
187+
"""Extract complexity strings from declare_variants! for issue comparison."""
188+
matches = re.findall(r'=>\s*"([^"]+)"', model_text)
189+
if matches:
190+
unique = list(dict.fromkeys(matches))
191+
return "complexity: " + "; ".join(unique)
192+
return "complexity: not found"
193+
194+
186195
def model_completeness(repo_root: Path, name: str) -> dict:
187196
file_stem = camel_to_snake(name)
188197
model_file = find_model_file(repo_root, file_stem)
@@ -206,7 +215,8 @@ def model_completeness(repo_root: Path, name: str) -> dict:
206215
else check_entry(status="fail", detail="missing ProblemSchemaEntry for model")
207216
),
208217
"declare_variants": (
209-
check_entry(status="pass", path=str(model_file.relative_to(repo_root)))
218+
check_entry(status="pass", path=str(model_file.relative_to(repo_root)),
219+
detail=_extract_complexity_strings(model_text))
210220
if model_file is not None
211221
and "crate::declare_variants!" in model_text
212222
and re.search(r"\b(?:default\s+)?(?:opt|sat)\b", model_text)

scripts/pipeline_skill_context.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,7 @@ def build_review_implementation_context(
812812
review_context_builder: Callable[..., dict] | None = None,
813813
) -> dict:
814814
merge_base_getter = merge_base_getter or (
815-
lambda repo_root: git_text_in(repo_root, "merge-base", "main", "HEAD").strip()
815+
lambda repo_root: git_text_in(repo_root, "merge-base", "origin/main", "HEAD").strip()
816816
)
817817
head_sha_getter = head_sha_getter or (
818818
lambda repo_root: git_text_in(repo_root, "rev-parse", "HEAD").strip()

0 commit comments

Comments
 (0)