feat(consolidate): audit-only near-duplicate cluster report — aelf doctor --consolidate (#1312) - #1313
Merged
Merged
Conversation
New `consolidate.py` plus `aelf doctor --consolidate`. Clusters active beliefs at the shipped dedup predicate (Jaccard >= jaccard_min AND Levenshtein ratio >= levenshtein_min), picks each component's medoid, and reports how many beliefs a contraction would retire. Read-only: no edge is inserted, no belief is retired, no log row is written. Reuses dedup.py's predicate rather than inventing one, so the report says what contraction would do at the thresholds the product already ships. Candidate pairs come from 4-gram blocking with a df cap instead of dedup's O(n^2) prefilter, which is ~991M pairs at 44.5k beliefs; skipped high-df shingles are counted and reported, since a blocking cap that does not report itself reads as full coverage. Contraction is deliberately not built. The operator funded the report only, on the measurement that it would retire 996 of 44,594 active beliefs (2.23%) — too little to carry a write path or to establish the SUPERSEDES direction convention with nothing gating it. The medoid tiebreak and the size->=3 floor are mutation-verified rather than observed green: flipping the tiebreak comparison, lowering the floor, and reverting the two-belief guard each fail a different test.
![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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesConsolidation audit
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant DoctorCLI
participant MemoryStore
participant ConsolidationAudit
Operator->>DoctorCLI: run doctor --consolidate
DoctorCLI->>MemoryStore: open store
DoctorCLI->>ConsolidationAudit: audit active beliefs with thresholds
ConsolidationAudit-->>DoctorCLI: return aggregate report
DoctorCLI-->>Operator: print read-only report
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 GuideAdds an audit-only Sequence diagram for aelf doctor --consolidate audit flowsequenceDiagram
actor Operator
participant CLI as cli__cmd_doctor
participant ConsolidateCmd as cli__cmd_doctor_consolidate
participant Store as MemoryStore
participant Algo as consolidation_audit
participant Formatter as format_consolidation_report
Operator->>CLI: aelf doctor --consolidate [overrides]
CLI->>ConsolidateCmd: _cmd_doctor_consolidate(args, out)
ConsolidateCmd->>Store: _open_store()
ConsolidateCmd->>Algo: consolidation_audit(store, jaccard_min, levenshtein_min, max_shingle_df)
Algo-->>ConsolidateCmd: ConsolidationReport
ConsolidateCmd->>Store: close()
ConsolidateCmd->>Formatter: format_consolidation_report(report)
Formatter-->>ConsolidateCmd: report_text
ConsolidateCmd->>Operator: print(report_text)
ConsolidateCmd-->>CLI: return 0 / 1
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 |
Owner
Author
|
[claim:review:Garsecg:2026-08-04T02:17:14Z] |
The existing assertions compare a belief count and an edge count, which an in-place UPDATE leaves untouched and which inspect two tables out of the whole schema. Injecting 'UPDATE beliefs SET alpha = alpha + 1.0' into consolidation_audit left the suite at 20 passed; with the total_changes delta it fails. Guards the PR's central safety claim (#1312).
…e df-cap boundary Three fields the report is priced on had no distinguishing coverage; each mutation below left the suite at 20 passed before this commit and fails after it. - largest_cluster: max(...) -> min(...). The only existing assertion is on a single-cluster fixture, where max == min. Pinned on the two-cluster fixture (sizes 4 and 3). - share_of_store: denominator n_beliefs_scanned -> n_beliefs_in_clusters, and the 100.0 percentage factor -> 1.0. Both were free to change; a new fixture adds an unclustered belief so the two denominators differ. - max_shingle_df: '>' -> '>='. An off-by-one changes which pairs are examined and therefore the published share (#1312).
…rge store The audit prints nothing until it finishes and has no progress output or budget knob, unlike the sibling doctor --dedup which ships max_candidate_pairs. Cost is dominated by the medoid phase, which is quadratic in both the largest cluster's size and belief length, so runtime does not track the belief count. Says so in the help text rather than leaving the silence unexplained (#1312).
Owner
Author
Review — verified, three fixes pushed to this branchReviewed the diff, re-ran the PR body's mutation claims, and measured the audit against a copy of the live store. Recommendation: merge once CI re-greens on the three commits I pushed ( Verified good — recorded so it isn't re-litigated
Fixed on this branch
Not fixed — author's callThe medoid phase is the only phase with no budget knob, unlike the sibling |
robotrocketscience
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged b9bfc29 → |
Owner
Author
|
[release:review:Garsecg:2026-08-04T03:04:56Z] |
This was referenced Aug 4, 2026
robotrocketscience
added a commit
that referenced
this pull request
Aug 4, 2026
test_doctor_consolidate_runs resolves thresholds through load_dedup_config(), which walks up from the working directory, so the assertion depended on where pytest was launched: a developer with [dedup] levenshtein_min set at or above the repo got a failure with nothing to do with the code. CI green, local red — the same shape as #1295, one release later. Written before #1313 merged and missed the push, so it did not land with the rest of that PR. Pinned by chdir to a scratch directory, with _assert_no_ambient_config verifying the walk is clean rather than assuming it — resolving first, because load_dedup_config resolves its start while Path.parents is lexical. Two arms then cover the TOML tier, which had none: [dedup] reaches the audit, and --consolidate-* outrank it. Every arm terminates by construction: in-process, no subprocess, no waiting.
robotrocketscience
added a commit
that referenced
this pull request
Aug 4, 2026
test_doctor_consolidate_runs resolves thresholds through load_dedup_config(), which walks up from the working directory, so the assertion depended on where pytest was launched: a developer with [dedup] levenshtein_min set at or above the repo got a failure with nothing to do with the code. CI green, local red — the same shape as #1295, one release later. Written before #1313 merged and missed the push, so it did not land with the rest of that PR. Pinned by chdir to a scratch directory, with _assert_no_ambient_config verifying the walk is clean rather than assuming it — resolving first, because load_dedup_config resolves its start while Path.parents is lexical. Two arms then cover the TOML tier, which had none: [dedup] reaches the audit, and --consolidate-* outrank it. Every arm terminates by construction: in-process, no subprocess, no waiting.
robotrocketscience
added a commit
that referenced
this pull request
Aug 4, 2026
test_doctor_consolidate_runs resolves thresholds through load_dedup_config(), which walks up from the working directory, so the assertion depended on where pytest was launched: a developer with [dedup] levenshtein_min set at or above the repo got a failure with nothing to do with the code. CI green, local red — the same shape as #1295, one release later. Written before #1313 merged and missed the push, so it did not land with the rest of that PR. Pinned by chdir to a scratch directory, with _assert_no_ambient_config verifying the walk is clean rather than assuming it — resolving first, because load_dedup_config resolves its start while Path.parents is lexical. Two arms then cover the TOML tier, which had none: [dedup] reaches the audit, and --consolidate-* outrank it. Every arm terminates by construction: in-process, no subprocess, no waiting.
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.
Closes #1312. Refs #1176 proposal 4.
Audit surface only. No write path, per the operator ruling: the report is
funded, contraction is not.
What this is
aelf doctor --consolidateclusters active beliefs at the shipped deduppredicate, picks each component's medoid, and reports what a contraction
would retire. Read-only — no edge inserted, no belief retired, no log row
written, and
dedup.pystays audit-only.New
src/aelfrice/consolidate.pyholds the algorithm;cli.pyholds thesurface. Same division
dedup.pyalready uses.Why the write path is absent
Not an oversight and not scope-trimming. Proposal 4 cleared its
pre-registered kill gate — the thresholds are sound — and was still not funded
for the build, because the same run priced it:
2.23% does not carry a high-risk write path, nor the
SUPERSEDESdirectionconvention it would establish with nothing gating it — there are still 0
such edges in the store, which is exactly why the spec's stated prerequisite
read as moot when it was really unratified.
The kill check used the vocabularies
relationship_detectoralready ships(
_NEGATION_TOKENS,_QUANTIFIER_TOKENS,_CONTRACTION_NEGATION_RE), so thecriterion is the contradiction lane's own and not one invented to pass.
Mechanical, no hand-labelling, no LLM.
Design notes worth reviewing
jaccard_minANDLevenshtein ratio ≥
levenshtein_min, read from[dedup]config. A secondpredicate would make "what would contraction do" unfalsifiable.
dedupbudgets direct prefiltering at a ~1.6kmedian; at 44.5k that is ~991M pairs. Candidates share an order-4 shingle
with
df <= 32. Tightening that cap from 400 to 32 left the componentsbit-identical, so it is not load-bearing — and skipped high-df shingles are
counted and printed, because a cap that does not report itself reads as
full coverage.
min(member_ids).dedup.DuplicateClusternames itsrepresentative by smallest id, which is deterministic but arbitrary. The
medoid is the member a contraction would actually keep. One of the tests
exists specifically to fail if this degrades back into a copy of dedup's
rule.
no paths — and a test asserts that.
A bug my own test found
The first draft guarded the early exit on
MIN_COMPONENT_SIZE, so atwo-belief store reported
n_duplicate_pairs = 0when it plainly had one.Clusters were unaffected, which is why it would have survived a happy-path
test. Guard is now
len(rows) < 2; reverting it is mutation 3 below.Verification
tests/test_consolidate.py: 20 passed. Withtest_dedup.pyandtest_benchmarks_dir.py: 88 passed.Mutation-verified, not observed green — three mutations, three distinct
failures:
<→<=test_tie_breaks_on_id_ascendingMIN_COMPONENT_SIZE3 → 2test_a_duplicate_pair_is_not_a_cluster(+ the constant pin)test_a_duplicate_pair_is_not_a_clusterThe published 2.23% is reproducible from the shipped command, not from a
scratch script — that was the point of putting it behind a product surface:
CHANGELOG insert-only against
main(0 removed lines); discretion grep onadded lines and on commit messages clean; two atomic signed commits; branch
FF on
main.Summary by Sourcery
Introduce an audit-only consolidation report via
aelf doctor --consolidatethat clusters near-duplicate beliefs using the existing dedup predicate and reports potential contraction impact without modifying the store.New Features:
aelf doctor --consolidateCLI subcommand to run a consolidation audit over active beliefs.Enhancements:
Documentation:
Tests:
doctor --consolidateCLI surface.Summary by CodeRabbit
New Features
aelf doctor --consolidateto identify groups of likely duplicate beliefs.Bug Fixes
Documentation
Safety