Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
58 changes: 39 additions & 19 deletions .claude/CLAUDE.md

Large diffs are not rendered by default.

47 changes: 30 additions & 17 deletions .claude/skills/add-model/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ Read these first to understand the patterns:
- **Model tests:** `src/unit_tests/models/graph/maximum_independent_set.rs`
- **Trait definitions / aggregate types:** `src/traits.rs` (`Problem`), `src/types.rs` (`Aggregate`, `Max`, `Min`, `Sum`, `Or`, `And`, `Extremum`)
- **Registry dispatch boundary:** `src/registry/mod.rs`, `src/registry/variant.rs`
- **CLI aliases:** `problemreductions-cli/src/problem_name.rs`
- **CLI creation:** `problemreductions-cli/src/commands/create.rs`
- **CLI and MCP construction:** discovered from the model's registry entry; no frontend model-name dispatch
- **Canonical model examples:** `src/example_db/model_builders.rs`

## Pre-review Checklist

Before implementing, make sure the plan explicitly covers these items that structural review checks later:
- `ProblemSchemaEntry` metadata is complete for the current schema shape (`display_name`, `aliases`, `dimensions`, and constructor-facing `fields`)
- Derive numeric implementation types from the mathematical domains in the issue and follow `docs/src/design.md#numeric-types-and-arithmetic`; serde/CLI construction uses the same validation as `new`/`try_new`, and boundary tests cover the supported maximum without requiring impractical allocation
- `ProblemSchemaEntry` metadata is complete (`display_name`, `aliases`, `dimensions`, explicit `category`, and construction `fields`)
- `Problem::Value` uses the correct aggregate wrapper and witness support is intentional
- `declare_variants!` is present with exactly one `default` variant when multiple concrete variants exist
- CLI discovery and `pred create <ProblemName>` support are included where applicable
Expand All @@ -92,6 +92,8 @@ Choose the appropriate sub-module under `src/models/`:
- `algebraic/` -- matrices, linear systems, lattices (QUBO, ILP, CVP, BMF)
- `misc/` -- unique input structures that don't fit other categories (BinPacking, PaintShop, Factoring)

Declare the same structural choice explicitly in `ProblemSchemaEntry.category`. This is required metadata and is never inferred from `module_path!()` or the file location.

## Step 1.5: Infer problem size getters

From the **best known exact algorithm** complexity (item 9), infer what problem size getter methods the struct should expose. The variables used in the complexity expression define the natural size metrics.
Expand Down Expand Up @@ -122,7 +124,7 @@ Create `src/models/<category>/<name>.rs`:
```

Key decisions:
- **Schema metadata:** `ProblemSchemaEntry` must reflect the current registry schema shape, including `display_name`, `aliases`, `dimensions`, and constructor-facing `fields`
- **Schema metadata:** `ProblemSchemaEntry` must include the explicit structural `category` and reflect the construction interface through `display_name`, `aliases`, `dimensions`, and `fields`
- **Objective problems:** use `type Value = Max<_>`, `Min<_>`, or `Extremum<_>` when the model should expose optimization-style witness helpers
- **Witness problems:** use `type Value = Or` for existential feasibility problems
- **Aggregate-only problems:** use a value-only aggregate such as `Sum<_>`, `And`, or a custom `Aggregate` when witnesses are not meaningful
Expand Down Expand Up @@ -168,22 +170,32 @@ The CLI now loads, serializes, and brute-force solves problems through the core
1. **Registry-backed dispatch comes from `declare_variants!`:**
- Make sure every concrete variant you want the CLI to load is listed in `declare_variants!`
- Mark the intended default variant with `default` when applicable
- Declare well-established problem aliases in `ProblemSchemaEntry.aliases` and variant-specific aliases in `declare_variants!`; CLI and MCP discover both from the registry

## Step 4.5: Add construction support

CLI and MCP construction are registry-driven. Do not edit either frontend to recognize a model name.

1. If user-facing inputs exactly match persisted JSON fields, do nothing. The ordinary `declare_variants!` entry uses `ProblemSchemaEntry.fields` as required construction inputs and deserializes the model directly.

2. If construction has derived fields, renamed inputs, defaults depending on other inputs, or a composite value assembled from multiple inputs, define a model-local DTO with `#[derive(Deserialize, CreateSpec)]`. Its named fields are the complete public construction contract. Use `Option<T>` only for genuinely optional inputs, doc comments for help text, and `#[create(codec = "...")]` when the transport syntax cannot be inferred from the Rust type. Set `ProblemSchemaEntry.fields` to `LocalCreateSpec::FIELDS` so the catalog and executable constructor share the derived metadata.

2. **`problemreductions-cli/src/problem_name.rs`:**
- Add a lowercase alias mapping in `resolve_alias()` (e.g., `"newproblem" => "NewProblem".to_string()`)
- Only add short aliases to the `ALIASES` array if the abbreviation is **well-established in the literature** (e.g., MIS, MVC, SAT, TSP, CVP are standard; "KS" for Knapsack or "BP" for BinPacking are NOT — do not invent new abbreviations)
3. Implement `TryFrom<LocalCreateSpec> for Model`. Validate before calling constructors that assert or panic, return a descriptive error, compute derived state there, and build the canonical model value.

## Step 4.5: Add CLI creation support
4. Register the spec on each applicable variant: `default Model => "..." create LocalCreateSpec`. Both frontends then discover the inputs automatically and serialize the constructed typed model back to canonical persisted JSON.

CLI creation is **schema-driven** — `pred create <ProblemName>` automatically maps `ProblemSchemaEntry` fields to CLI flags via `snake_case → kebab-case` convention. No match arm in `create.rs` is needed.
5. A new reusable external syntax may add one transport codec. It must dispatch by codec/type, never by canonical model name. Unknown or missing inputs are rejected by the core construction contract.

1. **Ensure CLI flags exist** in `problemreductions-cli/src/cli.rs` (`CreateArgs` struct) for each field in your `ProblemSchemaEntry`. The flag name must match the field name via `snake_case → kebab-case` (e.g., field `edge_weights` → flag `--edge-weights`). If a flag already exists with the right name, you're done.
### Optional random generation

2. **Add new CLI flags** only if the problem needs flags not already present. Add them to `CreateArgs` and update `all_data_flags_empty()` accordingly. Also add entries to the `flag_map()` method on `CreateArgs`.
Random generation is an optional model capability, not a model-completeness requirement. Many models do not have a natural or useful probability distribution over instances; leave random generation unregistered for those models. Do not invent arbitrary size limits, value ranges, or distributions merely to make `--random` available.

3. **Add type parser support** if the field uses a type not yet handled by `parse_field_value()` in `create.rs`. Check the existing type dispatch table — most standard types (`Vec<i32>`, `Vec<usize>`, `Vec<(usize, usize)>`, graph types, etc.) are already covered. Only add a new parser for genuinely new types.
When the model does have a well-defined generator with a concrete testing or example use, random generation is registry-driven and belongs beside the model. Do not edit CLI or MCP dispatch code.

4. **Schema alignment**: The `ProblemSchemaEntry` fields should list **constructor parameters** (what the user provides), not internal derived fields. For example, if `m` and `n` are derived from a matrix, only list `matrix` and `k` in the schema. Field names must match the struct field names exactly (used for JSON serialization and CLI flag mapping).
1. Define a typed random input DTO with `#[derive(Deserialize, CreateSpec)]`, or reuse a matching shared spec from `crate::random`.
2. Implement `RandomGenerate` with `crate::impl_random_generate!(ConcreteModel, RandomSpec, |spec| { ... })`. Validate values and return `Result`; do not round, clamp, or silently replace invalid inputs.
3. Add `random` only to the exact `declare_variants!` entries that implement the trait: `default Model => "..." create LocalCreateSpec random`.
4. The generated problem must have the same canonical name and variant as the selected registry entry. Use the concrete variant's actual graph and numeric types instead of attaching requested metadata to a different concrete instance.

## Step 4.6: Add canonical model example to example_db

Expand Down Expand Up @@ -303,19 +315,20 @@ Structural and quality review is handled by the `review-pipeline` stage, not her
|---------|-----|
| Implementing weight management as a trait | Use inherent methods: `weights()`, `set_weights()`, `is_weighted()` |
| Forgetting `inventory::submit!` | Every problem needs a `ProblemSchemaEntry` registration |
| Omitting or inferring the model category | Set the required `ProblemSchemaEntry.category` explicitly to one of `Algebraic`, `Formula`, `Graph`, `Misc`, or `Set`; never parse `module_path!()`. |
| Missing `#[path]` test link | Add `#[cfg(test)] #[path = "..."] mod tests;` at file bottom |
| Wrong `dims()` | Must match the actual configuration space (e.g., `vec![2; n]` for binary) |
| Using the wrong aggregate wrapper | Objective models use `Max` / `Min` / `Extremum`, witness models use `bool`, aggregate-only models use a fold value like `Sum` / `And` |
| Not registering in `mod.rs` | Must update both `<category>/mod.rs` and `models/mod.rs` |
| Forgetting `declare_variants!` | Required for variant complexity metadata and registry-backed load/serialize/solve dispatch |
| Wrong aggregate wrapper | Use `Max` / `Min` / `Extremum` for objective problems, `Or` for existential witness problems, and `Sum` / `And` (or a custom aggregate) for value-only folds |
| Wrong `declare_variants!` syntax | Entries no longer use `opt` / `sat`; one entry per problem may be marked `default` |
| Forgetting CLI alias | Must add lowercase entry in `problem_name.rs` `resolve_alias()` |
| Adding aliases in CLI code | Declare problem aliases in `ProblemSchemaEntry.aliases` and variant aliases in `declare_variants!` |
| Adding a hand-written decision model | Use `Decision<P>` wrapper instead — see `decision_problem_meta!` + `register_decision_variant!` in `src/models/graph/minimum_vertex_cover.rs` for the pattern |
| Inventing short aliases | Only use well-established literature abbreviations (MIS, SAT, TSP); do NOT invent new ones |
| Forgetting CLI flags | Schema-driven create needs matching CLI flags in `CreateArgs` for each `ProblemSchemaEntry` field (snake_case → kebab-case). Also add to `flag_map()`. |
| Missing type parser | If the problem uses a new field type, add a handler in `parse_field_value()` in `create.rs` |
| Schema lists derived fields | Schema should list constructor params, not internal fields (e.g., `matrix, k` not `matrix, m, n, k`) |
| Adding frontend model-name branches | Construction is model-owned. Use a local `CreateSpec` and register it with `declare_variants!`; CLI and MCP must discover it. |
| Hand-maintaining custom construction fields twice | Derive `CreateSpec`, use `LocalCreateSpec::FIELDS` in `ProblemSchemaEntry`, and register the same type in `declare_variants!`. |
| Calling a panicking constructor from `TryFrom<CreateSpec>` | Validate the spec first and return a descriptive conversion error. |
| Missing canonical model example | Add a builder in `src/example_db/model_builders.rs` and keep it aligned with paper/example workflows |
| Paper example not tested | Must include `test_<name>_paper_example` that verifies the exact instance, solution, and solution count shown in the paper |
| Claiming direct ILP solving but leaving `<Problem> -> ILP` for later | If the issue promises a direct ILP path, implement that rule in the same PR with exact overhead metadata and production-level ILP tests |
45 changes: 33 additions & 12 deletions .claude/skills/add-rule/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ grep "type Value = " src/models/*/<source_file>.rs src/models/*/<target_file>.rs

If incompatible, STOP and comment on the issue explaining the type mismatch and options. Do NOT proceed.

## Numeric Safety Gate

Read `docs/src/design.md#numeric-types-and-arithmetic`. Derive implementation
types, supported ranges, and checked conversions from the mathematical source,
target, and reduction algorithm. Ask the contributor only when a mathematical
domain or constraint is ambiguous; do not ask them to choose Rust types. Do not
use `as` for range/sign changes. Check target-size arithmetic and auxiliary
identifiers before constructing the target, verify serde/CLI uses the same
ranges, and add focused boundary tests.

## Reference Implementations

Read these first to understand the patterns:
Expand Down Expand Up @@ -106,13 +116,19 @@ impl ReductionResult for ReductionXToY {
type Source = SourceType;
type Target = TargetType;
fn target_problem(&self) -> &Self::Target { &self.target }
fn extract_solution(&self, target_solution: &[usize]) -> Vec<usize> {
// Map target solution back to source solution
// If Step 1 ran: translate the verified Python extract_solution() logic
fn extract_solution(
&self,
target_solution: &[usize],
) -> crate::rules::ExtractionResult<Vec<usize>> {
crate::rules::traits::validate_target_solution(self.target_problem(), target_solution)?;
let source_solution = /* translate the verified mathematical mapping exactly */;
Ok(source_solution)
}
}
```

Every direct extractor must call `validate_target_solution()` once before decoding. It checks only length and value domains, not feasibility, optimality, or rule-specific structure; reject malformed structure with `ExtractionError`.

**ReduceTo with `#[reduction]` macro** (overhead is **required**):
```rust
#[reduction(overhead = {
Expand Down Expand Up @@ -156,16 +172,18 @@ Additional recommended tests:
- Edge cases (empty graph, single vertex, etc.)
- Weight preservation (if applicable)

Test every malformed representation distinguished by the decoder (for example, zero or multiple one-hot selections, or duplicate permutation entries). The canonical example supplies shared wrong-length and out-of-domain tests.

For aggregate-only reductions, replace the closed-loop witness test with value-chain tests:
- Solve the target with `Solver::solve()`
- Map the aggregate value back with `extract_value()`
- If testing a path, use `ReductionGraph::reduce_aggregate_along_path(...)`

Link via `#[cfg(test)] #[path = "..."] mod tests;` at the bottom of the rule file.

## Step 5: Add canonical example to example_db
## Step 5: Add canonical example

Add a builder function in `src/example_db/rule_builders.rs` that constructs a small, canonical instance for this reduction. Follow the existing patterns in that file. Register the builder in `build_rule_examples()`.
Define `canonical_rule_example_specs()` in the rule module and include it from `src/rules/mod.rs::canonical_rule_example_specs()`. This enrolls the rule in shared round-trip, wrong-length, and out-of-domain extraction tests.

## Step 6: Document in paper (MANDATORY — DO NOT SKIP)

Expand Down Expand Up @@ -231,11 +249,11 @@ Checklist: notation self-contained, complexity cited, overhead consistent, examp
```bash
cargo run --example export_graph # Generate reduction_graph.json for docs/paper builds
cargo run --example export_schemas # Generate problem schemas for docs/paper builds
make regenerate-fixtures # Regenerate example_db/fixtures/examples.json (slow, needs ILP)
cargo run --features "example-db" --example export_examples
make test clippy # Must pass
```

`make regenerate-fixtures` is required so the paper can load the new rule's example data from `src/example_db/fixtures/examples.json`. Without it, the `reduction-rule` entry in Step 6 will reference missing fixture data.
`export_examples` refreshes the gitignored `docs/paper/data/examples.json` used by the paper.

Structural and quality review is handled by the `review-pipeline` stage, not here. The run stage just needs to produce working code.

Expand All @@ -249,7 +267,9 @@ Structural and quality review is handled by the `review-pipeline` stage, not her

## CLI Impact

Adding a witness-preserving reduction rule does NOT require CLI changes -- the reduction graph is auto-generated from `#[reduction]` macros and the CLI discovers paths dynamically. However, both source and target models must already be fully registered through their model files (`declare_variants!`), aliases as needed in `problem_name.rs`, and `pred create` support where applicable (see `add-model` skill).
Adding a witness-preserving reduction rule does NOT require CLI changes -- the reduction graph is auto-generated from `#[reduction]` macros and the CLI discovers paths dynamically. However, both source and target models must already be fully registered through their model files (`ProblemSchemaEntry` and `declare_variants!`), including any aliases and `pred create` construction contract (see `add-model` skill).

`ExtractionError` already propagates through `pred extract` and bundle `pred solve`; add a rule-specific CLI test only when the CLI surface changes.

Aggregate-only reductions currently have a narrower CLI surface:
- `pred solve <problem.json>` can still compute direct aggregate values for aggregate-only problems
Expand All @@ -261,7 +281,7 @@ Aggregate-only reductions currently have a narrower CLI surface:
- Rule file: `src/rules/<sourcelower>_<targetlower>.rs` -- no underscores within a problem name
- e.g., `maximumindependentset_qubo.rs`, `minimumvertexcover_maximumindependentset.rs`
- Test file: `src/unit_tests/rules/<sourcelower>_<targetlower>.rs`
- Canonical example: builder function in `src/example_db/rule_builders.rs`
- Canonical example: `canonical_rule_example_specs()` in the rule module, included from `src/rules/mod.rs`

## Common Mistakes

Expand All @@ -272,9 +292,10 @@ Aggregate-only reductions currently have a narrower CLI surface:
| Wrong overhead expression | Must accurately reflect the size relationship |
| Adding extra reduction metadata or duplicate primitive endpoint registration | Keep one primitive registration per endpoint pair and use only the `overhead` form of `#[reduction]` |
| Missing `extract_solution` mapping state | Store any index maps needed in the ReductionResult struct |
| Not adding canonical example to `example_db` | Add builder in `src/example_db/rule_builders.rs` |
| Permissive extraction | Validate first, then map exactly or return `ExtractionError` |
| Not adding a canonical example | Add the rule-local spec and include it from `src/rules/mod.rs` |
| Not regenerating reduction graph | Run `cargo run --example export_graph` after adding a rule |
| Skipping Step 5 (paper documentation) | **Every rule MUST have a `reduction-rule` entry in the paper. This is mandatory, not optional. PRs without documentation will be rejected.** |
| Source/target model not fully registered | Both problems must already have `declare_variants!`, aliases as needed, and CLI create support -- use `add-model` skill first |
| Skipping Step 6 (paper documentation) | **Every rule MUST have a `reduction-rule` entry in the paper. This is mandatory, not optional. PRs without documentation will be rejected.** |
| Source/target model not fully registered | Both problems must already have `ProblemSchemaEntry`, `declare_variants!`, registry aliases as needed, and a construction contract -- use `add-model` skill first |
| Treating a direct-to-ILP rule as a toy stub | Direct ILP reductions need exact overhead metadata and strong semantic regression tests, just like other production ILP rules |
| Skipping verification for complex reductions | Verification is default for a reason — `--no-verify` is for trivial identity/complement reductions only |
Loading
Loading