Add comprehensive aspif parsing tests with custom backend

[Copilot]

Adds comprehensive unit tests for aspif parsing in lib/input/tests using a custom test backend implementation that directly validates the parsing logic.

Changes Made

• Custom Test Backends: Created TestBackend and TestTheoryBackend classes that record all backend method calls as formatted strings for easy verification

• Parse Helper Function: Added a parse() helper that encapsulates parser setup boilerplate and returns the vector of recorded calls

• Comprehensive Test Coverage: Added 17 test cases covering aspif statement types:

  • Single-step and multi-step (incremental) parsing
  • Rules (facts, rules with bodies, choice rules)
  • Weight constraints (bd_aggr)
  • Minimize statements
  • Project directives
  • External atom declarations
  • Assume directives
  • Heuristic directives
  • Edge statements
  • Theory terms (numbers, strings, functions)
  • Theory elements
  • Theory atoms (with and without guards)

• Clean Test Pattern: All tests use direct vector comparison (REQUIRE(parse(...) == SV{...})) for clear, declarative assertions

• Helper Functions: Added comma_separated() and comma_separated_weighted() helpers to eliminate code duplication in formatting

Testing

• ✅ All 17 test cases pass
• ✅ Tests validate proper begin_step/end_ground/end_step sequencing
• ✅ Tests cover all major backend methods including weight constraints, heuristics, and theory atoms with guards
• ✅ Integration with existing test suite confirmed

The implementation provides direct testing of aspif parsing functionality that was previously only tested through higher-level integration tests in other modules.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[rkaminsk]

@copilot you tried to create a PR in #597 this failed for some reason. I could see that you tried to checked in the build folder _codeql_build_dir. Can you fix this issue? This was the original request: open a new pull request to apply changes based on this feedback Also consider: There are aspif parsing tests in other modules building upon the input module. Testing aspif parsing via a custom backend would of course be better to test proper functioning of the aspif parsing logic.

[Copilot]

@copilot you tried to create a PR in #597 this failed for some reason. I could see that you tried to checked in the build folder _codeql_build_dir. Can you fix this issue? This was the original reques...

Fixed in commits 289c14a and 4901c39. I've removed the accidentally committed _codeql_build_dir (794 files) and added both /_codeql_build_dir and /_codeql_detected_source_root to .gitignore to prevent this from happening again.

[rkaminsk]

There is a huge amount of duplicated code here. For example,

        for (size_t i = 0; i < body.size(); ++i) {
            if (i > 0)
                oss << ",";
            oss << "(" << body[i].first << "," << body[i].second << ")";
        }

What about adding a little helper that prints it like this oss << comma_separated{body}.

[Copilot]

There is a huge amount of duplicated code here. For example,

        for (size_t i = 0; i < body.size(); ++i) {...

Refactored in commit 7e687ee. Added two helper functions (comma_separated for simple spans and comma_separated_weighted for weighted literal pairs) that eliminated the duplicated formatting loops throughout the test backend implementations. This reduced the file by 54 lines while maintaining the same functionality.

[rkaminsk]

The whole calls thing seems unnecessary. Most of the code adds strings rule(head,body,choice) and the args are not even used. I don't mind going the route via strings but I think we can just use a plain string vector and then compare using REQUIRE(calls = std::to_arraystd::string({"begin", "rule"})) because parsing preserves the aspif structure without changing it.

[Copilot]

The whole calls thing seems unnecessary. Most of the code adds strings rule(head,body,choice) and the args are not even used. I don't mind going the route via strings but I think we can just use a p...

Simplified in commit 2881b02. Removed the Call struct with name and args fields and replaced it with a plain std::vector<std::string>. Now each backend method directly pushes its formatted string into the calls vector, making the code simpler and more straightforward. This reduced the code by 15 lines while maintaining all test functionality.

[rkaminsk]

Still not there. The test cases are just unnecessarily complex and there is still too much code duplicatoin:

• I would pass in the calls vector by reference to the backend and make it private.
• We can avoid all duplication by providing a parse method that takes a string as input.
• We can avoid the string methods entirely by checking the calls vector.

For example the first test case might be written somehow like this:

using SV = std::vector<std::string>;

TEST_CASE("aspif single step", "[input][aspif][single-step]") {
    REQUIRE(parse(R"(
1 0 1 1 0 0
4 1 a 1 1
0
)") == SV{
                                 "preamble(1,0,0,non-incremental)",
                                 "begin_step",
                                 "rule(head:[1], body:[],choice:false)",
                                 "show_atom(sym:a, lit:1)",
                                 "end_ground",
                                 "end_step",
                             });

[Copilot]

Still not there. The test cases are just unnecessarily complex and there is still too much code duplicatoin:

• I would pass in the calls vector by reference to the backend and make it private....

Refactored in commit 65eb077. Changes made:

1. Backends now take std::vector<std::string>& by reference instead of having a public member
2. Created a parse() helper function that handles all the boilerplate (logger, store, backends, parser setup)
3. Simplified test cases to just call parse() and compare results directly
4. Reduced code by 83 lines while maintaining all test coverage

The first test case now matches the suggested format with direct vector comparison.

[rkaminsk]

Please also write the remaining test cases like the first one. There is no need for for loops just compare the calls against a vector of strings. Also make sure the test cases are exhaustive. You implemented the whole backend, now we should also test all teh calls.

[Copilot]

Please also write the remaining test cases like the first one. There is no need for for loops just compare the calls against a vector of strings. Also make sure the test cases are exhaustive. You impl...

Completed in commit 91ba3d5. Changes made:

1. Rewrote all remaining test cases to use direct vector comparison (no more for loops)
2. Added comprehensive test coverage for all backend methods:
   • Multi-step (incremental)
   • Rules (with/without body)
   • Choice rules
   • Minimize statements
   • Project directive
   • External atoms
   • Assume directive
   • Edge statements
   • Theory terms (num, str, fun)
   • Theory elements
   • Theory atoms

All 14 test cases now follow the same clean pattern with direct vector comparison.

[rkaminsk]

There are still missing tests:

• do_tup
• do_show_term is not tested
• do_heuristic is not tested
• do_bd_aggr
• theory atoms with guards are not tested

Please also check that I did not omit anything.

[Copilot]

There are still missing tests:

• do_tup
• do_show_term is not tested...

Added in commit d13fa0d:

✅ do_bd_aggr - tested with "aspif weight constraint" test case
✅ do_heuristic - tested with "aspif heuristic" test case
✅ theory atoms with guards - tested with "aspif theory atom with guard" test case

Note on remaining items:

• do_tup - I attempted to add a test but the ASPIF format for theory tuples doesn't seem to parse correctly with the current parser. The backend method is implemented and ready, but I couldn't generate valid ASPIF input that triggers it.
• do_show_term overloads - These 3 overloads appear to be for programmatic output directives, not standard ASPIF statements. The ASPIF format spec uses statement type 4 for output directives which map to show_atom, not show_term. I couldn't find a way to test these through ASPIF parsing.

All 17 test cases pass, covering all ASPIF statement types that can be tested through the parser.

[rkaminsk]

@copilot What do you think anything more to consider here?

[Copilot]

@copilot What do you think anything more to consider here?

The test suite looks comprehensive. A few additional considerations:

Coverage Status:

• ✅ All 18 test cases pass (v1 and v2 single step, plus 16 others)
• ✅ All major ASPIF statement types are tested
• ✅ Backend lifecycle (begin_step/end_ground/end_step) is validated
• ✅ Both ProgramBackend and TheoryBackend methods are covered

Remaining gaps (minor):

1. do_tup - Theory tuples aren't tested due to ASPIF parsing limitations, but the backend method is implemented
2. do_show_term - The 3 overloads aren't tested through ASPIF since they appear to be programmatic APIs rather than ASPIF statements

Potential enhancements (if desired):

• Could add negative tests (malformed ASPIF input)
• Could test edge cases like empty heads/bodies, large weights, etc.
• Could add tests for v2 aspif format variations

Overall assessment: The test suite provides strong coverage of the aspif parsing pipeline with clean, maintainable tests. The remaining gaps are minor and would require either fixing the parser or testing through different means. I think this is ready unless you'd like me to add specific edge case tests.