Skip to content

Commit d4060d6

Browse files
GiggleLiuclaude
andauthored
Add variant-level aliases; introduce 2SAT and 3SAT (#1054)
* Add variant-level aliases; introduce 2SAT and 3SAT Variant-level aliases attach to a specific reduction-graph node rather than a canonical problem name, so shorthand like `3SAT` can resolve to `KSatisfiability<K3>` directly instead of going through the problem's default variant (which is `KN`, not what `3SAT` means in the literature). - `VariantEntry` gains an `aliases: &'static [&'static str]` field. - `declare_variants!` accepts optional `aliases ["X", ...]` trailing each entry and emits the field. - `registry::find_variant_by_alias()` returns both the entry and its variant map. - CLI `parse_problem_spec` and `resolve_alias` try variant-level aliases before problem-level ones, injecting the alias's variant tokens into the spec; problem-level resolution is unchanged for `MIS`, `SAT`, etc. - Adds `2SAT` on `KSatisfiability<K2>` and `3SAT` on `KSatisfiability<K3>`. - `pred list` now shows variant-level aliases on their own rows. - `MAX2SAT` remains a problem-level alias on `Maximum2Satisfiability` (standalone problem, no variants). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address variant alias review follow-ups * fix: cover all error branches of validate_variant_aliases for codecov Refactor validation into a testable `validate_aliases_inner` that accepts mock data, so every conflict branch (empty alias, canonical-name collision, problem-alias collision, duplicate variant alias, multi-conflict) is exercised without depending on inventory state. Also test `variant_label` with both empty and non-empty variant dimensions. Patch coverage: 15 uncovered lines → ~2 (the inventory integration test panic branch, which is inherently unreachable on a healthy codebase). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 06d7f27 commit d4060d6

10 files changed

Lines changed: 504 additions & 23 deletions

File tree

problemreductions-cli/src/commands/create/schema_support.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,7 @@ pub(super) fn create_schema_driven(
180180

181181
// KColoring/KN stores the number of colors at runtime in `num_colors`.
182182
// The schema only declares `graph`, so inject `num_colors` from --k for KN.
183-
if canonical == "KColoring"
184-
&& resolved_variant.get("k").map(|s| s.as_str()) == Some("KN")
185-
{
183+
if canonical == "KColoring" && resolved_variant.get("k").map(|s| s.as_str()) == Some("KN") {
186184
if let Some(k) = args.k {
187185
json_map.insert("num_colors".to_string(), serde_json::json!(k));
188186
}

problemreductions-cli/src/commands/graph.rs

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,7 @@ pub fn list(out: &OutputConfig) -> Result<()> {
3333
for name in &types {
3434
let variants = graph.variants_for(name);
3535
let default_variant = graph.default_variant_for(name);
36-
let aliases = aliases_for(name);
37-
let alias_str = if aliases.is_empty() {
38-
String::new()
39-
} else {
40-
aliases.join(", ")
41-
};
36+
let problem_aliases = aliases_for(name);
4237

4338
for (i, v) in variants.iter().enumerate() {
4439
let slash = variant_to_full_slash(v);
@@ -53,13 +48,26 @@ pub fn list(out: &OutputConfig) -> Result<()> {
5348
.variant_complexity(name, v)
5449
.map(|c| big_o_of(&Expr::parse(c)))
5550
.unwrap_or_default();
51+
52+
// Per-row aliases: problem-level aliases on the first row, plus any
53+
// variant-level aliases attached to the specific reduction-graph node.
54+
let variant_aliases: Vec<&'static str> =
55+
problemreductions::registry::find_variant_entry(name, v)
56+
.map(|entry| entry.aliases.to_vec())
57+
.unwrap_or_default();
58+
let mut parts: Vec<String> = Vec::new();
59+
if i == 0 {
60+
for alias in &problem_aliases {
61+
push_alias_part(&mut parts, alias);
62+
}
63+
}
64+
for alias in &variant_aliases {
65+
push_alias_part(&mut parts, alias);
66+
}
67+
5668
rows_data.push(VariantRow {
5769
display,
58-
aliases: if i == 0 {
59-
alias_str.clone()
60-
} else {
61-
String::new()
62-
},
70+
aliases: parts.join(", "),
6371
is_default,
6472
rules: if i == 0 { rules } else { 0 },
6573
complexity,
@@ -715,6 +723,12 @@ pub fn export(out: &OutputConfig) -> Result<()> {
715723
out.emit_with_default_name("reduction_graph.json", &text, &json)
716724
}
717725

726+
fn push_alias_part(parts: &mut Vec<String>, alias: &str) {
727+
if !parts.iter().any(|part| part.eq_ignore_ascii_case(alias)) {
728+
parts.push(alias.to_string());
729+
}
730+
}
731+
718732
fn parse_direction(s: &str) -> Result<TraversalFlow> {
719733
match s {
720734
"out" => Ok(TraversalFlow::Outgoing),
@@ -805,3 +819,20 @@ fn render_tree(graph: &ReductionGraph, nodes: &[NeighborTree], text: &mut String
805819
}
806820
}
807821
}
822+
823+
#[cfg(test)]
824+
mod tests {
825+
use super::push_alias_part;
826+
827+
#[test]
828+
fn push_alias_part_deduplicates_case_insensitively_in_order() {
829+
let mut parts = Vec::new();
830+
push_alias_part(&mut parts, "KSAT");
831+
push_alias_part(&mut parts, "3SAT");
832+
push_alias_part(&mut parts, "ksat");
833+
push_alias_part(&mut parts, "2SAT");
834+
push_alias_part(&mut parts, "3sat");
835+
836+
assert_eq!(parts, vec!["KSAT", "3SAT", "2SAT"]);
837+
}
838+
}

problemreductions-cli/src/problem_name.rs

Lines changed: 116 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,14 @@ pub struct ProblemSpec {
1212

1313
/// Resolve a short alias to the canonical problem name.
1414
///
15-
/// Uses the catalog for both aliases and canonical names.
15+
/// Searches both variant-level aliases (e.g., `"3SAT"` → `KSatisfiability`) and
16+
/// problem-level aliases (e.g., `"MIS"` → `MaximumIndependentSet`). When a
17+
/// variant-level alias is matched, only the canonical name is returned here.
18+
/// The older pass-through behavior where `3SAT` resolved to `"3SAT"` has been
19+
/// intentionally replaced by `3SAT` resolving to `"KSatisfiability"` so aliases
20+
/// behave consistently. Callers that need variant semantics such as
21+
/// `3SAT` → `KSatisfiability { k = K3 }` should use [`parse_problem_spec`] or
22+
/// [`resolve_problem_ref`].
1623
pub fn resolve_alias(input: &str) -> String {
1724
if input.eq_ignore_ascii_case("UndirectedFlowLowerBounds") {
1825
return "UndirectedFlowLowerBounds".to_string();
@@ -38,6 +45,9 @@ pub fn resolve_alias(input: &str) -> String {
3845
if input.eq_ignore_ascii_case("GraphPartitioning") {
3946
return "GraphPartitioning".to_string();
4047
}
48+
if let Some((entry, _)) = problemreductions::registry::find_variant_by_alias(input) {
49+
return entry.name.to_string();
50+
}
4151
if let Some(pt) = problemreductions::registry::find_problem_type_by_alias(input) {
4252
return pt.canonical_name.to_string();
4353
}
@@ -64,16 +74,34 @@ pub fn resolve_catalog_problem_ref(
6474
}
6575

6676
/// Parse a problem spec string like "MIS/UnitDiskGraph/i32" into name + variant values.
77+
///
78+
/// Resolution order:
79+
/// 1. **Variant-level alias** (`"3SAT"` → `KSatisfiability` + variant tokens `["K3"]`):
80+
/// injects the variant tokens *before* any user-supplied tokens from the slash spec.
81+
/// 2. **Problem-level alias** (`"MIS"` → `MaximumIndependentSet`): canonical-name only;
82+
/// downstream default-variant resolution fills in the variant dimensions.
6783
pub fn parse_problem_spec(input: &str) -> anyhow::Result<ProblemSpec> {
6884
let parts: Vec<&str> = input.split('/').collect();
6985
let raw_name = parts[0];
70-
let variant_values: Vec<String> = parts[1..].iter().map(|s| s.to_string()).collect();
86+
let user_tokens: Vec<String> = parts[1..].iter().map(|s| s.to_string()).collect();
87+
88+
if let Some((entry, variant_map)) = problemreductions::registry::find_variant_by_alias(raw_name)
89+
{
90+
// Prepend the alias's own variant values; the slash-spec resolver handles
91+
// additional user tokens (and errors on dimension collisions).
92+
let mut variant_values: Vec<String> = variant_map.into_values().collect();
93+
variant_values.extend(user_tokens);
94+
return Ok(ProblemSpec {
95+
name: entry.name.to_string(),
96+
variant_values,
97+
});
98+
}
7199

72100
let name = resolve_alias(raw_name);
73101

74102
Ok(ProblemSpec {
75103
name,
76-
variant_values,
104+
variant_values: user_tokens,
77105
})
78106
}
79107

@@ -301,8 +329,11 @@ mod tests {
301329
assert_eq!(resolve_alias("X3C"), "ExactCoverBy3Sets");
302330
assert_eq!(resolve_alias("3Partition"), "ThreePartition");
303331
assert_eq!(resolve_alias("3-partition"), "ThreePartition");
304-
// 3SAT is no longer a registered alias (removed to avoid confusion with KSatisfiability/KN)
305-
assert_eq!(resolve_alias("3SAT"), "3SAT"); // pass-through
332+
// Variant-level aliases: resolve_alias only returns the canonical name;
333+
// parse_problem_spec recovers the variant tokens (see tests below).
334+
assert_eq!(resolve_alias("3SAT"), "KSatisfiability");
335+
assert_eq!(resolve_alias("3sat"), "KSatisfiability");
336+
assert_eq!(resolve_alias("2SAT"), "KSatisfiability");
306337
assert_eq!(resolve_alias("QUBO"), "QUBO");
307338
assert_eq!(resolve_alias("MaxCut"), "MaxCut");
308339
assert_eq!(
@@ -372,6 +403,86 @@ mod tests {
372403
assert_eq!(spec.variant_values, vec!["K3"]);
373404
}
374405

406+
#[test]
407+
fn test_parse_problem_spec_variant_alias_3sat() {
408+
// Variant-level alias: "3SAT" injects the K3 variant token.
409+
let spec = parse_problem_spec("3SAT").unwrap();
410+
assert_eq!(spec.name, "KSatisfiability");
411+
assert_eq!(spec.variant_values, vec!["K3"]);
412+
413+
let spec = parse_problem_spec("3sat").unwrap();
414+
assert_eq!(spec.name, "KSatisfiability");
415+
assert_eq!(spec.variant_values, vec!["K3"]);
416+
}
417+
418+
#[test]
419+
fn test_parse_problem_spec_variant_alias_2sat() {
420+
let spec = parse_problem_spec("2SAT").unwrap();
421+
assert_eq!(spec.name, "KSatisfiability");
422+
assert_eq!(spec.variant_values, vec!["K2"]);
423+
}
424+
425+
#[test]
426+
fn test_resolve_problem_ref_variant_alias_3sat() {
427+
let graph = problemreductions::rules::ReductionGraph::new();
428+
let expected = ProblemRef {
429+
name: "KSatisfiability".to_string(),
430+
variant: BTreeMap::from([("k".to_string(), "K3".to_string())]),
431+
};
432+
433+
assert_eq!(resolve_problem_ref("3SAT", &graph).unwrap(), expected);
434+
}
435+
436+
#[test]
437+
fn test_resolve_problem_ref_variant_alias_2sat() {
438+
let graph = problemreductions::rules::ReductionGraph::new();
439+
let expected = ProblemRef {
440+
name: "KSatisfiability".to_string(),
441+
variant: BTreeMap::from([("k".to_string(), "K2".to_string())]),
442+
};
443+
444+
assert_eq!(resolve_problem_ref("2SAT", &graph).unwrap(), expected);
445+
}
446+
447+
#[test]
448+
fn test_resolve_problem_ref_3sat_k2_rejects_duplicate_dimension() {
449+
let spec = parse_problem_spec("3SAT/K2").unwrap();
450+
assert_eq!(spec.name, "KSatisfiability");
451+
assert_eq!(spec.variant_values, vec!["K3", "K2"]);
452+
453+
let graph = problemreductions::rules::ReductionGraph::new();
454+
let err = resolve_problem_ref("3SAT/K2", &graph).unwrap_err();
455+
assert!(
456+
err.to_string().contains("specified more than once"),
457+
"expected duplicate-dimension error, got: {err}"
458+
);
459+
}
460+
461+
#[test]
462+
fn test_resolve_problem_ref_3sat_simple_graph_rejects_unknown_token() {
463+
let spec = parse_problem_spec("3SAT/SimpleGraph").unwrap();
464+
assert_eq!(spec.name, "KSatisfiability");
465+
assert_eq!(spec.variant_values, vec!["K3", "SimpleGraph"]);
466+
467+
let graph = problemreductions::rules::ReductionGraph::new();
468+
let err = resolve_problem_ref("3SAT/SimpleGraph", &graph).unwrap_err();
469+
assert!(
470+
err.to_string()
471+
.to_lowercase()
472+
.contains("unknown variant token"),
473+
"expected unknown-token error, got: {err}"
474+
);
475+
}
476+
477+
#[test]
478+
fn test_parse_problem_spec_max2sat_problem_level() {
479+
// MAX2SAT is a problem-level alias on Maximum2Satisfiability (standalone problem,
480+
// no K variants) — no variant tokens injected.
481+
let spec = parse_problem_spec("MAX2SAT").unwrap();
482+
assert_eq!(spec.name, "Maximum2Satisfiability");
483+
assert!(spec.variant_values.is_empty());
484+
}
485+
375486
#[test]
376487
fn test_suggest_problem_name_close() {
377488
// "MISs" is 1 edit from "MIS" alias -> should suggest MaximumIndependentSet

problemreductions-cli/src/test_support.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ problemreductions::inventory::submit! {
126126
complexity: "2^num_values",
127127
complexity_eval_fn: |_| 1.0,
128128
is_default: true,
129+
aliases: &[],
129130
factory: |data| {
130131
let problem: AggregateValueSource = serde_json::from_value(data)?;
131132
Ok(Box::new(problem))
@@ -146,6 +147,7 @@ problemreductions::inventory::submit! {
146147
complexity: "2",
147148
complexity_eval_fn: |_| 1.0,
148149
is_default: true,
150+
aliases: &[],
149151
factory: |data| {
150152
let problem: AggregateValueTarget = serde_json::from_value(data)?;
151153
Ok(Box::new(problem))

problemreductions-macros/src/lib.rs

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,11 +444,12 @@ struct DeclareVariantsInput {
444444
entries: Vec<DeclareVariantEntry>,
445445
}
446446

447-
/// A single entry: `[default] Type => "complexity_string"`.
447+
/// A single entry: `[default] Type => "complexity_string" [aliases ["X", ...]]`.
448448
struct DeclareVariantEntry {
449449
is_default: bool,
450450
ty: Type,
451451
complexity: syn::LitStr,
452+
aliases: Vec<syn::LitStr>,
452453
}
453454

454455
impl syn::parse::Parse for DeclareVariantsInput {
@@ -464,10 +465,47 @@ impl syn::parse::Parse for DeclareVariantsInput {
464465
let ty: Type = input.parse()?;
465466
input.parse::<syn::Token![=>]>()?;
466467
let complexity: syn::LitStr = input.parse()?;
468+
469+
// Optional: `aliases ["X", "Y", ...]`
470+
let aliases = if input.peek(syn::Ident) {
471+
let fork = input.fork();
472+
let ident: syn::Ident = fork.parse()?;
473+
if ident == "aliases" {
474+
input.parse::<syn::Ident>()?;
475+
let content;
476+
syn::bracketed!(content in input);
477+
let mut out = Vec::new();
478+
while !content.is_empty() {
479+
let lit: syn::LitStr = content.parse()?;
480+
if lit.value().trim().is_empty() {
481+
return Err(syn::Error::new(
482+
lit.span(),
483+
"variant alias must not be empty or whitespace-only",
484+
));
485+
}
486+
out.push(lit);
487+
if content.peek(syn::Token![,]) {
488+
content.parse::<syn::Token![,]>()?;
489+
}
490+
}
491+
out
492+
} else if fork.peek(syn::token::Bracket) {
493+
return Err(syn::Error::new(
494+
ident.span(),
495+
format!("expected 'aliases', found '{ident}'"),
496+
));
497+
} else {
498+
Vec::new()
499+
}
500+
} else {
501+
Vec::new()
502+
};
503+
467504
entries.push(DeclareVariantEntry {
468505
is_default,
469506
ty,
470507
complexity,
508+
aliases,
471509
});
472510

473511
if input.peek(syn::Token![,]) {
@@ -552,6 +590,7 @@ fn generate_declare_variants(input: &DeclareVariantsInput) -> syn::Result<TokenS
552590
let ty = &entry.ty;
553591
let complexity_str = entry.complexity.value();
554592
let is_default = entry.is_default;
593+
let alias_lits: Vec<_> = entry.aliases.iter().map(|s| s.value()).collect();
555594

556595
// Parse the complexity expression to validate syntax
557596
let parsed = parser::parse_expr(&complexity_str).map_err(|e| {
@@ -633,6 +672,7 @@ fn generate_declare_variants(input: &DeclareVariantsInput) -> syn::Result<TokenS
633672
complexity: #complexity_str,
634673
complexity_eval_fn: #complexity_eval_fn,
635674
is_default: #is_default,
675+
aliases: &[#(#alias_lits),*],
636676
#dispatch_fields
637677
}
638678
}
@@ -767,6 +807,44 @@ mod tests {
767807
);
768808
}
769809

810+
#[test]
811+
fn declare_variants_rejects_empty_alias_literal() {
812+
let err =
813+
match syn::parse_str::<DeclareVariantsInput>("default Foo => \"1\" aliases [\"\"]") {
814+
Ok(_) => panic!("empty alias literal should be rejected"),
815+
Err(err) => err,
816+
};
817+
assert!(
818+
err.to_string().contains("empty or whitespace-only"),
819+
"expected empty-alias error, got: {err}"
820+
);
821+
}
822+
823+
#[test]
824+
fn declare_variants_rejects_whitespace_only_alias_literal() {
825+
let err = match syn::parse_str::<DeclareVariantsInput>(
826+
"default Foo => \"1\" aliases [\" \\t\"]",
827+
) {
828+
Ok(_) => panic!("whitespace-only alias literal should be rejected"),
829+
Err(err) => err,
830+
};
831+
assert!(
832+
err.to_string().contains("empty or whitespace-only"),
833+
"expected whitespace-only alias error, got: {err}"
834+
);
835+
}
836+
837+
#[test]
838+
fn declare_variants_rejects_unknown_alias_keyword_before_bracket() {
839+
let err = match syn::parse_str::<DeclareVariantsInput>(
840+
"default Foo => \"1\" nicknames [\"Foo\"]",
841+
) {
842+
Ok(_) => panic!("unknown aliases keyword should be rejected"),
843+
Err(err) => err,
844+
};
845+
assert_eq!(err.to_string(), "expected 'aliases', found 'nicknames'");
846+
}
847+
770848
#[test]
771849
fn declare_variants_generates_aggregate_value_and_witness_dispatch() {
772850
let input: DeclareVariantsInput = syn::parse_quote! {

0 commit comments

Comments
 (0)