Skip to content

Fix #414: [Model] LongestCommonSubsequence#666

Merged
zazabap merged 7 commits intomainfrom
issue-414-longest-common-subsequence
Mar 18, 2026
Merged

Fix #414: [Model] LongestCommonSubsequence#666
zazabap merged 7 commits intomainfrom
issue-414-longest-common-subsequence

Conversation

@GiggleLiu
Copy link
Contributor

Summary

  • add an execution plan for issue [Model] LongestCommonSubsequence #414
  • refactor the existing LongestCommonSubsequence stack to match the issue's decision-model contract
  • keep the ILP, CLI, tests, examples, and paper entries aligned during execution

Fixes #414

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.17%. Comparing base (057025f) to head (842c792).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #666   +/-   ##
=======================================
  Coverage   97.17%   97.17%           
=======================================
  Files         304      304           
  Lines       40108    40061   -47     
=======================================
- Hits        38976    38931   -45     
+ Misses       1132     1130    -2     

☔ 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.

@GiggleLiu
Copy link
Contributor Author

Implementation Summary

Changes

  • Reworked LongestCommonSubsequence into the issue-414 decision model with alphabet_size, strings, and bound, plus satisfaction semantics, canonical examples, and updated paper/example-db wiring.
  • Replaced the existing LongestCommonSubsequence -> ILP reduction with a bounded-witness feasibility encoding that uses symbol-selection variables, per-string match-position variables, ordering constraints, and witness extraction.
  • Updated CLI creation support for LCS to require --strings and --bound, support both numeric strings (0,1,0;1,0,1) and compact raw-string input (010110;100101), and refresh the help text/examples.
  • Rewrote the LCS model/rule/unit tests and refreshed generated docs artifacts (examples.json, problem_schemas.json, reduction_graph.json) and the paper section/reference entry.

Deviations from Plan

  • The repository already contained an older optimization-oriented LongestCommonSubsequence model and ILP rule on main, so the work was a semantic refactor of those existing files rather than a net-new model/rule addition.
  • The implementation accepts the degenerate bound = 0 case for library consistency, even though the issue statement focuses on positive K.

Open Questions

  • None.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the Issue #414 decision-model contract for LongestCommonSubsequence (fixed witness length K), refactors the ILP reduction to a feasibility formulation that supports multiple strings, and updates CLI/docs/examples/tests to match the new schema and semantics.

Changes:

  • Refactor LongestCommonSubsequence from an optimization/selection model to a SatisfactionProblem with (alphabet_size, strings, bound) and witness-as-config semantics.
  • Replace the LCS→ILP reduction with a bounded-witness feasibility ILP (symbol-choice + per-string embedding variables + monotonicity constraints).
  • Align unit tests, CLI pred create parsing/help, example fixtures, and generated docs/paper content with the new model.

Reviewed changes

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

Show a summary per file
File Description
src/unit_tests/trait_consistency.rs Adds trait-consistency coverage for the updated LCS model.
src/unit_tests/rules/longestcommonsubsequence_ilp.rs Updates ILP reduction tests to the new bounded-witness feasibility formulation.
src/unit_tests/models/misc/longest_common_subsequence.rs Rewrites model tests for new schema, boolean metric, and witness configs.
src/rules/longestcommonsubsequence_ilp.rs Refactors the rule to build a feasibility ILP for bounded-witness LCS over multiple strings.
src/models/misc/mod.rs Registers canonical example specs for the LCS model.
src/models/misc/longest_common_subsequence.rs Implements the new decision/satisfaction LCS model and variant complexity.
src/example_db/fixtures/examples.json Adds/updates canonical examples for the new LCS model and its ILP reduction.
problemreductions-cli/src/commands/create.rs Updates pred create help + parsing for LCS (strings, bound, alphabet-size).
problemreductions-cli/src/cli.rs Updates CLI flag listing/docs for LCS arguments.
docs/src/reductions/reduction_graph.json Regenerates reduction graph data for updated complexity/overhead formulas.
docs/src/reductions/problem_schemas.json Regenerates schema docs to reflect new LCS fields/types.
docs/paper/references.bib Adds Wagner–Fischer reference for the LCS discussion.
docs/paper/reductions.typ Updates paper section and ILP reduction description to the bounded-witness decision model.

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

Comment on lines +1698 to +1705
let find-embed(target, candidate) = {
let positions = ()
let j = 0
for (i, ch) in target.enumerate() {
if j < candidate.len() and ch == candidate.at(j) {
positions.push(i)
j += 1
}
"Vec<W>" => "comma-separated: 1,2,3",
"Vec<CNFClause>" => "semicolon-separated clauses: \"1,2;-1,3\"",
"Vec<Vec<W>>" => "semicolon-separated rows: \"1,0.5;0.5,2\"",
"Vec<Vec<usize>>" => "semicolon-separated strings: \"0,1,2;1,2,0\"",
"--alphabet-size {} is smaller than the inferred alphabet size ({})",
alphabet_size,
inferred_alphabet_size
);
GiggleLiu and others added 3 commits March 17, 2026 03:29
…14-longest-common-subsequence

# Conflicts:
#	docs/src/reductions/problem_schemas.json
#	docs/src/reductions/reduction_graph.json
#	problemreductions-cli/src/cli.rs
#	problemreductions-cli/src/commands/create.rs
#	src/example_db/fixtures/examples.json
#	src/unit_tests/trait_consistency.rs
Resolve merge conflicts from recent main additions (StaffScheduling,
MultiprocessorScheduling, StringToStringCorrection) and regenerate
examples.json fixtures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zazabap zazabap merged commit 3e409d1 into main Mar 18, 2026
3 checks passed
@zazabap zazabap deleted the issue-414-longest-common-subsequence branch March 18, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Model] LongestCommonSubsequence

3 participants