feat(cli): belief introspection + reversible soft-delete curation (#1081) - #1101
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
robotrocketscience
added
the
attn:review
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds an Changesaelf introspect feature
aelf retire/restore curation
Supporting changes Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as aelf CLI
participant Introspect as introspect.build_report
participant Store as MemoryStore
participant Audit as feedback_history
CLI->>Introspect: introspect --by session/project
Introspect->>Store: load active beliefs
Store-->>Introspect: beliefs
Introspect-->>CLI: grouped report (text/JSON)
CLI->>Store: retire belief_id
Store-->>CLI: soft_delete_belief result
CLI->>Audit: log retire event
CLI->>Store: restore belief_id
Store-->>CLI: restore_belief result
CLI->>Audit: log restore event
Related issues: Suggested labels: enhancement, cli, tests Suggested reviewers: robotrocketscience 🐰 A belief once floated, now marked with care, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/test_introspect.py (1)
41-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test covers the
recurrencesignal.
BeliefSignals.recurrence(mapped frombelief.corroboration_count) is one of the four named signals in the PR description, but no test assertssig.recurrenceagainst a seededcorroboration_count. Coverage exists for posterior/evidence, grounding, status, and noise, but not recurrence.✅ Suggested test
def test_recurrence_reflects_corroboration_count() -> None: s = MemoryStore(":memory:") try: b = _mk("a") b.corroboration_count = 3 s.insert_belief(b) report = build_report(s) finally: s.close() assert _by_id(report)["a"].recurrence == 3Also applies to: 154-167
🤖 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/test_introspect.py` around lines 41 - 65, Add a test that covers the missing recurrence signal by seeding a Belief with corroboration_count and asserting the generated BeliefSignals.recurrence value in build_report output. Use the existing _mk helper and locate the behavior through MemoryStore and build_report, then verify _by_id(report)[...].recurrence matches the seeded count so the mapping from belief.corroboration_count is exercised.src/aelfrice/introspect.py (2)
133-149: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffN+1 query pattern in
_status.
_statusissues two separate SQL round-trips (edges_to+edges_from) per belief.build_reportcalls this once for every belief surfaced (up to--limit, default 100, unbounded when--limit 0), so a full report does up to2 * totalindividual queries instead of a single batched lookup — unlikeentity_persistence_scores, which is already batched bybelief_id IN (...). For large stores or--limit 0, this scales linearly with query round-trips rather than being O(1) queries.Consider adding a batched edge-lookup on the store (mirroring
entity_persistence_scores) that returns incoming/outgoing RESOLVES/POTENTIALLY_STALE edges keyed by belief id for the fullidslist, then deriving status from that map instead of per-belief calls.🤖 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/introspect.py` around lines 133 - 149, The _status helper is causing an N+1 query pattern by calling edges_to and edges_from for each belief individually. Update build_report to fetch all needed edges in one batched store lookup for the full ids set, similar to entity_persistence_scores, then derive each belief’s status from a keyed map instead of per-belief round-trips. Keep the status precedence logic from _status (incoming RESOLVES, then POTENTIALLY_STALE, then outgoing RESOLVES, else floated) while moving the data access into a batched store method.
195-201: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffFull active-belief table fetched before filtering/limiting.
store.list_active_beliefs(limit=None, order="recent")always pulls every active belief into memory, thensession/projectfiltering and the--limitslice happen in Python. For stores with many thousands of active beliefs this means the read-onlyintrospectcommand's cost scales with total store size rather than with the requested--limit, even though the default limit is 100.This is arguably a deliberate tradeoff (limit must apply after filtering per the docstring), but for large stores it's worth considering pushing
session_id/project_contextfilters into the SQL layer (e.g., an optional filter parameter onlist_active_beliefs) so only matching rows are fetched, with the Python-side slice still applied afterward.🤖 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/introspect.py` around lines 195 - 201, The introspect path is loading all active beliefs in memory before applying session/project filtering and the requested limit. Update the store access used by the introspection flow so `list_active_beliefs` (or the underlying store query it calls) can accept optional `session_id` and `project_context` filters, then fetch only matching rows while still applying the final `limit` slice afterward in the introspection logic.
🤖 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 7372-7375: The `--limit` option in the CLI allows negative
integers, which then reach `_cmd_introspect` and `build_report` where slicing
with a negative value silently truncates from the end instead of failing. Update
the argument validation for `p_introspect.add_argument("--limit", ...)` to
reject values below 0 while still allowing 0 as the existing “no cap” case,
ideally by reusing a validator pattern like `_positive_int` with a zero-allowed
variant so invalid input fails fast with a clear error.
In `@src/aelfrice/store.py`:
- Around line 2246-2256: The restore flow in the beliefs persistence path
currently returns True after a pre-check SELECT even if the subsequent UPDATE
did not actually restore anything, which can happen under concurrent restores.
Update the logic in the store method that restores beliefs to rely on the UPDATE
outcome instead of the earlier SELECT: keep the existence check if needed, but
only return True when the UPDATE on beliefs changes a row by checking its
rowcount, and return False otherwise so callers do not emit duplicate feedback
or invalidation events.
---
Nitpick comments:
In `@src/aelfrice/introspect.py`:
- Around line 133-149: The _status helper is causing an N+1 query pattern by
calling edges_to and edges_from for each belief individually. Update
build_report to fetch all needed edges in one batched store lookup for the full
ids set, similar to entity_persistence_scores, then derive each belief’s status
from a keyed map instead of per-belief round-trips. Keep the status precedence
logic from _status (incoming RESOLVES, then POTENTIALLY_STALE, then outgoing
RESOLVES, else floated) while moving the data access into a batched store
method.
- Around line 195-201: The introspect path is loading all active beliefs in
memory before applying session/project filtering and the requested limit. Update
the store access used by the introspection flow so `list_active_beliefs` (or the
underlying store query it calls) can accept optional `session_id` and
`project_context` filters, then fetch only matching rows while still applying
the final `limit` slice afterward in the introspection logic.
In `@tests/test_introspect.py`:
- Around line 41-65: Add a test that covers the missing recurrence signal by
seeding a Belief with corroboration_count and asserting the generated
BeliefSignals.recurrence value in build_report output. Use the existing _mk
helper and locate the behavior through MemoryStore and build_report, then verify
_by_id(report)[...].recurrence matches the seeded count so the mapping from
belief.corroboration_count is exercised.
🪄 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: 132352a7-8dac-4726-9ed6-424e0f5e7e4f
📒 Files selected for processing (10)
CHANGELOG/v3.mdsrc/aelfrice/cli.pysrc/aelfrice/introspect.pysrc/aelfrice/slash_commands/introspect.mdsrc/aelfrice/slash_commands/restore.mdsrc/aelfrice/slash_commands/retire.mdsrc/aelfrice/store.pytests/test_cli_retire_restore.pytests/test_introspect.pytests/test_slash_commands.py
|
[claim:review:Setr:2026-07-06T19:00:10Z] |
Review — LGTM, mergeableReviewed the diff against Correctness/quality: clean two-commit series (both signed), FF on On the two CodeRabbit "Minor" items — both assessed non-blocking:
Merging. |
robotrocketscience
added
ready-to-merge
Add the reversible curation pair the introspection tool points users at. retire is the gentle sibling of delete: it sets valid_to (the belief drops out of retrieval and FTS search) but preserves the evidence trail — edges, entities, and corroborations survive — so it can be undone. restore clears valid_to and re-inserts the FTS index row soft_delete pruned. - store.restore_belief: the missing inverse of soft_delete_belief; idempotent via a valid_to IS NOT NULL guard, returns False on already-active/unknown. - retire: --force to retire locked beliefs; already-retired is a no-op with no second audit row; writes user_retired / user_retired_force (valence -1). - restore: writes user_restored (valence +1); not-restorable exits non-zero. - Both honour the #661 read-only federation contract (assert_local_ownership). - Slash files + EXPECTED_COMMANDS entries for the two visible verbs.
A native answer to "look at my conversations and analyse the beliefs it extracted." introspect groups the active beliefs by session (or --by project) and surfaces, per belief, the signals the store already holds but never displays together: - posterior mean μ + evidence weight (α+β) - recurrence (corroboration count), labelled as recurrence, NOT truth - grounding: durable / ephemeral / neutral (the #1096 entity-persistence axis — standalone-meaningful vs context-bound) - status: floated vs decided, from RESOLVES / POTENTIALLY_STALE edges - noise: the #1081 stranded-capture predicate; junk floats to the top of each group as the prime retire candidates The deterministic core is a pure module (aelfrice.introspect.build_report); the CLI wrapper formats text or --json. Read-only per #605 (pure counts + edge lookups, no model, no clock) — curation stays in retire/lock/resolve, which the footer points at. Flags: --by, --session, --project, --only-noise, --limit (0 = no cap), --json. Also drops the #1081 curation-half CHANGELOG entries.
5c08be8 to
748a225
Compare
|
Rebased onto current |
robotrocketscience
added
the
ready-to-merge
|
merge-train: merged 748a225 → |
github-actions
Bot
removed
the
ready-to-merge
|
[release:review:Setr:2026-07-06T19:16:34Z] |
1 participant
Summary
Completes the curation half of #1081 — the two deliverables the operator
enumerated for it: a native belief-introspection tool and a reversible
soft-delete curation pass. The other curation-half pieces already merged
(hedge-float drop at ingest, stranded-capture prune,
aelf contextsource-turnrecovery); this is the last of the enumerated scope.
The scoring-redesign half (exposure-≠-endorsement, organic sink, recurrence-vs-
truth axis, decided-vs-floated status) remains tracked separately under #1086.
What lands
aelf introspect— a read-only, honest-signal view answering "look at myconversations and analyse the beliefs it extracted" without hand-rolled SQL.
Groups active beliefs by ingest session (or
--by project) and surfaces, perbelief, the signals the store already holds but never displayed together:
not truth (a junk line re-captured every session scores high on it alone)
durable/ephemeral/neutral(the standalone-meaningful vs context-bound signal)RESOLVES/POTENTIALLY_STALEedgesis_stranded_capture_noisepredicate; stranded rowsfloat to the top of each group as the prime retire candidates
Deterministic per #605 (pure counts + edge lookups, no model, no clock). The
core is a pure module (
aelfrice.introspect.build_report); the CLI wrapperformats text or
--json. Flags:--by,--session,--project,--only-noise(the retire shortlist),--limit N(0 = no cap),--json.aelf retire/aelf restore— reversible soft-delete curation.retireis the gentle sibling of
delete: it setsvalid_to(belief drops out ofretrieval + FTS search) while preserving the evidence trail (edges, entities,
corroborations);
restore— the previously-missing inverse ofsoft_delete_belief— clearsvalid_toand re-inserts the pruned FTS row. Thismakes the store's soft-delete reversible end-to-end, so curation off the
introspectview is a low-friction, undoable pass. Audit rows:user_retired/user_retired_force(−1),user_restored(+1). Lock guard (--force),#661 read-only-federation guard, already-retired/already-active no-ops.
The command surface (new
aelf introspectvs extendaelf review) and thereversal shape (
restore/retireverbs vs a curation-file workflow) were bothoperator-confirmed before build.
Tests
tests/test_cli_retire_restore.py— retire/restore CLI +store.restore_belief(valid_to transitions, FTS membership, audit rows, edge preservation vs delete,
round-trip, no-op paths).
tests/test_introspect.py— the deterministic module (grouping, every signal,within-group ordering, filters, soft-delete exclusion, invalid group_by) + CLI
text/JSON smoke.
EXPECTED_COMMANDSupdated for the three new visible verbs (slash files ship).Closes #1081.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation