Merge remote-tracking branch 'origin/main' into review-pr-192-issue-122-steiner-tree

Author: GiggleLiu

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

@@ -29,6 +29,26 @@ Before any implementation, collect all required information. If called from `iss
 
 If any item is missing, ask the user to provide it. Do NOT proceed until the checklist is complete.
 
+### Associated Rule Check
+
+Before implementation, verify that at least one reduction rule exists or is planned for this problem — otherwise it will be an orphan node in the reduction graph.
+
+**Check both directions:**
+
+1. **Outbound (this issue → rule issues):** Look for rule issue numbers in the model issue's "Reduction Rule Crossref" section.
+2. **Inbound (rule issues → this problem):** Search open rule issues that reference this problem as source or target:
+   ```bash
+   gh issue list --label rule --state open --limit 500 --json number,title | \
+     jq '[.[] | select(.title | test("<ProblemName>"; "i"))]'
+   ```
+
+**If no associated rules are found:**
+- Warn the user: "This model has no associated rule issues. It will be an orphan node in the reduction graph and will be flagged during review."
+- Ask whether to proceed anyway or file a companion rule issue first (via `/propose rule`).
+- If proceeding, add a visible `<!-- WARNING: orphan model — no associated rule issue -->` comment in the PR description.
+
+**If associated rules are found:** List them and continue.
+
 ## Reference Implementations
 
 Read these first to understand the patterns:

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

@@ -58,6 +58,32 @@ Collect all information needed for the review:
 
 1f. **Check for conflicts with main**: Run `gh pr view <number> --json mergeable`. If there are merge conflicts, launch a subagent to merge `origin/main` into the PR branch (in a worktree) and push the merge commit.
 
+1g. **PR / issue comment audit (REQUIRED)**: Final review must check the comment history before recommending merge.
+  - Set `REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner)`
+  - Fetch and read:
+    - PR conversation comments: `gh api repos/$REPO/issues/<number>/comments`
+    - PR inline review comments: `gh api repos/$REPO/pulls/<number>/comments`
+    - PR review bodies: `gh api repos/$REPO/pulls/<number>/reviews`
+    - linked issue comments, if an issue exists
+  - Build a list of every actionable comment and classify each as:
+    - `addressed`
+    - `superseded / no longer applicable`
+    - `still open`
+  - Pay special attention to the `## Review Pipeline Report` comment. If it contains a `Remaining issues for final review` section, those items must be reviewed explicitly here.
+  - Do **not** recommend merge until every actionable comment has been dispositioned.
+
+1h. **Comment status summary**: Prepare a short summary for later steps:
+
+> **Comment Audit**
+>
+> [N addressed, M superseded, K still open]
+>
+> Open items:
+> - [comment / issue summary]
+> - ...
+
+If no actionable comments remain, report `No open actionable comments`.
+
 ### Step 2: Usefulness assessment
 
 Think critically about whether this model/rule is genuinely useful. Consider:
@@ -102,6 +128,43 @@ Use `AskUserQuestion` to confirm:
 > - "I see an issue" — reviewer describes the problem
 > - "Skip" — skip this check
 
+### Step 3b: File whitelist check
+
+Check that the PR only touches files expected for its type. Any file outside the whitelist is flagged for review — it may be a legacy pattern or an unrelated change.
+
+**Whitelist for [Model] PRs:**
+- `src/models/<category>/<name>.rs` — model implementation
+- `src/unit_tests/models/<category>/<name>.rs` — unit tests
+- `src/example_db/model_builders.rs` — canonical example registration
+- `src/example_db/rule_builders.rs` — only if updating nonempty-style assertions
+- `docs/paper/reductions.typ` — paper entry
+- `docs/src/reductions/problem_schemas.json` — schema export
+- `docs/src/reductions/reduction_graph.json` — graph export
+- `tests/suites/trait_consistency.rs` — trait consistency entry
+- `problemreductions-cli/tests/cli_tests.rs` — CLI integration tests for `pred create`
+
+**Whitelist for [Rule] PRs:**
+- `src/rules/<source>_<target>.rs` — reduction implementation
+- `src/rules/mod.rs` — module registration
+- `src/unit_tests/rules/<source>_<target>.rs` — unit tests
+- `src/example_db/rule_builders.rs` — canonical example registration
+- `src/models/<category>/<name>.rs` — only if adding getters needed for overhead expressions
+- `docs/paper/reductions.typ` — paper entry
+- `docs/src/reductions/reduction_graph.json` — graph export
+- `docs/src/reductions/problem_schemas.json` — only if updating field descriptions
+- `problemreductions-cli/tests/cli_tests.rs` — CLI integration tests if adding CLI support
+
+If any file falls outside these whitelists, flag it:
+
+> **File Whitelist Check**
+>
+> Found N file(s) outside expected whitelist:
+> - `path/to/file` — [what it does, why it may not belong]
+>
+> These should be reviewed — they may follow a deprecated pattern or be unrelated to this PR.
+
+If all files are whitelisted, report "All files within expected whitelist" and continue.
+
 ### Step 4: Completeness check
 
 Verify the PR includes all required components. Check:
@@ -124,6 +187,13 @@ Verify the PR includes all required components. Check:
 - [ ] Canonical rule example in `src/example_db/rule_builders.rs`
 - [ ] Paper section in `docs/paper/reductions.typ` (`reduction-rule` entry)
 
+**Paper-example consistency check (both Model and Rule PRs):**
+
+The paper example must use data from the generated JSON (`docs/paper/examples/generated/`), not hand-written data. To verify:
+1. Run `make examples` on the PR branch to regenerate `docs/paper/examples/generated/models.json` and `rules.json`.
+2. For **[Rule] PRs**: the paper's `reduction-rule` entry must call `load-example(source, target)` (defined in `reductions.typ`) to load the canonical example from `rules.json`, and derive all concrete values from the loaded data using Typst array operations — no hand-written instance data.
+3. For **[Model] PRs**: read the problem's entry in `models.json` and compare its `instance` field against the paper's `problem-def` example. The paper example must use the same instance (allowing 0-indexed JSON vs 1-indexed math notation). If they differ, flag: "Paper example does not match `example_db` canonical instance in `models.json`."
+
 Report missing items:
 
 > **Completeness Check**
@@ -162,32 +232,40 @@ Present to reviewer:
 > Strengths:
 > - [bullet points]
 >
-> Weaknesses:
-> - [bullet points]
+> Weaknesses (numbered):
+> 1. [issue description — file:line if applicable]
+> 2. [issue description — file:line if applicable]
+> ...
 >
 > Comparable to: [name a similar-quality existing model/rule for reference]
 
 ### Step 6: Final decision
 
-Summarize all findings and ask the reviewer for a decision.
+Summarize all findings and present the numbered issues as selectable options.
 
 Present a summary table:
 
 | Aspect | Result |
 |--------|--------|
+| Comments | [All addressed / Open: X, Y] |
 | Usefulness | [Useful/Marginal/Not useful] |
 | Safety | [Safe/Concerns found] |
 | Completeness | [Complete/Missing: X, Y] |
 | Quality | [N%] |
 | PR URL | [link] |
 
-Use `AskUserQuestion`:
+Then present all numbered issues from Step 5 as a multi-select `AskUserQuestion`:
+
+> **Which issues should be fixed before merging?** (select all that apply, or "Merge as-is")
+> - "Merge as-is" — no fixes needed
+> - "Fix 1: [short description]" — [one-line summary]
+> - "Fix 2: [short description]" — [one-line summary]
+> - ...
+> - "OnHold" — move to OnHold column with a reason
+
+This lets the reviewer cherry-pick exactly which issues to fix. If the reviewer selects fixes, proceed to Step 7 Quick fix. If "Merge as-is", proceed to Step 7 Merge.
 
-> **What would you like to do with this PR?**
-> - "Merge" — approve and show merge link for browser
-> - "OnHold" — move to OnHold column with a reason comment
-> - "Quick fix" — fix specific issues before merging (describe what to fix)
-> - "Reject" — close the PR with explanation
+If any actionable PR / issue comment from Step 1g is still open, `Merge as-is` must **not** be your recommendation. Recommend either **Quick fix** or **OnHold** instead.
 
 ### Step 7: Execute decision
 
@@ -208,7 +286,7 @@ Use `AskUserQuestion`:
    ```
 
 **If Quick fix:**
-1. Ask the reviewer what needs fixing (use `AskUserQuestion`).
+1. Apply only the fixes the reviewer selected in Step 6.
 2. Checkout the PR branch in a worktree, apply fixes, commit, push.
 3. After push, go back to Step 6 to re-confirm the decision.