Skip to content

fix: Fix the return number of function execute of chain. - #274

Merged
hzhangxyz merged 1 commit into
mainfrom
dev/fix/chain-return
Mar 14, 2026
Merged

fix: Fix the return number of function execute of chain.#274
hzhangxyz merged 1 commit into
mainfrom
dev/fix/chain-return

Conversation

@hzhangxyz

Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings March 14, 2026 05:34
@hzhangxyz
hzhangxyz merged commit fe46257 into main Mar 14, 2026
106 of 173 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 adjusts Chain.execute()’s returned count to reflect all newly inferred results during execution, aligning behavior across the C++ core and the Python/JS/C++ test suites.

Changes:

  • Update ds::chain_t::execute() to return the combined number of inferred intermediate rules and inferred facts.
  • Update Chain tests in Python, JS, and C++ to assert the corrected return values.

Reviewed changes

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

File Description
src/chain.cc Fixes execute() return value to include both temp_rules and temp_facts.
tests/test_chain.py Updates expected counts for Chain.execute() in Python tests.
tests/test_chain.mjs Updates expected counts for Chain.execute() in JS tests.
tests/test_chain.cc Updates expected counts for chain_t::execute() in C++ tests.
Comments suppressed due to low confidence (1)

src/chain.cc:135

  • execute() inserts newly inferred facts into facts with current_cycle, but it never updates last_fact_cycle. Because the rule-skip condition earlier uses last_fact_cycle <= done_cycle, subsequent execute() calls can incorrectly skip processing even when this call inferred new facts (breaking multi-step inference across rules). Update last_fact_cycle when new facts are added here (e.g., set it to current_cycle if temp_facts is non-empty).
        for (auto it = temp_facts.begin(); it != temp_facts.end();) {
            auto node = temp_facts.extract(it++);
            facts.emplace(std::move(node.value()), current_cycle);
        }
        return count;

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

You can also share your feedback on Copilot code review. Take the survey.

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.

2 participants