docs(feature-aelf-core): spec memo for #439 - #456
Merged
Conversation
Reviewer's GuideAdds a full implementation spec for the planned Sequence diagram for the aelf core CLI execution pathsequenceDiagram
actor Operator
participant AelfCLI as aelf_core_CLI
participant Store as BeliefStore
participant Emitter as CoreEmitter
Operator->>AelfCLI: run aelf core [flags]
AelfCLI->>Store: _open_store()
activate Store
alt no_locked is false
AelfCLI->>Store: list_locked_beliefs()
Store-->>AelfCLI: locked_beliefs
else no_locked is true
AelfCLI-->>AelfCLI: locked_beliefs = []
end
AelfCLI-->>AelfCLI: candidates = []
alt locked_only is false
AelfCLI->>Store: list_belief_ids()
Store-->>AelfCLI: belief_id_list
loop for each belief_id
AelfCLI->>Store: get_belief(belief_id)
Store-->>AelfCLI: belief
alt belief is None or belief.lock_level != none
AelfCLI-->>AelfCLI: skip (locked handled above)
else belief qualifies
AelfCLI-->>AelfCLI: add to candidates
end
end
else locked_only is true
AelfCLI-->>AelfCLI: skip candidate walk
end
AelfCLI->>Store: close()
deactivate Store
AelfCLI->>Emitter: _emit(locked_beliefs, candidates, args, out)
activate Emitter
Emitter-->>Emitter: dedupe by id, sort, apply limit
alt json flag set
Emitter-->>Operator: JSON list with signals
else text output
Emitter-->>Operator: formatted text lines
end
deactivate Emitter
Flow diagram for qualifying a belief as coreflowchart TD
Start([Start]) --> CheckLocked{lock_level != none?}
CheckLocked -->|yes| Core[Mark as core via lock signal]
CheckLocked -->|no| CheckCorr{corroboration_count >= MIN_CORROBORATION?}
CheckCorr -->|yes| Core
CheckCorr -->|no| CheckPosterior{posterior_mean >= MIN_POSTERIOR?}
CheckPosterior -->|no| NotCore[Belief is not core]
CheckPosterior -->|yes| CheckAlphaBeta{alpha + beta >= MIN_ALPHA_BETA?}
CheckAlphaBeta -->|yes| Core
CheckAlphaBeta -->|no| NotCore
Core --> End([End])
NotCore --> End
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA comprehensive feature specification document for the aelf core CLI is added, detailing the purpose of exposing load-bearing beliefs, signal thresholds and posterior computation, command contract including flags and output formats (text and JSON), implementation flow, and test plan sketches. Changesaelf Core Feature Specification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![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:
- The semantics of "disabling" the numeric thresholds are a bit ambiguous: with
MIN_CORROBORATIONdefined ascorroboration_count >= MIN_CORROBORATION, setting--min-corroboration 0would make the signal always fire rather than disable it; consider spelling out the exact interpretation of 0 for each flag (e.g., 0 means the signal is never considered vs. always passes) and how that interacts with thecoredefinition. - For the posterior signal, it would help to be explicit about precedence when
--min-posteriorand--min-alpha-betaare set inconsistently (e.g.,--min-posterior 0.0 --min-alpha-beta 4): document whether either being at the "disabled" value disables the whole posterior signal or whether both must pass for the signal to count.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The semantics of "disabling" the numeric thresholds are a bit ambiguous: with `MIN_CORROBORATION` defined as `corroboration_count >= MIN_CORROBORATION`, setting `--min-corroboration 0` would make the signal always fire rather than disable it; consider spelling out the exact interpretation of 0 for each flag (e.g., 0 means the signal is never considered vs. always passes) and how that interacts with the `core` definition.
- For the posterior signal, it would help to be explicit about precedence when `--min-posterior` and `--min-alpha-beta` are set inconsistently (e.g., `--min-posterior 0.0 --min-alpha-beta 4`): document whether either being at the "disabled" value disables the whole posterior signal or whether both must pass for the signal to count.
## Individual Comments
### Comment 1
<location path="docs/feature-aelf-core.md" line_range="87" />
<code_context>
+| `--limit N` | none | Cap result count after filtering / sort. |
+| `--min-corroboration N` | `2` | Threshold for the corroboration signal. `0` disables. |
+| `--min-posterior FLOAT` | `0.6666...` | Threshold for posterior-mean signal. `0.0` disables. |
+| `--min-alpha-beta N` | `4` | Co-gate on posterior signal — rules out single-event case. |
+| `--locked-only` | off | Equivalent to setting both other signals to "disabled". |
+| `--no-locked` | off | Suppress the locked subset (debug — surface only the corroboration / posterior subsets). |
</code_context>
<issue_to_address>
**nitpick (typo):** Minor grammar nit: consider "rules out the single-event case"
Consider adding "the" before "single-event case" for smoother wording: `Co-gate on posterior signal — rules out the single-event case.`
```suggestion
| `--min-alpha-beta N` | `4` | Co-gate on posterior signal — rules out the single-event case. |
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| | `--limit N` | none | Cap result count after filtering / sort. | | ||
| | `--min-corroboration N` | `2` | Threshold for the corroboration signal. `0` disables. | | ||
| | `--min-posterior FLOAT` | `0.6666...` | Threshold for posterior-mean signal. `0.0` disables. | | ||
| | `--min-alpha-beta N` | `4` | Co-gate on posterior signal — rules out single-event case. | |
There was a problem hiding this comment.
nitpick (typo): Minor grammar nit: consider "rules out the single-event case"
Consider adding "the" before "single-event case" for smoother wording: Co-gate on posterior signal — rules out the single-event case.
Suggested change
| | `--min-alpha-beta N` | `4` | Co-gate on posterior signal — rules out single-event case. | | |
| | `--min-alpha-beta N` | `4` | Co-gate on posterior signal — rules out the single-event case. | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
docs/feature-aelf-core.md (4)
71-79: ⚡ Quick winAdd language specifier to fenced code block.
The CLI usage example should specify a language for proper syntax highlighting and markdown compliance.
📝 Suggested fix
-``` +```bash aelf core [--json] [--limit N]As per coding guidelines, static analysis tool markdownlint-cli2 reports: "Fenced code blocks should have a language specified (MD040)".
🤖 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 `@docs/feature-aelf-core.md` around lines 71 - 79, The fenced code block showing the CLI usage lacks a language specifier; update the block that contains the aelf core usage (the lines starting with "aelf core [--json]" through "--no-locked") to include a language tag (e.g., "bash") on the opening fence so the block becomes a fenced bash code block for proper syntax highlighting and to satisfy MD040.
108-118: ⚡ Quick winAdd language specifier to text output example.
The example output block should specify a language for proper markdown rendering.
📝 Suggested fix
-``` +```text <belief-id> [LOCK,CORR=3,α=4.0,β=1.0,μ=0.800]: <content one-line></details> As per coding guidelines, static analysis tool markdownlint-cli2 reports: "Fenced code blocks should have a language specified (MD040)". <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/feature-aelf-core.mdaround lines 108 - 118, The fenced code block
showing the belief example lacks a language tag; update the block delimiter
around " [LOCK,CORR=3,α=4.0,β=1.0,μ=0.800]: " to
include a language (e.g., changetotext) so markdownlint MD040 is
satisfied and the example renders correctly; ensure the surrounding example and
any references to the tag fields (LOCK,CORR,α,β,μ) remain
unchanged.</details> --- `208-210`: _⚡ Quick win_ **Add language specifier to doc update example.** The markdown table example should have a language tag for consistency. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```markdown | `core [--json] [--limit N] [--min-corroboration N] [--min-posterior FLOAT] [--min-alpha-beta N] [--locked-only] [--no-locked]` | (v2.0+, `#439`) Surface load-bearing beliefs: locked ∪ {corroboration ≥ 2} ∪ {posterior ≥ 2/3 with α+β ≥ 4}. Read-only. | ``` ``` </details> As per coding guidelines, static analysis tool markdownlint-cli2 reports: "Fenced code blocks should have a language specified (MD040)". <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/feature-aelf-core.mdaround lines 208 - 210, Add a language specifier
to the fenced code block containing the command/table example so markdownlint
MD040 is satisfied: update the triple-backtick fence that currently precedes the
line withcore [--json] [--limit N] ...to include a language tag (e.g.,
markdown) and keep the closingunchanged; this change is in the
docs/feature-aelf-core.md example block.</details> --- `119-125`: _⚡ Quick win_ **Add language specifier to empty-store output example.** The example should specify a language for consistency with markdown best practices. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text no core beliefs ``` ``` </details> As per coding guidelines, static analysis tool markdownlint-cli2 reports: "Fenced code blocks should have a language specified (MD040)". <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/feature-aelf-core.mdaround lines 119 - 125, Update the fenced code
block shown under "Empty store / no matches" (the block containing "no core
beliefs") to include a language specifier (e.g., add "text" after the opening
backticks -> ```text) so the example complies with markdownlint rule MD040;
ensure the closing backticks remain unchanged.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Nitpick comments:
In@docs/feature-aelf-core.md:
- Around line 71-79: The fenced code block showing the CLI usage lacks a
language specifier; update the block that contains the aelf core usage (the
lines starting with "aelf core [--json]" through "--no-locked") to include a
language tag (e.g., "bash") on the opening fence so the block becomes a fenced
bash code block for proper syntax highlighting and to satisfy MD040.- Around line 108-118: The fenced code block showing the belief example lacks a
language tag; update the block delimiter around "
[LOCK,CORR=3,α=4.0,β=1.0,μ=0.800]: " to include a language
(e.g., changetotext) so markdownlint MD040 is satisfied and the example
renders correctly; ensure the surrounding example and any references to the tag
fields (LOCK,CORR,α,β,μ) remain unchanged.- Around line 208-210: Add a language specifier to the fenced code block
containing the command/table example so markdownlint MD040 is satisfied: update
the triple-backtick fence that currently precedes the line withcore [--json] [--limit N] ...to include a language tag (e.g.,markdown) and keep the closingunchanged; this change is in the docs/feature-aelf-core.md example
block.- Around line 119-125: Update the fenced code block shown under "Empty store /
no matches" (the block containing "no core beliefs") to include a language
specifier (e.g., add "text" after the opening backticks -> ```text) so the
example complies with markdownlint rule MD040; ensure the closing backticks
remain unchanged.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `c294d93b-4f62-41ee-97d3-47648686ff6a` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 985c367630bdfb1c22d36ab00d760b5c9b68ee09 and 68101dafe2370d11009ca624bb553bbb902d5f72. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `docs/feature-aelf-core.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
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 |
Collaborator
Author
|
[claim:review:Toug:2026-05-06T20:58:49Z] |
robotrocketscience
force-pushed
the
feat/issue-439-cli-core
branch
from
May 6, 2026 20:59
68101da to
fe2dd96
Compare
robotrocketscience
force-pushed
the
feat/issue-439-cli-core
branch
from
May 6, 2026 21:00
fe2dd96 to
e623072
Compare
Collaborator
Author
|
[release:review:Toug:2026-05-06T21:01:30Z] |
This was referenced May 6, 2026
robotrocketscience
added a commit
that referenced
this pull request
May 7, 2026
Sourcery flagged a semantics mismatch: help text said '0 disables' but the implementation reads as 'threshold = 0 admits any non-negative value'. Existing test test_core_disabled_posterior_and_corr_includes_all_nonprior asserts b-thin (posterior-only candidate) is included with --min-posterior 0.0 --min-alpha-beta 0, which only holds under the lowering reading — so the implementation is intentional and the test pins it; only the help text was misleading. Reword help to 'lower to widen lens — 0 admits …' so the documented behavior matches code + existing test. No semantics change. Spec memo (docs/feature-aelf-core.md, PR #456) flag table also says '0 disables'; that is a separate doc-edit follow-up — not flipped here to keep this PR focused on the Sourcery findings.
robotrocketscience
added a commit
that referenced
this pull request
May 7, 2026
Sourcery flagged a semantics mismatch: help text said '0 disables' but the implementation reads as 'threshold = 0 admits any non-negative value'. Existing test test_core_disabled_posterior_and_corr_includes_all_nonprior asserts b-thin (posterior-only candidate) is included with --min-posterior 0.0 --min-alpha-beta 0, which only holds under the lowering reading — so the implementation is intentional and the test pins it; only the help text was misleading. Reword help to 'lower to widen lens — 0 admits …' so the documented behavior matches code + existing test. No semantics change. Spec memo (docs/feature-aelf-core.md, PR #456) flag table also says '0 disables'; that is a separate doc-edit follow-up — not flipped here to keep this PR focused on the Sourcery findings.
robotrocketscience
added a commit
that referenced
this pull request
May 7, 2026
Sourcery flagged a semantics mismatch: help text said '0 disables' but the implementation reads as 'threshold = 0 admits any non-negative value'. Existing test test_core_disabled_posterior_and_corr_includes_all_nonprior asserts b-thin (posterior-only candidate) is included with --min-posterior 0.0 --min-alpha-beta 0, which only holds under the lowering reading — so the implementation is intentional and the test pins it; only the help text was misleading. Reword help to 'lower to widen lens — 0 admits …' so the documented behavior matches code + existing test. No semantics change. Spec memo (docs/feature-aelf-core.md, PR #456) flag table also says '0 disables'; that is a separate doc-edit follow-up — not flipped here to keep this PR focused on the Sourcery findings.
robotrocketscience
added a commit
that referenced
this pull request
May 8, 2026
Sourcery flagged a semantics mismatch: help text said '0 disables' but the implementation reads as 'threshold = 0 admits any non-negative value'. Existing test test_core_disabled_posterior_and_corr_includes_all_nonprior asserts b-thin (posterior-only candidate) is included with --min-posterior 0.0 --min-alpha-beta 0, which only holds under the lowering reading — so the implementation is intentional and the test pins it; only the help text was misleading. Reword help to 'lower to widen lens — 0 admits …' so the documented behavior matches code + existing test. No semantics change. Spec memo (docs/feature-aelf-core.md, PR #456) flag table also says '0 disables'; that is a separate doc-edit follow-up — not flipped here to keep this PR focused on the Sourcery findings.
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.
Spec memo for #439 —
aelf coreCLI. Closes the recovery-inventory line atdocs/ROADMAP.mdv2.0.0 row (core / unlock / delete / confirm (CLI surface)).What this PR is
Docs-only. New file at
docs/feature-aelf-core.md. Converts the bare issue acceptance sketch into a buildable contract: definition of "core", default thresholds with justifications, CLI shape, output shape, implementation sketch, and test plan.No code, no schema, no flag wiring. This PR moves #439 from
needs-specto spec-ready.Definition of "core" (issue acceptance #1)
A belief is core if any of:
lock_level != 'none'(locked).corroboration_count >= 2(independently re-ingested from ≥2 distinct sources).posterior_mean >= 2/3ANDalpha + beta >= 4(multi-event majority-positive Beta-Bernoulli).The three signals are independent, each operator-tunable via a flag. Defaults are justified in the memo against the Beta(1,1) prior and the structure of
belief_corroborations.Rejected alternatives:
aelf locked. No new information.CLI contract
--locked-onlyand--no-lockedare mutually exclusive (argparse exit 2).Output
Text — one line per belief, locked-first sort matching
aelf locked:Tag block omits non-signalling fields.
--jsonemits a list withsignals: ["lock", "corroboration", "posterior"]so the operator can see why each row is included.Implementation (issue acceptance #2-4)
_cmd_coreinsrc/aelfrice/cli.pycomposes existing API:store.list_locked_beliefs()for the locked subset.store.list_belief_ids()+store.get_belief()walk for the corroboration / posterior subsets.No new store method. Memo argues the N+1 pattern is fine at v2.0 store sizes (target ~10⁴ beliefs, low-ms walk) since
aelf coreis a research-line / operator verb, not a per-turn hot path. Promote to a single SQL query later if measurement justifies it.Reconciliation
aelf locked:locked= L0 only (lock_level != 'none');core= L0 ∪ corroborated ∪ high-posterior.core --locked-onlyis equivalent tolockedmodulo output formatting.aelf wonder:wondersurfaces consolidation candidates (beliefs that look like they should merge);coresurfaces currently load-bearing beliefs (already anchoring retrieval). Different verbs, different intents.Substrate
All on
mainas of985c367:src/aelfrice/cli.py:_cmd_locked— output-shape and store-open precedent.src/aelfrice/cli.py:_cmd_unlock— argparse / store-open / exit-code precedent for sibling verbs.src/aelfrice/store.py:list_locked_beliefs— reused.src/aelfrice/store.py:list_belief_ids+get_belief— used for the candidate walk.src/aelfrice/models.py:Belief—alpha,beta,lock_level,corroboration_countfields all already populated by_row_to_belief.No new dependencies. No schema changes.
Test plan
github/main— clean.G).MIN_CORROBORATION=2,MIN_POSTERIOR=2/3,MIN_ALPHA_BETA=4defaults; confirm PageRank deferral is acceptable.Out of scope (deferred)
aelf core --explain <id>— useful follow-up verb.Refs
aelf coreCLI — research-line surface, sibling ofaelf unlock#439docs/ROADMAP.mdv2.0.0 row.aelf locked(feat(v1.3): partial Bayesian-weighted ranking — log-additive (closes #146) #178),aelf unlock(shipped),aelf confirm([v2.0]aelf confirmCLI — explicit corroboration of an existing belief #441 PR feat(cli): aelf confirm — explicit user affirmation (#441) #445 merged 2026-05-05).aelf delete, [v2.0]aelf deleteCLI — sibling ofaelf unlock, ship-as-deferred-by-default #440).Summary by Sourcery
Document the
aelf coreCLI as an implementation-ready feature spec defining its purpose, core-belief criteria, CLI contract, and behavior within the belief store ecosystem.Documentation:
aelf coreCLI covering definition of core beliefs, tunable thresholds, CLI flags, and text/JSON output formats.aelf corerelates to existing commands (locked,unlock,wonder) and defer advanced signals like PageRank and hibernation-aware filtering.aelf coreimplementation and slash command wiring.Summary by CodeRabbit