feat: replay_full_equality probe — flip-readiness gate for #262 - #304
Merged
Conversation
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Reviewer's GuideImplements the v2.x full-equality replay probe as a concrete shape-equality comparison between re-derived beliefs and the canonical store, and wires it into Sequence diagram for aelf doctor --replay flowsequenceDiagram
actor User
participant CLI as aelf_doctor
participant Doctor as _cmd_doctor
participant Replay as _cmd_doctor_replay
participant Store as MemoryStore
participant ReplayFn as replay_full_equality
User->>CLI: run `aelf doctor --replay [--max-drift N] [--drift-examples N] [--replay-scope all|since-v2]`
CLI->>Doctor: parse args, call _cmd_doctor(args, out)
Doctor->>Doctor: check args.gc_orphan_feedback
Doctor->>Doctor: check args.classify_orphans
Doctor->>Replay: args.replay is True, call _cmd_doctor_replay(args, out)
Replay->>Store: _open_store()
Replay->>ReplayFn: replay_full_equality(store, max_drift, drift_examples, scope)
ReplayFn-->>Replay: FullEqualityReport
Replay->>Store: close()
Replay->>CLI: _print_replay_report(report, out)
Replay->>Replay: drift_total = mismatched + derived_orphan
Replay->>Replay: threshold = max_drift or 0
Replay-->>Doctor: exit_code = 0 if drift_total <= threshold else 1
Doctor-->>CLI: return exit_code
CLI-->>User: process exits with code 0 or 1
Class diagram for FullEqualityReport and replay scopeclassDiagram
class FullEqualityReport {
+bool implemented
+int total_log_rows
+int excluded_legacy_unknown
+int matched
+int mismatched
+int derived_orphan
+int canonical_orphan
+int legacy_origin_backfill
+int feedback_derived_edges
+dict~str,list~dict~~ drift_examples
+bool has_drift()
}
class ReplayScope {
<<type alias>>
+all
+since_v2
}
FullEqualityReport .. ReplayScope : used_by
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (4)
📝 WalkthroughWalkthroughThe PR implements the v2.x flip-readiness validation probe ( Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as aelf doctor --replay
participant ReplayFn as replay_full_equality
participant Store as MemoryStore
participant Derive as derive()
participant Compare as Shape Equality
User->>CLI: Run with max_drift threshold
CLI->>Store: Load canonical beliefs
CLI->>ReplayFn: Invoke with store config
ReplayFn->>Store: Iterate non-legacy log rows
loop For each log row
ReplayFn->>Derive: derive(DerivationInput)
Derive->>Derive: Synthesize belief
ReplayFn->>Compare: Compare derived vs canonical
alt Match
Compare->>ReplayFn: increment matched
else Mismatch
Compare->>ReplayFn: increment mismatched, sample drift
else Orphan
Compare->>ReplayFn: increment derived_orphan
end
end
ReplayFn->>ReplayFn: Detect canonical orphans
ReplayFn->>CLI: Return FullEqualityReport
CLI->>CLI: Print bucket summary + drift examples
alt Drift ≤ max_drift
CLI->>User: Exit 0
else Drift > max_drift
CLI->>User: Exit 1
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 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)
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. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 58 seconds.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 4 issues, and left some high level feedback:
- In
_print_replay_report, thefeedback_derived_edgesline is split into two separate list entries, so the count and the(informational)suffix print on separate lines; if you intend a single line as described in the docstring, join them into one formatted string. - The shape-equality definition in
FullEqualityReportmentions matching the deterministic edge set, butreplay_full_equalitynever inspects or compares edges; if edge equality is part of the contract, consider either implementing that comparison or explicitly calling out that current semantics are belief-only.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_print_replay_report`, the `feedback_derived_edges` line is split into two separate list entries, so the count and the `(informational)` suffix print on separate lines; if you intend a single line as described in the docstring, join them into one formatted string.
- The shape-equality definition in `FullEqualityReport` mentions matching the deterministic edge set, but `replay_full_equality` never inspects or compares edges; if edge equality is part of the contract, consider either implementing that comparison or explicitly calling out that current semantics are belief-only.
## Individual Comments
### Comment 1
<location path="src/aelfrice/replay.py" line_range="129-131" />
<code_context>
+ feedback_derived_edges: int # non-deterministic edges (informational)
+ drift_examples: dict[str, list[dict]] # type: ignore[type-arg]
+
+ @property
+ def has_drift(self) -> bool:
+ return self.mismatched > 0 or self.derived_orphan > 0
+
+
</code_context>
<issue_to_address>
**question:** Canonical orphans are not counted as drift, which may or may not match the intended semantics.
The docstring notes `feedback_derived_edges` as informational-only but doesn’t clarify whether `canonical_orphan` should affect drift. Since `has_drift` only checks `mismatched` and `derived_orphan`, `canonical_orphan` is also effectively informational. If canonical-only beliefs are intended to count as drift, this predicate should include them; otherwise, consider updating the docstring to state that `canonical_orphan` is informational-only to prevent confusion.
</issue_to_address>
### Comment 2
<location path="src/aelfrice/replay.py" line_range="296-305" />
<code_context>
+ # --- Canonical orphans -------------------------------------------------
+ # A canonical belief is an orphan when every log row pointing at it is
+ # legacy_unknown (or there are no log rows at all, pre-#205).
+ belief_ids = store.list_belief_ids()
+ canonical_orphan = 0
+ examples_canonical_orphan: list[dict] = [] # type: ignore[type-arg]
+
+ for bid in belief_ids:
+ all_rows = store.iter_ingest_log_for_belief(bid)
+ has_non_legacy = any(
+ str(r.get("source_kind", "")) != INGEST_SOURCE_LEGACY_UNKNOWN
+ for r in all_rows
+ )
+ if not has_non_legacy:
+ canonical_orphan += 1
+ if len(examples_canonical_orphan) < drift_examples:
+ b = store.get_belief(bid)
+ examples_canonical_orphan.append({
+ "belief_id": bid,
+ "content_hash": b.content_hash if b is not None else None,
</code_context>
<issue_to_address>
**suggestion (performance):** Canonical orphan detection may result in an N+1 query pattern over beliefs.
Each `belief_id` triggers `iter_ingest_log_for_belief`, and sampled orphans also call `get_belief`, which scales linearly with the number of beliefs (N+1 pattern). For large stores, consider a set-based query (e.g., join beliefs to ingest_log and use `GROUP BY` / `HAVING source_kind != legacy_unknown`) to compute orphans in one or a few queries, then fetch only the sampled beliefs for `content_hash`.
Suggested implementation:
```python
# --- Canonical orphans -------------------------------------------------
# A canonical belief is an orphan when every log row pointing at it is
# legacy_unknown (or there are no log rows at all, pre-#205).
#
# NOTE: This computation is implemented in the store using a set-based
# query (joining beliefs to ingest_log and grouping) to avoid an N+1
# pattern over beliefs.
canonical_orphan, orphan_belief_examples = store.get_canonical_orphans(
example_limit=drift_examples,
legacy_unknown_kind=INGEST_SOURCE_LEGACY_UNKNOWN,
)
examples_canonical_orphan: list[dict] = [
{
"belief_id": b.id,
"content_hash": b.content_hash,
}
for b in orphan_belief_examples
]
```
To fully implement this optimization and avoid the N+1 query pattern, you will also need to:
1. **Add a new method on the store interface / implementation**, something like:
```python
def get_canonical_orphans(
self,
example_limit: int,
legacy_unknown_kind: str,
) -> tuple[int, Sequence[Belief]]:
...
```
2. **Implement `get_canonical_orphans` using a set-based query**, e.g. (SQL-ish pseudocode):
```sql
SELECT
b.id AS belief_id,
b.content_hash,
CASE WHEN EXISTS (
SELECT 1
FROM ingest_log il
WHERE il.belief_id = b.id
AND il.source_kind <> :legacy_unknown_kind
) THEN 0 ELSE 1 END AS is_orphan
FROM beliefs b;
```
Or more efficiently:
```sql
SELECT
b.id AS belief_id,
b.content_hash
FROM beliefs b
LEFT JOIN ingest_log il
ON il.belief_id = b.id
AND il.source_kind <> :legacy_unknown_kind
GROUP BY b.id, b.content_hash
HAVING COUNT(il.id) = 0;
```
Then:
- `canonical_orphan` is the total count of rows returned.
- `orphan_belief_examples` is at most `example_limit` belief objects built from the first rows.
3. **Ensure the store implementation returns belief objects** (or a lightweight struct) with `.id` and `.content_hash` attributes so the list comprehension in `replay.py` works as written.
4. **Update any mocks / tests** for the store to provide `get_canonical_orphans` with the expected signature and behavior, replacing checks that previously depended on `list_belief_ids`, `iter_ingest_log_for_belief`, and `get_belief` in the canonical-orphan logic.
</issue_to_address>
### Comment 3
<location path="src/aelfrice/cli.py" line_range="2149-2156" />
<code_context>
+ """
+ from aelfrice.replay import replay_full_equality, FullEqualityReport
+
+ max_drift: int | None = getattr(args, "max_drift", None)
+ drift_examples: int = int(getattr(args, "drift_examples", 10) or 10)
+ replay_scope: str = getattr(args, "replay_scope", "all") or "all"
+
+ store = _open_store()
+ try:
+ report = replay_full_equality(
+ store,
+ max_drift=max_drift,
+ drift_examples=drift_examples,
+ scope=replay_scope, # type: ignore[arg-type]
+ )
+ finally:
+ store.close()
+
+ _print_replay_report(report, out)
+
+ drift_total = report.mismatched + report.derived_orphan
+ threshold = max_drift if max_drift is not None else 0
+ return 0 if drift_total <= threshold else 1
+
</code_context>
<issue_to_address>
**suggestion:** Clamp --max-drift to a non-negative value to avoid surprising exit codes for negative inputs.
With a negative `--max-drift`, `threshold` becomes negative, so even `drift_total == 0` returns a non-zero exit code. To avoid this, clamp the threshold to zero, e.g. `threshold = max(0, max_drift) if max_drift is not None else 0`, so accidental negative values don’t change the semantics or cause unexpected failures.
```suggestion
def _cmd_doctor_replay(args: argparse.Namespace, out: object) -> int:
"""Run the v2.x full-equality replay probe (#262).
Called from `_cmd_doctor` when ``--replay`` is set. Opens the
store, runs `replay_full_equality`, prints the report, and returns
0 or 1 based on drift counts and the ``--max-drift`` threshold.
"""
from aelfrice.replay import replay_full_equality, FullEqualityReport
max_drift: int | None = getattr(args, "max_drift", None)
drift_examples: int = int(getattr(args, "drift_examples", 10) or 10)
replay_scope: str = getattr(args, "replay_scope", "all") or "all"
store = _open_store()
try:
report: FullEqualityReport = replay_full_equality(
store,
max_drift=max_drift,
drift_examples=drift_examples,
scope=replay_scope, # type: ignore[arg-type]
)
finally:
store.close()
_print_replay_report(report, out)
drift_total = report.mismatched + report.derived_orphan
# Clamp threshold to zero so negative --max-drift values do not
# cause surprising non-zero exit codes for drift_total == 0.
threshold = max(0, max_drift) if max_drift is not None else 0
return 0 if drift_total <= threshold else 1
```
</issue_to_address>
### Comment 4
<location path="tests/test_replay_full_equality.py" line_range="478-487" />
<code_context>
+# ---------------------------------------------------------------------------
+
+
+def test_cli_doctor_replay_exit_0_clean_store(
+ tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+ """Hypothesis: `aelf doctor --replay` exits 0 on a store with no drift.
+ Falsifiable by any non-zero exit code."""
+ import io
+ from aelfrice.cli import main
+
+ db = str(tmp_path / "brain.db")
+ monkeypatch.setenv("AELFRICE_DB", db)
+
+ # Ingest a belief so total_log_rows >= 1.
+ s = MemoryStore(db)
+ _ingest(s, _FACTUAL_SENTENCE)
+ s.close()
+
+ out = io.StringIO()
+ rc = main(["doctor", "--replay"], out=out)
+ assert rc == 0
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a CLI test that a canonical-orphan-only scenario still exits 0 with `--replay`
You already cover `--replay` exit codes for no drift, mismatches, and `--max-drift`. Since `canonical_orphan` is informational and shouldn’t affect the exit code (only `mismatched` and `derived_orphan` do), please add a CLI-level test that creates a canonical orphan (e.g., a belief with no non-legacy log rows), runs `doctor --replay`, and asserts exit code 0. This will guard that the CLI continues to respect the `has_drift` / drift-count semantics as internals evolve.
Suggested implementation:
```python
def test_cli_doctor_replay_exit_0_clean_store(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Hypothesis: `aelf doctor --replay` exits 0 on a store with no drift.
Falsifiable by any non-zero exit code."""
import io
from aelfrice.cli import main
db = str(tmp_path / "brain.db")
monkeypatch.setenv("AELFRICE_DB", db)
# Ingest a belief so total_log_rows >= 1.
s = MemoryStore(db)
_ingest(s, _FACTUAL_SENTENCE)
s.close()
out = io.StringIO()
rc = main(["doctor", "--replay"], out=out)
assert rc == 0
def test_cli_doctor_replay_exit_0_canonical_orphan_only(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Hypothesis: `aelf doctor --replay` exits 0 when store has only
`canonical_orphan` drift. Falsifiable by any non-zero exit code.
Setup: create a belief, then delete all corresponding non-legacy log rows so
only canonical rows remain, yielding a canonical-orphan-only scenario.
"""
import io
import sqlite3
from aelfrice.cli import main
db = str(tmp_path / "brain.db")
monkeypatch.setenv("AELFRICE_DB", db)
# Create a belief with at least one log row.
s = MemoryStore(db)
_ingest(s, _FACTUAL_SENTENCE)
# Manually delete all non-legacy log rows to force a canonical-orphan-only scenario.
#
# NOTE: this assumes `MemoryStore` exposes a `.conn` sqlite3.Connection and that
# non-legacy log rows live in a `log` table. If the actual schema or attribute
# name differs, adjust the DELETE accordingly.
conn: sqlite3.Connection = s.conn # type: ignore[attr-defined]
conn.execute("DELETE FROM log")
conn.commit()
s.close()
out = io.StringIO()
rc = main(["doctor", "--replay"], out=out)
assert rc == 0
```
To make this test pass in your actual codebase, verify and, if necessary, adjust:
1. The way you access the underlying SQLite connection from `MemoryStore`. If it is not exposed as `.conn`, either:
- Change `s.conn` to the correct attribute (e.g. `s._conn`, `s.connection`, or a method like `s.raw_connection()`), or
- Add a small helper on `MemoryStore` that returns the `sqlite3.Connection`.
2. The table name and predicate used to delete non-legacy log rows:
- If your log table is not named `log`, update the `DELETE FROM log` statement accordingly.
- If you need to preserve legacy rows while deleting only non-legacy ones, refine the DELETE to something like:
`DELETE FROM log WHERE is_legacy = 0` or whatever drift-detection uses to distinguish legacy rows.
3. If there is already a test/helper in this file that creates a canonical-orphan-only scenario using public APIs, prefer reusing that instead of direct SQL and update the body of this test to call that helper for consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_replay_full_equality.py (1)
14-19: Unused imports.
_belief_idand_content_hashare imported but never used in the test file.♻️ Remove unused imports
from aelfrice.derivation import ( DerivationInput, - _belief_id, - _content_hash, derive, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_replay_full_equality.py` around lines 14 - 19, The test imports unused symbols _belief_id and _content_hash from aelfrice.derivation; remove those two names from the import statement so only DerivationInput and derive are imported (i.e., update the import tuple in tests/test_replay_full_equality.py to eliminate _belief_id and _content_hash).src/aelfrice/replay.py (1)
220-225: Redundant conditional assignments.Several expressions like
source_path if source_path is not None else Noneare tautologies that always return the original value. They can be simplified.♻️ Simplify redundant conditionals
inp = DerivationInput( raw_text=raw_text, source_kind=source_kind, - source_path=source_path if source_path is not None else None, + source_path=source_path, raw_meta=None, # raw_meta is metadata only; derive() does not use it - session_id=session_id if session_id is not None else None, + session_id=session_id, ts=ts, - classifier_version=classifier_version if classifier_version is not None else None, - rule_set_hash=rule_set_hash if rule_set_hash is not None else None, + classifier_version=classifier_version, + rule_set_hash=rule_set_hash, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/aelfrice/replay.py` around lines 220 - 225, The code in replay.py uses redundant ternary expressions like "source_path if source_path is not None else None" (and similarly for session_id, classifier_version, rule_set_hash) when building the object/kwargs; simplify these by passing the variables directly (e.g., use source_path, session_id, classifier_version, rule_set_hash) instead of the tautological conditional expressions inside the function or constructor where these appear ( locate the block that sets source_path=..., raw_meta=..., session_id=..., ts=..., classifier_version=..., rule_set_hash=... and replace the redundant conditionals with the plain variable names ).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/aelfrice/cli.py`:
- Line 2159: The current assignment for drift_examples uses "or 10" which treats
an explicit --drift-examples 0 as falsy and replaces it with 10; change it to
check for None instead: read getattr(args, "drift_examples", None) into a
temporary (or inline conditional) and set drift_examples = 10 only when that
retrieved value is None, otherwise cast the retrieved value to int so that
explicit 0 is preserved (update the line that defines drift_examples in
src/aelfrice/cli.py).
---
Nitpick comments:
In `@src/aelfrice/replay.py`:
- Around line 220-225: The code in replay.py uses redundant ternary expressions
like "source_path if source_path is not None else None" (and similarly for
session_id, classifier_version, rule_set_hash) when building the object/kwargs;
simplify these by passing the variables directly (e.g., use source_path,
session_id, classifier_version, rule_set_hash) instead of the tautological
conditional expressions inside the function or constructor where these appear (
locate the block that sets source_path=..., raw_meta=..., session_id=...,
ts=..., classifier_version=..., rule_set_hash=... and replace the redundant
conditionals with the plain variable names ).
In `@tests/test_replay_full_equality.py`:
- Around line 14-19: The test imports unused symbols _belief_id and
_content_hash from aelfrice.derivation; remove those two names from the import
statement so only DerivationInput and derive are imported (i.e., update the
import tuple in tests/test_replay_full_equality.py to eliminate _belief_id and
_content_hash).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 259b281c-6521-4609-b340-6d31f58190d8
📒 Files selected for processing (4)
src/aelfrice/cli.pysrc/aelfrice/replay.pytests/test_ingest_log.pytests/test_replay_full_equality.py
Owner
Author
|
[claim:review:Kulili:2026-04-29T04:56:26Z] |
Owner
Author
|
[claim:review:Gylf:2026-04-29T04:56:45Z] |
Owner
Author
|
[release:review:Gylf:2026-04-29T04:56:50Z] |
Owner
Author
|
Review: Code matches the spec, CI green, discretion clean. Two minor input-handling fixes blocking merge:
Non-blocking (sourcery's other notes — canonical_orphan semantics already documented, N+1 query is fine at current scale, |
Owner
Author
|
[release:review:Kulili:2026-04-29T04:58:09Z] |
Owner
Author
|
[claim:review:Gylf:2026-04-29T15:49:30Z] |
Owner
Author
|
[release:review:Gylf:2026-04-29T15:50:41Z] |
Replace FullEqualityReport stub with the ratified v2.x shape-equality implementation (#262). Adds total_log_rows, excluded_legacy_unknown, matched, mismatched, derived_orphan, canonical_orphan, legacy_origin_backfill, feedback_derived_edges, and drift_examples counters to the report. The has_drift property triggers only on mismatched + derived_orphan. Shape-equality contract (2026-04-29 ratification): content_hash + type + (origin OR canonical origin IS NULL) must match; alpha/beta/ last_retrieved_at and feedback-driven edges are excluded from the check. Note: the edges table has no source column in the current schema, so feedback_derived_edges is always 0 (informational only). Updates test_ingest_log.py to reflect the now-implemented state (implemented=True). Adds test_replay_full_equality.py covering all pure-function paths: empty store, single matched belief, posterior mutation is not drift, origin NULL backfill cohort, genuine mismatch, derived orphan, canonical orphan, legacy_unknown exclusion, drift example cap, and scope flag equivalence.
Wire replay_full_equality through aelf doctor with four new flags:
--replay (bool), --max-drift N (int), --drift-examples N (int),
--replay-scope {all,since-v2} (default all).
Exit 0 when mismatched + derived_orphan == 0 (or <= --max-drift N).
canonical_orphan and legacy_origin_backfill are informational and do
not affect the exit code. --replay bypasses the hooks/graph checks
and returns immediately after printing the report.
Output format is stable: one summary line per counter, followed by
a drift examples section when drift > 0.
Adds CLI integration tests to test_replay_full_equality.py covering
clean-store exit 0, mismatch exit 1, --max-drift threshold, and
output format assertions.
robotrocketscience
force-pushed
the
feat/issue-262-replay-full-equality
branch
from
April 29, 2026 15:52
928c698 to
51983d9
Compare
Expand the has_drift docstring on FullEqualityReport to spell out that canonical_orphan, legacy_origin_backfill, and feedback_derived_edges are informational counters and do not contribute to drift. Drift is strictly mismatched + derived_orphan. Also annotate the canonical-orphan iteration block with a perf TODO: the current implementation issues one ingest_log query per belief, which is N+1; a set-based store query is tracked in a follow-up issue.
A negative --max-drift threshold is nonsensical and previously meant a clean store (drift_total=0) would exit 1 because 0 <= -1 is false. Clamp the threshold to max(0, max_drift) so users who pass negative values still get a sensible exit code, matching the documented contract that exit 0 means drift is within the allowed budget.
The previous `int(getattr(...) or 10)` coerced any falsy value, so a user passing `--drift-examples 0` (asking for a no-examples summary) silently got the default of 10. Use an explicit `is None` check so 0 is preserved verbatim and the report contains no per-bucket example sub-blocks even when drift is present.
Add a CLI integration test that constructs a belief whose only ingest_log row is legacy_unknown — i.e., a pure canonical_orphan with no other drift — and asserts `aelf doctor --replay` exits 0. This pins the contract that canonical_orphan is informational-only and never triggers a non-zero exit code.
This was referenced Apr 30, 2026
4 tasks
4 tasks
Merged
5 tasks
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
1 participant
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
replay_full_equality(store, *, max_drift=None, drift_examples=10, scope="all") -> FullEqualityReportper the spec atdocs/v2_replay.md, replacing theimplemented=Falsestub.aelf doctor --replaywith--max-drift N,--drift-examples N, and--replay-scope {all,since-v2}flags.Spec decisions applied
mismatched + derived_orphan == 0(or <=--max-drift N).canonical_orphanandlegacy_origin_backfillare informational.source_kind=legacy_unknownare excluded fromtotal_log_rows; beliefs whose only log row islegacy_unknowncount ascanonical_orphan(not drift).--drift-examples N).Spec ambiguity resolved: feedback_derived_edges
The spec says to count edges with
source NOT IN (deterministic_set), but theedgestable schema is(src, dst, type, weight, anchor_text)— nosourcecolumn exists. All edges are structurally identical at the store level; there is no column to filter on.feedback_derived_edgesis therefore always0in this implementation, as documented in thereplay_full_equalitydocstring. This is informational-only per spec and never triggershas_drift. A schema migration addingedge_sourcewould unlock this counter.Commits
feat(replay): full-equality probe with shape-equality contract—FullEqualityReportredesign +replay_full_equalitybody + pure-function tests.feat(cli): aelf doctor --replay flag and exit codes— argparse wiring +_cmd_doctorextension + CLI integration tests.Test plan
uv run pytest tests/test_replay_full_equality.py -q— 17 tests, all passtimeout-marker collection errors):uv run pytest -q --ignore=tests/test_bfs_multihop.py --ignore=tests/test_context_rebuilder_hook.py --ignore=tests/test_rebuilder_triggers.py --ignore=tests/test_worktree_concurrency.py --ignore=tests/regression— 1776 passed, 8 skippedCLEANCloses #262
Summary by Sourcery
Implement the v2.x full-equality replay probe and wire it into the
aelf doctorCLI for flip-readiness checks.New Features:
replay_full_equalityprobe with a detailedFullEqualityReportfor comparing re-derived beliefs to canonical store state.aelf doctor --replaywith--max-drift,--drift-examples, and--replay-scopeflags to run the full-equality probe from the CLI and control exit thresholds and sampling.Enhancements:
Tests:
replay_full_equalitycovering shape-equality semantics, legacy exclusions, drift example caps, scope behavior, and CLI integration, and update existing ingest-log tests to expect the implemented replay behavior.Summary by CodeRabbit
Release Notes
aelf doctor --replaydiagnostic command for advanced data store validation. Detects synchronization issues by reporting matched and mismatched record counts, identifying drift patterns with concrete examples, and supporting configurable acceptance thresholds plus flexible sampling limits.