Conversation
hzhangxyz
commented
Mar 12, 2026
hzhangxyz
commented
- Create ddss/chain.py and ddss/chain.ts using Chain API from apyds/atsds
- Add chain component to main.ts and main.py component maps (not in default list)
- Add tests: tests/test_chain.py and tests/test_chain.ts
- Update README.md and documentation (docs/en/.md, docs/zh/.md)
- Create ddss/chain.py and ddss/chain.ts using Chain API from apyds/atsds - Add chain component to main.ts and main.py component maps (not in default list) - Add tests: tests/test_chain.py and tests/test_chain.ts - Update README.md and documentation (docs/en/*.md, docs/zh/*.md)
There was a problem hiding this comment.
Pull request overview
Adds a new “chain” component that uses the apyds/atsds Chain API as an alternative forward-chaining engine, wired into both the Python and TypeScript CLIs and documented alongside existing components.
Changes:
- Introduce
ddss/chain.pyandddss/chain.tsimplementing the new Chain-based engine. - Register
chainin the Python/TS component maps (while keeping defaults unchanged) and add corresponding tests. - Bump apyds/atsds dependency versions and update README + docs (EN/ZH) to mention the new component.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ddss/chain.py |
New Chain-based forward-chaining component for Python |
ddss/chain.ts |
New Chain-based forward-chaining component for TypeScript |
ddss/main.py |
Adds chain to the component map |
ddss/main.ts |
Adds chain to the component map |
tests/test_chain.py |
New Python tests for chain inference behavior |
tests/test_chain.ts |
New TS tests for chain inference behavior |
pyproject.toml |
Bumps apyds* dependencies to ~0.0.18 |
uv.lock |
Lockfile update for apyds* to 0.0.18 |
package.json |
Bumps atsds* dependencies to ^0.0.18 |
package-lock.json |
Lockfile update for atsds* to 0.0.18 |
README.md |
Documents the new chain component |
docs/en/usage.md |
Adds chain to available components |
docs/en/modules.md |
Adds Chain module listing |
docs/zh/usage.md |
Adds chain to available components (ZH) |
docs/zh/modules.md |
Adds Chain module listing (ZH) |
💡 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.
| const ds = rule.toString(); | ||
| tasks.push(insertOrIgnore(Fact, ds)); | ||
| const idea = strRuleGetStrIdea(ds); | ||
| if (idea) { | ||
| tasks.push(insertOrIgnore(Idea, idea)); | ||
| } |
There was a problem hiding this comment.
The new code inserts derived ideas via strRuleGetStrIdea(ds) and insertOrIgnore(Idea, ...), but the added Chain tests only assert inferred facts. Add at least one test case that exercises and verifies idea creation (similar to the existing Search test that expects an Ideas row) so this behavior is protected for the Chain engine too.
| ds = str(rule) | ||
| tasks.append(asyncio.create_task(insert_or_ignore(sess, Facts, ds))) | ||
| if idea := str_rule_get_str_idea(ds): | ||
| tasks.append(asyncio.create_task(insert_or_ignore(sess, Ideas, idea))) | ||
| return False |
There was a problem hiding this comment.
ddss.chain.main creates Ideas rows when str_rule_get_str_idea(ds) returns a value, but the newly added Chain tests never assert that ideas are created. Add a test that triggers idea generation and verifies an Ideas record is inserted, mirroring the coverage that exists for the Search engine.
| # Add facts that will produce a duplicate: a => b twice with => a | ||
| async with session() as sess: | ||
| sess.add(Facts(data="a\n----\nc\n")) | ||
| sess.add(Facts(data="b\n----\nc\n")) | ||
| sess.add(Facts(data="----\na\n")) | ||
| sess.add(Facts(data="----\nb\n")) |
There was a problem hiding this comment.
The test comment says this scenario is "a => b twice with => a", but the actual inserted rules are a => c, b => c, => a, => b (deriving => c). Update the comment/docstring to match the setup so the test intent stays clear.
