feat: Implement unified generator mode with common_generator abstraction#98
Open
feat: Implement unified generator mode with common_generator abstraction#98
Conversation
Fixed 5 critical issues from code review: 1. Accessor format bug - now generates fact[arg0] instead of fact[0] 2. N-way joins - support for 3+ goal joins with nested loops 3. Negation support - implements not/1 and backslash-plus 4. API compatibility - added csharp_query_target shim 5. Enhanced tests - generator test now compiles and runs C# code Also renamed csharp_query_target to csharp_target and updated all references.
Critical fixes for multi-way joins: - Thread accumulated Goal-Access pairs through compile_nway_join recursion - Base case now uses ALL accumulated pairs for VarMap, not just FirstGoal - Join conditions now properly reference correct source variables - Removed broken collect_previous_goals and get_source_for_index helpers - Fixed VarMap bug that caused variables from later goals to resolve to null This fixes the core correctness issue where rules with 3+ relational goals would generate incorrect C# code with broken variable lookups.
Fixed all remaining blocking issues from second review: Issue 3 - Test File Syntax: - Removed duplicate/corrupted code in test_csharp_target.pl - File now loads without syntax errors - Eliminated raw C# code appearing outside Prolog terms Issue 4 - Test Suite Import: - Updated run_all_tests.pl to use test_csharp_target - Removed references to old test_csharp_query_target module Additional: - Wired test_common_generator.pl into test suite - Added module import and invocation to run_all_tests.pl All blocking issues from Codex Review 2 now resolved.
# Conflicts: # src/unifyweaver/core/recursive_compiler.pl # src/unifyweaver/targets/csharp_query_target.pl # src/unifyweaver/targets/python_target.pl
…e-unified-api # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements a unified generator mode architecture across C#, Python, and Bash targets by introducing a shared
common_generator.plmodule. It refactors the C# target to support both query and generator modes, fixes critical bugs identified in code reviews, and establishes a foundation for consistent cross-target behavior.Type of Change
Key Changes
1. Architecture Improvements
common_generator.pl: Centralized variable mapping, expression translation, and negation handlingcsharp_query_target.pl→csharp_target.plwith dual-mode supportcsharp_query_target.plshim to prevent breaking existing code2. Critical Bug Fixes (from Code Reviews)
Codex Review #1 Fixes (Commit:
b4429eb)fact["arg0"]generation (was generatingfact["0"])\+andnot/1translation to!total.Contains(...)Codex Review #2 Fixes (Commits:
2633a2b,173e022)compile_nway_jointo properly thread Goal-Access pairsrun_all_tests.plto use renamed modules3. New Modules
common_generator.pltest_common_generator.plcsharp_target.plcsharp_query_target.pl4. Modified Files
Core:
src/unifyweaver/core/recursive_compiler.pl- Updated imports to use empty lists (avoid conflicts)Tests:
tests/core/test_csharp_target.pl- Renamed, added generator mode execution teststests/core/test_csharp_janus.pl- Updated importstests/core/test_csharp_constraint_operand.pl- Updated importsrun_all_tests.pl- Updated to new module names, addedtest_common_generatorRemoved:
tests/core/test_csharp_query_target.pl- Replaced bytest_csharp_target.plBreaking Changes
Module Rename
csharp_query_target→csharp_targetCommits in This PR
b4429eb- fix: Address CodeX review - implement generator mode fixes2633a2b- fix: Address Codex Review 2 - fix N-way join VarMap threading173e022- fix: Address Codex Review 2 remaining issuesTesting
swipl -g "consult('run_all_tests.pl'), main, halt" -t haltStatus: Ready for Review