refactor(intelligence): merge the two causal-chain engines into one (P1.2)#67
Merged
Conversation
Closes #48 (P1.2). There were two parallel engines: ImpactChainEngine (in-code templates, substring match) and CausalChainConstructor/EventImpactEngine (YAML templates, regex + LLM fallback). The latter is canonical (core trading pipeline). Make ImpactChainEngine a thin ADAPTER over CausalChainConstructor so there is one chain-construction engine, preserving its public API. - impact_chain.py: ImpactChainEngine now delegates to CausalChainConstructor. build_chain() bridges CausalChain→ImpactChain (bullish→positive, confidence→ magnitude, order→lag). CHAIN_TEMPLATES is retained ONLY to build a sector→stock map that re-binds affected_stocks (Engine B resolves sectors only) — so find_stock_impact keeps working. Engine B and its callers (aggregator/investment_director/trading_loop) are untouched. - config/impact_chain_templates.yaml: add usd_strengthen / usd_weaken templates so USD→gold transmission (the one capability the in-code engine had and the YAML lacked) is preserved — making the merge lossless. - tests: rewrite test_impact_chain.py to the new contract (incl. a test proving find_stock_impact returns stock-level impacts); restore test_usd_affects_gold. Verified: full unit suite 3811 passed; ruff clean. detect_event_type is now single-match (documented semantic shift; no caller depended on multi-match). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Closes #48. Two parallel engines existed:
ImpactChainEngine(in-code templates, substring match) andCausalChainConstructor/EventImpactEngine(YAML templates, regex + LLM fallback). The latter is canonical (it drives the core trading pipeline). This makesImpactChainEnginea thin adapter overCausalChainConstructor— one engine — while preserving its public API.How
impact_chain.py:ImpactChainEnginedelegates toCausalChainConstructor;build_chainbridgesCausalChain→ImpactChain(bullish→positive, confidence→magnitude, order→lag). The oldCHAIN_TEMPLATESis kept only to build a sector→stock map that re-bindsaffected_stocks— because Engine B resolves sectors only — sofind_stock_impactkeeps working (this was the silent-regression trap; it's defused + tested). Engine B + its callers (aggregator / investment_director / trading_loop) are untouched.config/impact_chain_templates.yaml: addedusd_strengthen/usd_weakenso USD→gold (the one capability the in-code engine had that the YAML lacked) is preserved — the merge is lossless.test_impact_chain.pyrewritten to the new contract (incl. a test provingfind_stock_impactreturns stock-level impacts);test_usd_affects_goldrestored.Verified
Full unit suite 3811 passed;
ruffclean; intelligence sweep 234 green.detect_event_typeis now single-match (documented semantic shift; no caller relied on multi-match). Simulation-only; not investment advice.