feat(cli): aelf reason + aelf wonder — Track B v2.0 graph-walk surfaces (#389) - #409
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR introduces two new CLI commands ( ChangesAelf Reason & Wonder CLI Commands
Sequence DiagramssequenceDiagram
actor User
participant CLI as aelf CLI
participant Store as MemoryStore
participant BFS as expand_bfs
participant Output as Output Formatter
User->>CLI: aelf reason --seed-id bid1 [--json]
CLI->>Store: load store
CLI->>Store: search_beliefs(query) or validate seed
Store-->>CLI: seed belief IDs
CLI->>BFS: expand_bfs(seeds, depth, fanout, budget)
BFS-->>CLI: hop tree {belief_id, edges, hops}
CLI->>Output: format chain or JSON
Output-->>User: indented tree or JSON payload
sequenceDiagram
actor User
participant CLI as aelf CLI
participant Store as MemoryStore
participant BFS as expand_bfs
participant Scoring as wonder_consolidation
participant Output as Output Formatter
User->>CLI: aelf wonder [--seed id] [--emit-phantoms] [--json]
CLI->>Store: load store
CLI->>CLI: pick_seed (deterministic: max degree, tie-break by id)
CLI->>BFS: expand_bfs(seed, top, fanout, budget)
BFS-->>CLI: hop neighbors {belief_id, edges, hop_score}
CLI->>Scoring: score(belief) for each hop
Scoring-->>CLI: consolidation score per candidate
CLI->>CLI: rank by combined score (hop + consolidation)
CLI->>Output: format candidates + optional Phantom rows
Output-->>User: human-readable rows, JSON, or Phantom JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideImplements two new graph-walk CLI surfaces ( Sequence diagram for the new aelf reason CLI graph-walksequenceDiagram
actor User
participant aelf_cli
participant _cmd_reason
participant MemoryStore
participant expand_bfs
User->>aelf_cli: run `aelf reason <query> [flags]`
aelf_cli->>_cmd_reason: dispatch with args, out
_cmd_reason->>MemoryStore: _open_store()
activate MemoryStore
alt has_seed_id
_cmd_reason->>MemoryStore: get_belief(seed_id)
MemoryStore-->>_cmd_reason: Belief or None
alt seed_missing
_cmd_reason-->>User: print error seed-id not found
_cmd_reason->>MemoryStore: close()
_cmd_reason-->>aelf_cli: exit code 2
end
else bm25_seed_search
_cmd_reason->>MemoryStore: search_beliefs(query, limit=k)
MemoryStore-->>_cmd_reason: seeds list
end
alt no_seeds
_cmd_reason-->>User: print no seeds message
_cmd_reason->>MemoryStore: close()
_cmd_reason-->>aelf_cli: exit code 0
else have_seeds
_cmd_reason->>expand_bfs: expand_bfs(seeds, store, depth, fanout, budget)
expand_bfs-->>_cmd_reason: hops
_cmd_reason->>MemoryStore: close()
end
deactivate MemoryStore
alt json_output
_cmd_reason-->>User: print JSON payload {query,seeds,hops}
else tree_output
_cmd_reason-->>User: print seeds and hop tree
end
_cmd_reason-->>aelf_cli: exit code 0
Class diagram for Phantom scaffolding and belief relationshipsclassDiagram
class Belief {
+str id
+str content
+str origin
}
class Phantom {
+tuple~str~ constituent_belief_ids
+str generator
+str content
+float score
}
class OriginConstants {
+str ORIGIN_USER_STATED
+str ORIGIN_AGENT_INFERRED
+str ORIGIN_DOCUMENT_RECENT
+str ORIGIN_AGENT_REMEMBERED
+str ORIGIN_UNKNOWN
+str ORIGIN_SPECULATIVE
+frozenset~str~ ORIGINS
}
class WonderCLI {
+int _cmd_wonder(args, out)
+object _wonder_pick_seed(store)
+str _suggested_action_for(path)
}
class ReasonCLI {
+int _cmd_reason(args, out)
}
class MemoryStore {
+Belief get_belief(belief_id)
+list~Belief~ search_beliefs(query, limit)
+list~str~ list_belief_ids()
+list edges_from(belief_id)
+void close()
}
class WonderConsolidation {
+float score(seed_belief, candidate_belief)
}
Phantom --> Belief : constituent_belief_ids
WonderCLI --> Phantom : creates
WonderCLI --> Belief : reads
ReasonCLI --> Belief : reads
WonderCLI --> MemoryStore : uses
ReasonCLI --> MemoryStore : uses
WonderCLI --> WonderConsolidation : uses
OriginConstants --> Belief : origin_field
OriginConstants --> Phantom : planned_origin_speculative
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| store.close() | ||
|
|
||
| if args.json: | ||
| import json |
| ] | ||
|
|
||
| if args.emit_phantoms: | ||
| import json |
| return 0 | ||
|
|
||
| if args.json: | ||
| import json |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the
_wonder_pick_seed/_cmd_wonderflow, the use ofobject | Noneand repeated# type: ignore[union-attr]suggests the types could be tightened toBelief | None(and the return type of_wonder_pick_seedupdated accordingly) so that downstream attribute access is type-safe without ignores. - The bench-gate helpers
_build_storeintest_reason.pyandtest_wonder_online.pyduplicate Belief/Edge construction logic; consider extracting a shared helper (or reusing existing test fixtures) so future changes to the Belief schema or defaults stay consistent across gates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `_wonder_pick_seed` / `_cmd_wonder` flow, the use of `object | None` and repeated `# type: ignore[union-attr]` suggests the types could be tightened to `Belief | None` (and the return type of `_wonder_pick_seed` updated accordingly) so that downstream attribute access is type-safe without ignores.
- The bench-gate helpers `_build_store` in `test_reason.py` and `test_wonder_online.py` duplicate Belief/Edge construction logic; consider extracting a shared helper (or reusing existing test fixtures) so future changes to the Belief schema or defaults stay consistent across gates.
## Individual Comments
### Comment 1
<location path="src/aelfrice/cli.py" line_range="842-849" />
<code_context>
+}
+
+
+def _suggested_action_for(path: list[str]) -> str:
+ """Map a BFS edge-type path to a one-word suggested action.
+
+ Picks the highest-priority edge type seen on the path, with
+ fall-through to "relate" when none match. Priority order matches
+ `_WONDER_ACTION_BY_EDGE` insertion order.
+ """
+ for edge_type in path:
+ if edge_type in _WONDER_ACTION_BY_EDGE:
+ return _WONDER_ACTION_BY_EDGE[edge_type]
</code_context>
<issue_to_address>
**issue (bug_risk):** Suggested-action priority does not match the docstring description.
The docstring describes choosing the highest-priority edge type based on `_WONDER_ACTION_BY_EDGE` insertion order, but the loop instead returns the first matching edge in `path`. If `path` can contain multiple edge types, this prefers path order over the defined priority. Please either iterate over `_WONDER_ACTION_BY_EDGE` and check membership in `path`, or update the docstring to match the actual behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+842
to
+849
| def _suggested_action_for(path: list[str]) -> str: | ||
| """Map a BFS edge-type path to a one-word suggested action. | ||
|
|
||
| Picks the highest-priority edge type seen on the path, with | ||
| fall-through to "relate" when none match. Priority order matches | ||
| `_WONDER_ACTION_BY_EDGE` insertion order. | ||
| """ | ||
| for edge_type in path: |
There was a problem hiding this comment.
issue (bug_risk): Suggested-action priority does not match the docstring description.
The docstring describes choosing the highest-priority edge type based on _WONDER_ACTION_BY_EDGE insertion order, but the loop instead returns the first matching edge in path. If path can contain multiple edge types, this prefers path order over the defined priority. Please either iterate over _WONDER_ACTION_BY_EDGE and check membership in path, or update the docstring to match the actual behavior.
…lding) Adds the wire-format constant and dataclass for the wonder-generated phantom-belief mechanic. ORIGIN_SPECULATIVE is intentionally NOT yet in the ORIGINS frozenset — store-write integration is deferred to a follow-up issue (#229 promotion-trigger lane). Constant + dataclass land now so 'aelf wonder' (this issue) can produce phantom candidates in-memory and the future integration sub-issue has the scaffolding to wire into.
Two new CLI subcommands shipping together per issue #389 ratification ("both or neither" — no partial-ship surface). aelf reason <query>: BM25 top-k seeds (or --seed-id), expand_bfs walk with terminal-tight defaults (depth=2, budget=10, fanout=8). Indented hop tree by default; --json for machine consumption. Read-only over the graph. aelf wonder: Highest-degree non-locked belief as seed (id-asc tiebreak) or --seed override. expand_bfs + wonder_consolidation.score combined scoring; suggested_action {merge,supersede,contradict,relate} derived from edge-type heuristic on path. --top N controls list length; --emit-phantoms emits Phantom JSON for offline use. Phantom-belief STORE-write integration deferred to v2.x #229 lane per operator amendment 9 — TODO marker in source flags the hookpoint. Implements operator-ratified defaults from issue #389 comment 4372792969 plus amendment 9 (phantom scaffolding additive).
tests/test_cli_reason_wonder.py: 9 in-process atomic tests against
a 3-belief synthetic graph. Covers seed selection (BM25 + --seed-id),
JSON output shape, unknown-id error path, empty-store handling,
deterministic seed pick, --emit-phantoms output, and suggested-action
vocabulary.
src/aelfrice/slash_commands/{reason,wonder}.md: aelf:reason +
aelf:wonder slash command files mirroring the existing search.md
template. Both surfaces are now invokable via slash command.
tests/test_slash_commands.py: registers reason/wonder in
EXPECTED_COMMANDS so the visible-CLI ↔ slash-dir parity test passes.
Full suite green: 2413 passed, 16 skipped.
) Two new bench-gate tests mirroring the test_bfs_multihop_relates_to.py pattern. Both skip cleanly when AELFRICE_CORPUS_ROOT is unset (public CI), when their module dir is empty, or when fewer than MIN_ROWS=20 non-seed rows are present. reasoning/ gate: chain hit@k uplift over baseline_search_only_top_k ≥+3pp (#389 decision-ask 4). wonder_online/ gate: ≥1 expected candidate in top-10 across ≥60% of rows (#389 decision-ask 8). Schema validator (test_corpus_schema.py) registers both new modules with their per-row field specs. Lab corpus content lives under ~/projects/aelfrice-lab/tests/corpus/v2_0/{reasoning,wonder_online}/ per directory-of-origin rules — public tree carries .gitkeep placeholders only.
COMMANDS.md: bump count 26→28 and add Memory-operations rows for both commands. Notes that aelf wonder's phantom-store integration is deferred to v2.x #229 lane. SLASH_COMMANDS.md: bump count 15→17 and add reference rows for /aelf:reason and /aelf:wonder.
robotrocketscience
force-pushed
the
feat/issue-389-reason-wonder-cli
branch
from
May 4, 2026 23:30
bf6e199 to
cf00de3
Compare
yoshi280
enabled auto-merge (rebase)
May 4, 2026 23:30
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/aelfrice/cli.py (1)
789-790:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the three redundant local
import jsonstatements.
jsonis already imported at module level on line 35. The local re-imports inside_cmd_reason(line 790) and_cmd_wonder(lines 947, 961) shadow the module import without adding value. CodeQL flagged all three, and the same finding was raised on prior commits.♻️ Proposed fix
@@ in _cmd_reason - if args.json: - import json - payload = { + if args.json: + payload = {@@ in _cmd_wonder (--emit-phantoms branch) - if args.emit_phantoms: - import json - payload = [ + if args.emit_phantoms: + payload = [@@ in _cmd_wonder (--json branch) - if args.json: - import json - payload2 = { + if args.json: + payload2 = {Also applies to: 946-947, 960-961
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aelfrice/cli.py` around lines 789 - 790, Remove the redundant local "import json" statements that shadow the module-level import; specifically delete the conditional local imports inside the functions/methods handling the commands (the "if args.json: import json" lines found in _cmd_reason and _cmd_wonder) so the code uses the json symbol already imported at module scope (no other changes needed).
🧹 Nitpick comments (6)
src/aelfrice/models.py (1)
133-138: 💤 Low valueTrack the
ORIGIN_SPECULATIVE↔ORIGINSreconciliation explicitly.The comment notes this is deferred to
#229, which is correct. To prevent the constant from drifting silently, consider either (a) adding a# noqa: future-#229style marker that a follow-up PR can grep for, or (b) leaving a briefTODO(#229)next toORIGINSitself so anyone editing that frozenset sees the link. Today, only this side documents the connection — a future contributor adding a new origin toORIGINSwould have no signal thatORIGIN_SPECULATIVEis intentionally missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aelfrice/models.py` around lines 133 - 138, Add an explicit reconciliation marker so the ORIGIN_SPECULATIVE ↔ ORIGINS relationship is discoverable: either annotate the ORIGIN_SPECULATIVE constant with a searchable tag (e.g., "# TODO(`#229`)" or "# noqa: future-#229") and/or add a brief "TODO(`#229`)" comment next to the ORIGINS frozenset declaration so future editors see the linkage; reference ORIGIN_SPECULATIVE and ORIGINS when adding the comment to ensure both places are easily grep-able for follow-up PR `#229`.src/aelfrice/cli.py (2)
855-882: ⚖️ Poor tradeoff
_wonder_pick_seedis O(N)get_beliefround-trips on every wonder invocation.For each belief id,
get_belief(bid)is called once, thenedges_from(bid)again — that's 2N SQLite queries on top oflist_belief_ids(). On a project-scale store (10k+ beliefs) this turns a "wonder" into seconds of latency.If
MemoryStorehas acount_outbound_edges_per_belief()or similar aggregate query (or you can runSELECT b.id, COUNT(e.src) FROM beliefs b LEFT JOIN edges e ON ... WHERE b.lock_level != 'user' GROUP BY b.id), prefer that. Alternatively, add a single SQL helper to do this in one round-trip. Not a blocker for the 10–100 belief test corpora, but worth flagging before this hits a real project.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aelfrice/cli.py` around lines 855 - 882, _wonder_pick_seed currently calls store.get_belief(bid) and store.edges_from(bid) for every belief id, causing 2N DB round-trips; replace that hot loop with a single aggregated query or helper on MemoryStore that returns non-locked belief ids and their outbound edge counts in one call (for example count_outbound_edges_per_belief or a method that returns (id, degree) pairs), then pick the max-degree/lowest-id tie from that result and finally call get_belief only once for the chosen id (or return the pre-fetched belief object if the helper returns it) to reduce round-trips to one.
915-915: ⚡ Quick win
total_budget=args.top * 2may surface fewer than--topcandidates.
expand_bfscaps total expanded nodes attotal_budget; for--top 10that's 20 nodes across all hops, which can easily under-deliver on dense graphs (e.g., when fanout produces many depth-1 hits with low path-scores that displace the depth-2 hits the user is asking for) and under-deliver on sparse graphs (visited-set + cycle pruning eats the budget). The user expects--top Nto actually return up to N rows; the implementation can silently truncate.Consider scaling more generously (e.g.,
total_budget=max(args.top * 4, 32)), or honoring the same--budgetflag thereasoncommand exposes so operators can tune. At minimum, add a unit test that pins the "we get back ≤ args.top, but typically ≈ args.top on a normal graph" expectation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aelfrice/cli.py` at line 915, The current call to expand_bfs(seed_b, store, max_depth=2, total_budget=args.top * 2) can under-deliver results; change the total_budget calculation to a more generous or configurable value (for example total_budget=max(args.top * 4, 32) or wire it to the same --budget/args.budget flag used by the reason command) so expand_bfs can return up to args.top candidates reliably; update the call site where hops = expand_bfs([seed_b], store, max_depth=2, total_budget=...) and add a unit test that simulates dense and sparse graphs asserting the command returns ≤ args.top but typically ≈ args.top (and verify the new budget prevents silent truncation).tests/corpus/v2_0/README.md (1)
85-86: ⚡ Quick winMissing ship-gate documentation sections for
reasoningandwonder_online.Every other module documented in this README has a dedicated subsection explaining the ship gate, threshold, per-row shape, and aggregation rule (e.g.,
### tests_edge ship gate (#384)). The newreasoningandwonder_onlinemodules add only the schema-table rows at lines 85-86 — readers don't see:
- The bench-gate thresholds (per the PR: ≥+3pp hit@k uplift for
aelf reason; ≥60% row-recall@10 foraelf wonder).- The aggregation formula (
baseline_search_only_top_kis a comparison set — its semantics aren't defined anywhere).- Skip-on-empty-corpus behavior for the bench-gate harnesses at
tests/bench_gate/test_reason.pyandtests/bench_gate/test_wonder_online.py.Add subsections analogous to the existing ones so the corpus contract is self-contained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/corpus/v2_0/README.md` around lines 85 - 86, Add missing ship-gate subsections for the new modules reasoning and wonder_online: create two subsections (e.g., "### tests_reasoning ship gate" and "### tests_wonder_online ship gate") that document the bench-gate thresholds (reasoning: ≥+3pp hit@k uplift for "aelf reason"; wonder_online: ≥60% row-recall@10 for "aelf wonder"), the per-row input/output shape (fields: reasoning uses query, beliefs, edges, expected_hit_ids, baseline_search_only_top_k, k; wonder_online uses beliefs, edges, seed_id, expected_candidate_ids), the aggregation rule clarifying baseline_search_only_top_k semantics (it's a comparison set used to compute uplift against baseline_search_only_top_k), and the skip-on-empty-corpus behavior consistent with tests/bench_gate/test_reason.py and tests/bench_gate/test_wonder_online.py so the corpus README is self-contained.tests/bench_gate/test_wonder_online.py (1)
58-72: 🏗️ Heavy liftPrefer a shared wonder-ranking helper over re-implementing scoring in the gate.
Line 58-Line 72 duplicates the
_cmd_wonderranking math in test code. Centralizing the ranking path avoids future divergence between ship behavior and bench-gate measurement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/bench_gate/test_wonder_online.py` around lines 58 - 72, The test duplicates the ranking math from _cmd_wonder in _row_top_k_candidates; extract the combined scoring + ranking into a single shared helper (e.g., aelfrice.wonder_consolidation.rank_or_get_top_k) and have _row_top_k_candidates call that helper instead of re-implementing the logic; update callers (_cmd_wonder and this test) to import and use the new helper so scoring uses wonder_consolidation.score and expand_bfs only in one place, returning the same set[str] top-k ids.tests/bench_gate/test_reason.py (1)
61-63: 🏗️ Heavy liftReduce gate/CLI drift by sharing reason-surface defaults and ranking path.
Line 61-Line 63 hardcode seed-limit/expansion behavior locally. If CLI defaults evolve, this gate can silently validate a different algorithm than the shipped command. Consider routing both CLI and bench gate through one shared helper/config source.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/bench_gate/test_reason.py` around lines 61 - 63, The test hardcodes seed/expansion parameters (calls to store.search_beliefs and expand_bfs and subsequent surfaced_ids assembly) which can drift from the CLI; refactor the test to consume the shared reason-surface defaults and ranking path used by the CLI (e.g., a single helper or config provider) instead of literal limit/expansion behavior — replace direct calls to store.search_beliefs(...) and expand_bfs(...) with calls to that shared helper (or import the CLI's defaults/ranking function) so the test uses the same parameters and surface-ordering logic as the shipped command.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aelfrice/cli.py`:
- Around line 922-927: The blending formula currently computes combined =
h.score * (0.5 + 0.5 * relatedness) so relatedness can only attenuate h.score;
update the code to either (A) explicitly document this as a "path-score with
relatedness penalty" in the surrounding docstring/comment near combined and keep
the formula, or (B) change the blending to actually let both signals contribute
(e.g., use an additive mix like 0.5*h.score + 0.5*relatedness or a geometric
mean like math.sqrt(h.score * relatedness)) so relatedness can boost candidates;
locate the combined calculation and _suggested_action_for(...) call and apply
the chosen change, and ensure candidates.append still receives (combined, h,
action, relatedness).
- Around line 842-852: The docstring for _suggested_action_for incorrectly
states that priority follows _WONDER_ACTION_BY_EDGE insertion order while the
implementation actually uses path order; either update the docstring to state
"first matching edge in the path wins" (e.g., "priority is path-order: the first
decisional edge encountered") or change the implementation to respect dict
insertion priority by iterating _WONDER_ACTION_BY_EDGE keys and returning the
first edge_type present in the path; reference the function
_suggested_action_for and the mapping _WONDER_ACTION_BY_EDGE when making the
change.
- Around line 982-987: The loop unpacks four values from candidates into
(combined, h, action, relatedness) but never uses relatedness, triggering Ruff
B007; update the unpacking in the for loop that iterates over candidates to use
an unused-name (e.g., _relatedness or _) instead of relatedness so the intent is
clear and the linter is satisfied — modify the for statement that currently
reads for combined, h, action, relatedness in candidates: to use the unused
variable name.
- Around line 746-787: The seed-id error prints to the success output stream;
change the error prints in the seed-not-found branches to write to sys.stderr
instead of using the out parameter so error messages are kept off stdout/JSON;
update the seed-id failure handling in _cmd_reason (the loop that calls
store.get_belief and prints "aelf reason: seed-id not found: {sid}") to print to
sys.stderr and keep the return code 2, and make the same adjustment in the
analogous seed-not-found branch in _cmd_wonder so both handlers consistently
emit errors on stderr.
---
Duplicate comments:
In `@src/aelfrice/cli.py`:
- Around line 789-790: Remove the redundant local "import json" statements that
shadow the module-level import; specifically delete the conditional local
imports inside the functions/methods handling the commands (the "if args.json:
import json" lines found in _cmd_reason and _cmd_wonder) so the code uses the
json symbol already imported at module scope (no other changes needed).
---
Nitpick comments:
In `@src/aelfrice/cli.py`:
- Around line 855-882: _wonder_pick_seed currently calls store.get_belief(bid)
and store.edges_from(bid) for every belief id, causing 2N DB round-trips;
replace that hot loop with a single aggregated query or helper on MemoryStore
that returns non-locked belief ids and their outbound edge counts in one call
(for example count_outbound_edges_per_belief or a method that returns (id,
degree) pairs), then pick the max-degree/lowest-id tie from that result and
finally call get_belief only once for the chosen id (or return the pre-fetched
belief object if the helper returns it) to reduce round-trips to one.
- Line 915: The current call to expand_bfs(seed_b, store, max_depth=2,
total_budget=args.top * 2) can under-deliver results; change the total_budget
calculation to a more generous or configurable value (for example
total_budget=max(args.top * 4, 32) or wire it to the same --budget/args.budget
flag used by the reason command) so expand_bfs can return up to args.top
candidates reliably; update the call site where hops = expand_bfs([seed_b],
store, max_depth=2, total_budget=...) and add a unit test that simulates dense
and sparse graphs asserting the command returns ≤ args.top but typically ≈
args.top (and verify the new budget prevents silent truncation).
In `@src/aelfrice/models.py`:
- Around line 133-138: Add an explicit reconciliation marker so the
ORIGIN_SPECULATIVE ↔ ORIGINS relationship is discoverable: either annotate the
ORIGIN_SPECULATIVE constant with a searchable tag (e.g., "# TODO(`#229`)" or "#
noqa: future-#229") and/or add a brief "TODO(`#229`)" comment next to the ORIGINS
frozenset declaration so future editors see the linkage; reference
ORIGIN_SPECULATIVE and ORIGINS when adding the comment to ensure both places are
easily grep-able for follow-up PR `#229`.
In `@tests/bench_gate/test_reason.py`:
- Around line 61-63: The test hardcodes seed/expansion parameters (calls to
store.search_beliefs and expand_bfs and subsequent surfaced_ids assembly) which
can drift from the CLI; refactor the test to consume the shared reason-surface
defaults and ranking path used by the CLI (e.g., a single helper or config
provider) instead of literal limit/expansion behavior — replace direct calls to
store.search_beliefs(...) and expand_bfs(...) with calls to that shared helper
(or import the CLI's defaults/ranking function) so the test uses the same
parameters and surface-ordering logic as the shipped command.
In `@tests/bench_gate/test_wonder_online.py`:
- Around line 58-72: The test duplicates the ranking math from _cmd_wonder in
_row_top_k_candidates; extract the combined scoring + ranking into a single
shared helper (e.g., aelfrice.wonder_consolidation.rank_or_get_top_k) and have
_row_top_k_candidates call that helper instead of re-implementing the logic;
update callers (_cmd_wonder and this test) to import and use the new helper so
scoring uses wonder_consolidation.score and expand_bfs only in one place,
returning the same set[str] top-k ids.
In `@tests/corpus/v2_0/README.md`:
- Around line 85-86: Add missing ship-gate subsections for the new modules
reasoning and wonder_online: create two subsections (e.g., "### tests_reasoning
ship gate" and "### tests_wonder_online ship gate") that document the bench-gate
thresholds (reasoning: ≥+3pp hit@k uplift for "aelf reason"; wonder_online: ≥60%
row-recall@10 for "aelf wonder"), the per-row input/output shape (fields:
reasoning uses query, beliefs, edges, expected_hit_ids,
baseline_search_only_top_k, k; wonder_online uses beliefs, edges, seed_id,
expected_candidate_ids), the aggregation rule clarifying
baseline_search_only_top_k semantics (it's a comparison set used to compute
uplift against baseline_search_only_top_k), and the skip-on-empty-corpus
behavior consistent with tests/bench_gate/test_reason.py and
tests/bench_gate/test_wonder_online.py so the corpus README is self-contained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1dcb0bdc-bf21-460c-b668-d90ee5f886ee
📒 Files selected for processing (14)
docs/COMMANDS.mddocs/SLASH_COMMANDS.mdsrc/aelfrice/cli.pysrc/aelfrice/models.pysrc/aelfrice/slash_commands/reason.mdsrc/aelfrice/slash_commands/wonder.mdtests/bench_gate/test_reason.pytests/bench_gate/test_wonder_online.pytests/corpus/v2_0/README.mdtests/corpus/v2_0/reasoning/.gitkeeptests/corpus/v2_0/wonder_online/.gitkeeptests/test_cli_reason_wonder.pytests/test_corpus_schema.pytests/test_slash_commands.py
Comment on lines
+746
to
+787
| def _cmd_reason(args: argparse.Namespace, out: object) -> int: | ||
| """Surface a reasoning chain over the belief graph for a query. | ||
|
|
||
| Seeds: explicit `--seed-id` (repeatable) wins; otherwise top-k | ||
| `search_beliefs` BM25 hits over `args.query`. Walks `expand_bfs` | ||
| from those seeds with terminal-tight defaults and prints either | ||
| an indented hop tree (default) or JSON when `--json`. | ||
|
|
||
| Read-only: never writes to the store. | ||
| """ | ||
| store = _open_store() | ||
| try: | ||
| seeds: list = [] | ||
| if args.seed_id: | ||
| for sid in args.seed_id: | ||
| b = store.get_belief(sid) | ||
| if b is None: | ||
| print( | ||
| f"aelf reason: seed-id not found: {sid}", | ||
| file=out, # type: ignore[arg-type] | ||
| ) | ||
| return 2 | ||
| seeds.append(b) | ||
| else: | ||
| seeds = store.search_beliefs(args.query, limit=args.k) | ||
| if not seeds: | ||
| print( | ||
| "aelf reason: no seeds (empty store, or query didn't " | ||
| "match any indexed belief). Try --seed-id <id> to " | ||
| "force a starting point.", | ||
| file=out, # type: ignore[arg-type] | ||
| ) | ||
| return 0 | ||
| hops = expand_bfs( | ||
| seeds, | ||
| store, | ||
| max_depth=args.depth, | ||
| nodes_per_hop=args.fanout, | ||
| total_budget=args.budget, | ||
| ) | ||
| finally: | ||
| store.close() |
There was a problem hiding this comment.
Reason handler logic LGTM, with one robustness nit on --seed-id exit semantics.
--seed-id failure on line 762-767 prints to out (stdout) but returns exit 2. Other CLI handlers in this file (e.g., _cmd_demote line 1113, _cmd_uninstall line 1834) emit error messages on sys.stderr and reserve out for successful payloads. Inconsistency means a JSON-consuming caller using --json could get an error string mixed into stdout when one seed-id is bogus.
♻️ Proposed fix
- print(
- f"aelf reason: seed-id not found: {sid}",
- file=out, # type: ignore[arg-type]
- )
+ print(
+ f"aelf reason: seed-id not found: {sid}",
+ file=sys.stderr,
+ )
return 2Same fix applies to the _cmd_wonder "seed not found" branch at line 901-904.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _cmd_reason(args: argparse.Namespace, out: object) -> int: | |
| """Surface a reasoning chain over the belief graph for a query. | |
| Seeds: explicit `--seed-id` (repeatable) wins; otherwise top-k | |
| `search_beliefs` BM25 hits over `args.query`. Walks `expand_bfs` | |
| from those seeds with terminal-tight defaults and prints either | |
| an indented hop tree (default) or JSON when `--json`. | |
| Read-only: never writes to the store. | |
| """ | |
| store = _open_store() | |
| try: | |
| seeds: list = [] | |
| if args.seed_id: | |
| for sid in args.seed_id: | |
| b = store.get_belief(sid) | |
| if b is None: | |
| print( | |
| f"aelf reason: seed-id not found: {sid}", | |
| file=out, # type: ignore[arg-type] | |
| ) | |
| return 2 | |
| seeds.append(b) | |
| else: | |
| seeds = store.search_beliefs(args.query, limit=args.k) | |
| if not seeds: | |
| print( | |
| "aelf reason: no seeds (empty store, or query didn't " | |
| "match any indexed belief). Try --seed-id <id> to " | |
| "force a starting point.", | |
| file=out, # type: ignore[arg-type] | |
| ) | |
| return 0 | |
| hops = expand_bfs( | |
| seeds, | |
| store, | |
| max_depth=args.depth, | |
| nodes_per_hop=args.fanout, | |
| total_budget=args.budget, | |
| ) | |
| finally: | |
| store.close() | |
| def _cmd_reason(args: argparse.Namespace, out: object) -> int: | |
| """Surface a reasoning chain over the belief graph for a query. | |
| Seeds: explicit `--seed-id` (repeatable) wins; otherwise top-k | |
| `search_beliefs` BM25 hits over `args.query`. Walks `expand_bfs` | |
| from those seeds with terminal-tight defaults and prints either | |
| an indented hop tree (default) or JSON when `--json`. | |
| Read-only: never writes to the store. | |
| """ | |
| store = _open_store() | |
| try: | |
| seeds: list = [] | |
| if args.seed_id: | |
| for sid in args.seed_id: | |
| b = store.get_belief(sid) | |
| if b is None: | |
| print( | |
| f"aelf reason: seed-id not found: {sid}", | |
| file=sys.stderr, | |
| ) | |
| return 2 | |
| seeds.append(b) | |
| else: | |
| seeds = store.search_beliefs(args.query, limit=args.k) | |
| if not seeds: | |
| print( | |
| "aelf reason: no seeds (empty store, or query didn't " | |
| "match any indexed belief). Try --seed-id <id> to " | |
| "force a starting point.", | |
| file=out, # type: ignore[arg-type] | |
| ) | |
| return 0 | |
| hops = expand_bfs( | |
| seeds, | |
| store, | |
| max_depth=args.depth, | |
| nodes_per_hop=args.fanout, | |
| total_budget=args.budget, | |
| ) | |
| finally: | |
| store.close() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aelfrice/cli.py` around lines 746 - 787, The seed-id error prints to the
success output stream; change the error prints in the seed-not-found branches to
write to sys.stderr instead of using the out parameter so error messages are
kept off stdout/JSON; update the seed-id failure handling in _cmd_reason (the
loop that calls store.get_belief and prints "aelf reason: seed-id not found:
{sid}") to print to sys.stderr and keep the return code 2, and make the same
adjustment in the analogous seed-not-found branch in _cmd_wonder so both
handlers consistently emit errors on stderr.
Comment on lines
+842
to
+852
| def _suggested_action_for(path: list[str]) -> str: | ||
| """Map a BFS edge-type path to a one-word suggested action. | ||
|
|
||
| Picks the highest-priority edge type seen on the path, with | ||
| fall-through to "relate" when none match. Priority order matches | ||
| `_WONDER_ACTION_BY_EDGE` insertion order. | ||
| """ | ||
| for edge_type in path: | ||
| if edge_type in _WONDER_ACTION_BY_EDGE: | ||
| return _WONDER_ACTION_BY_EDGE[edge_type] | ||
| return "relate" |
There was a problem hiding this comment.
_suggested_action_for priority is path-order, not the documented dict-insertion order.
The docstring claims "Picks the highest-priority edge type seen on the path, with fall-through to 'relate'. Priority order matches _WONDER_ACTION_BY_EDGE insertion order." But the loop returns on the first edge type in path that's in the dict — so for a path [EDGE_SUPPORTS, EDGE_SUPERSEDES] the result is "merge" (path-order), even though EDGE_SUPERSEDES has higher dict-insertion priority.
If path-order is intended (e.g., "the first decisional edge encountered while walking out from the seed wins"), reword the docstring. If dict-priority was intended, walk the dict instead:
♻️ Proposed fix (dict-priority semantics)
def _suggested_action_for(path: list[str]) -> str:
- """Map a BFS edge-type path to a one-word suggested action.
-
- Picks the highest-priority edge type seen on the path, with
- fall-through to "relate" when none match. Priority order matches
- `_WONDER_ACTION_BY_EDGE` insertion order.
- """
- for edge_type in path:
- if edge_type in _WONDER_ACTION_BY_EDGE:
- return _WONDER_ACTION_BY_EDGE[edge_type]
- return "relate"
+ """Map a BFS edge-type path to a one-word suggested action.
+
+ Picks the highest-priority edge type present on the path, with
+ fall-through to "relate". Priority follows `_WONDER_ACTION_BY_EDGE`
+ insertion order (SUPERSEDES > CONTRADICTS > SUPPORTS).
+ """
+ seen = set(path)
+ for edge_type, action in _WONDER_ACTION_BY_EDGE.items():
+ if edge_type in seen:
+ return action
+ return "relate"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aelfrice/cli.py` around lines 842 - 852, The docstring for
_suggested_action_for incorrectly states that priority follows
_WONDER_ACTION_BY_EDGE insertion order while the implementation actually uses
path order; either update the docstring to state "first matching edge in the
path wins" (e.g., "priority is path-order: the first decisional edge
encountered") or change the implementation to respect dict insertion priority by
iterating _WONDER_ACTION_BY_EDGE keys and returning the first edge_type present
in the path; reference the function _suggested_action_for and the mapping
_WONDER_ACTION_BY_EDGE when making the change.
Comment on lines
+922
to
+927
| # Combine BFS path-score with token-overlap relatedness. | ||
| # Multiplicative so both signals must be non-trivial for a | ||
| # candidate to rank high. | ||
| combined = h.score * (0.5 + 0.5 * relatedness) | ||
| action = _suggested_action_for(h.path) | ||
| candidates.append((combined, h, action, relatedness)) |
There was a problem hiding this comment.
Relatedness can only attenuate combined, never boost it — clarify the docstring or rebalance the formula.
combined = h.score * (0.5 + 0.5 * relatedness) produces a multiplier in [0.5, 1.0], so a high token-overlap relatedness yields at best h.score, while a low relatedness halves the score. The accompanying comment ("Multiplicative so both signals must be non-trivial for a candidate to rank high") reads as if both signals are co-equal, but in practice BFS path-score dominates and relatedness only acts as a discount.
If the intent was to actually require both, an additive blend (0.5*h.score + 0.5*relatedness) or a geometric mean (sqrt(h.score * relatedness)) would let either signal dominate. If the intent really is "BFS path-score with a relatedness penalty", the docstring should say that explicitly so future tuners don't try to "fix" what looks like a bug.
This is also relevant to the wonder bench gate (≥60% row-recall@10) — if the corpus run misses the threshold, this formula is the first knob to revisit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aelfrice/cli.py` around lines 922 - 927, The blending formula currently
computes combined = h.score * (0.5 + 0.5 * relatedness) so relatedness can only
attenuate h.score; update the code to either (A) explicitly document this as a
"path-score with relatedness penalty" in the surrounding docstring/comment near
combined and keep the formula, or (B) change the blending to actually let both
signals contribute (e.g., use an additive mix like 0.5*h.score + 0.5*relatedness
or a geometric mean like math.sqrt(h.score * relatedness)) so relatedness can
boost candidates; locate the combined calculation and _suggested_action_for(...)
call and apply the chosen change, and ensure candidates.append still receives
(combined, h, action, relatedness).
Comment on lines
+982
to
+987
| print(f"top {len(candidates)} consolidation candidate(s):", file=out) # type: ignore[arg-type] | ||
| for combined, h, action, relatedness in candidates: | ||
| print( | ||
| f" [{combined:.3f}] ({action}) {h.belief.id}: {h.belief.content}", | ||
| file=out, # type: ignore[arg-type] | ||
| ) |
There was a problem hiding this comment.
Unused loop variable relatedness (Ruff B007).
The unpacking on line 983 binds relatedness, but it's not referenced inside the loop body — only combined, h, and action are printed. Rename to _relatedness (or _) to silence Ruff and signal intent.
♻️ Proposed fix
- for combined, h, action, relatedness in candidates:
+ for combined, h, action, _relatedness in candidates:
print(
f" [{combined:.3f}] ({action}) {h.belief.id}: {h.belief.content}",
file=out, # type: ignore[arg-type]
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| print(f"top {len(candidates)} consolidation candidate(s):", file=out) # type: ignore[arg-type] | |
| for combined, h, action, relatedness in candidates: | |
| print( | |
| f" [{combined:.3f}] ({action}) {h.belief.id}: {h.belief.content}", | |
| file=out, # type: ignore[arg-type] | |
| ) | |
| print(f"top {len(candidates)} consolidation candidate(s):", file=out) # type: ignore[arg-type] | |
| for combined, h, action, _relatedness in candidates: | |
| print( | |
| f" [{combined:.3f}] ({action}) {h.belief.id}: {h.belief.content}", | |
| file=out, # type: ignore[arg-type] | |
| ) |
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 983-983: Loop control variable relatedness not used within loop body
(B007)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aelfrice/cli.py` around lines 982 - 987, The loop unpacks four values
from candidates into (combined, h, action, relatedness) but never uses
relatedness, triggering Ruff B007; update the unpacking in the for loop that
iterates over candidates to use an unused-name (e.g., _relatedness or _) instead
of relatedness so the intent is clear and the linter is satisfied — modify the
for statement that currently reads for combined, h, action, relatedness in
candidates: to use the unused variable name.
Merged
8 tasks
This was referenced Jul 29, 2026
Merged
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
3 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
Closes #389 — Track B v2.0 sub-issue: re-introduces
aelf reasonandaelf wonderCLI surfaces, shipping together per umbrella ratification ("both or neither").Implementation matches the operator-ratified defaults in #389 comment 4372792969 and amendment 9 (phantom-belief scaffolding additive).
What ships
aelf reason <query>— BM25 top-3 seeds (or--seed-id),expand_bfswalk with terminal-tight defaults (depth=2, budget=10, fanout=8). Indented hop-tree output by default;--jsonfor tooling. Read-only.aelf wonder— highest-degree non-locked seed (id-asc tiebreak) or--seed. Combined BFS path-score ×wonder_consolidation.scoreranking. Suggested actions in{merge, supersede, contradict, relate}from edge-type heuristic.--top Ncontrols list length;--emit-phantomsemitsPhantomJSON for offline review;--jsontoggles output format.ORIGIN_SPECULATIVEconstant +Phantomdataclass + amendment-9 store-write integration TODO marker. NOT inORIGINSvalidation set yet — phantom-store integration ships in a follow-up issue under the v2.x [v2.0] Phantom promotion-trigger rule — three rejected naive triggers, need a benchmarked rule #229 lane./aelf:reasonand/aelf:wonder.tests/bench_gate/test_reason.py+test_wonder_online.py, both skip-on-no-corpus. Skipped on public CI; lab-side run withAELFRICE_CORPUS_ROOTset produces ship-decision evidence.reasoning/andwonder_online/registered intests/test_corpus_schema.pyandtests/corpus/v2_0/README.md. Public tree carries.gitkeepplaceholders only.docs/COMMANDS.mdanddocs/SLASH_COMMANDS.mdupdated.Bench gates (must clear before ship-decision can flip to merge)
aelf reasonchain hit@k uplift over baselinetests/bench_gate/test_reason.pyaelf wonderrow-recall@10 (≥1 expected candidate per row)tests/bench_gate/test_wonder_online.pyThis PR opens with
bench-gatedflagged. Lab-side corpus rows under~/projects/aelfrice-lab/tests/corpus/v2_0/{reasoning,wonder_online}/will run the gates and produce the ship-or-skip evidence; numbers will be posted as a follow-up comment on this PR.If either gate misses: per #389, the surface does not ship; commits revert via
git revert.Out of scope
aelf wonderproducesPhantomobjects in-memory only. Insertion viaBelief(..., origin=ORIGIN_SPECULATIVE),wonder_ingestcorroboration row,wonder_gccleanup, and promotion-trigger wiring all defer to a follow-up issue ([v2.0] Phantom promotion-trigger rule — three rejected naive triggers, need a benchmarked rule #229 lane).Test plan
uv run pytest --ignore=tests/bench_gatepasses (2413 passed, 18 skipped).uv run aelf reason --helpanduv run aelf wonder --helpparse and document the flags.tests/test_cli_reason_wonder.py(9 tests) cover seed selection, JSON output, unknown-id error path, empty-store handling, deterministic seed pick,--emit-phantoms, and suggested-action vocabulary.tests/test_slash_commands.py) green withreason/wonderregistered.tests/test_corpus_schema.py) green with new module specs.Summary by Sourcery
Introduce new graph-walk reasoning and consolidation surfaces in the CLI and slash commands, backed by phantom-belief scaffolding and bench-gated corpus modules for Track B v2.0.
New Features:
aelf reasonCLI and/aelf:reasonslash command to surface reasoning chains over the belief graph with JSON output support.aelf wonderCLI and/aelf:wonderslash command to surface consolidation candidates and emit Phantom JSON for offline review.Phantomdataclass andORIGIN_SPECULATIVEorigin constant for future store integration.Enhancements:
reasoningandwonder_onlinemodules for graph-walk evaluation.aelf reasonhit@k uplift andaelf wonderonline recall to gate shipping of the new surfaces.reasonandwondercommands and their behavior.reasonandwonderentries.Tests:
aelf reasonandaelf wonderCLI behavior, including JSON output, error paths, deterministic seed selection, and phantom emission.Summary by CodeRabbit
New Features
reasoncommand to surface reasoning chains over belief graphs with configurable depth and seed selectionwondercommand to identify consolidation candidates with optional speculative belief suggestionsDocumentation
Tests