feat(doctor): document + flag missing v2.1 auto-capture hooks (#557) - #560
Conversation
Reviewer's GuideImplements a v2.1-aware Sequence diagram for aelf doctor detecting missing auto-capture hookssequenceDiagram
actor User
participant AelfCLI
participant DoctorModule
participant SettingsJson
participant SetupCommand
User->>AelfCLI: run aelf doctor
AelfCLI->>DoctorModule: diagnose()
DoctorModule->>SettingsJson: scan settings.json for hook commands
SettingsJson-->>DoctorModule: findings list
DoctorModule->>DoctorModule: _check_auto_capture_hooks(findings)
DoctorModule-->>DoctorModule: missing_auto_capture_hooks list
DoctorModule->>DoctorModule: format_report(report)
DoctorModule->>DoctorModule: _format_missing_auto_capture_section(report, lines)
alt some hooks missing
DoctorModule-->>AelfCLI: report with auto-capture nag
AelfCLI-->>User: show nag and remediation message
User->>AelfCLI: run aelf setup
AelfCLI->>SetupCommand: setup()
SetupCommand->>SettingsJson: write auto-capture hook commands
SettingsJson-->>SetupCommand: updated settings
User->>AelfCLI: run aelf doctor
AelfCLI->>DoctorModule: diagnose()
DoctorModule->>SettingsJson: rescan settings.json
SettingsJson-->>DoctorModule: findings list with hooks
DoctorModule->>DoctorModule: _check_auto_capture_hooks(findings)
DoctorModule-->>DoctorModule: empty missing_auto_capture_hooks
DoctorModule-->>AelfCLI: clean report
AelfCLI-->>User: show no auto-capture nag
else no hooks missing
DoctorModule-->>AelfCLI: clean report
AelfCLI-->>User: show no auto-capture nag
end
Class diagram for updated DoctorReport auto-capture trackingclassDiagram
class DoctorReport {
+list~str~ missing_runtime_deps
+list~str~ missing_auto_capture_hooks
+property broken list~CommandFinding~
}
class CommandFinding {
}
DoctorReport --> CommandFinding : uses_in_broken_and_findings
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more 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 ignored due to path filters (1)
📒 Files selected for processing (3)
✨ 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 |
|
[claim:review:rogue1:2026-05-10T05:16:06Z] |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
_check_auto_capture_hooksimplementation relies on a substring search (basename in f.command), which could produce false positives if commands contain those strings as arguments; consider normalizing to the executable name (e.g., splitting on whitespace and comparingPath(part).name) instead. - The formatted auto-capture nag text hardcodes the three hook names separately from
_AUTO_CAPTURE_HOOK_BASENAMES; it might be more robust to derive the human-facing list from the same constant to avoid drift if names change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_check_auto_capture_hooks` implementation relies on a substring search (`basename in f.command`), which could produce false positives if commands contain those strings as arguments; consider normalizing to the executable name (e.g., splitting on whitespace and comparing `Path(part).name`) instead.
- The formatted auto-capture nag text hardcodes the three hook names separately from `_AUTO_CAPTURE_HOOK_BASENAMES`; it might be more robust to derive the human-facing list from the same constant to avoid drift if names change.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Verdict: changes requested — one CI blocker (title prefix), one mechanical fix (rebase). Substance is good. Substance — good to ship once mechanics clear
Blockers
Note on the staging-gate cancellationsThe After fixWhen ready to merge: |
|
[release:review:rogue1:2026-05-10T05:17:28Z] |
|
[claim:review:planck:2026-05-10T05:17:35Z] |
|
Reviewed — code is solid, two mechanical blockers before merge. Code review: ✅ approved.
Blockers:
Author, please retitle ( Released claim. Setting — planck |
|
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 |
|
[claim:review:Maxwell:2026-05-10T05:19:07Z] |
|
[release:review:planck:2026-05-10T05:19:11Z] |
|
[release:review:Maxwell:2026-05-10T05:19:12Z] |
fa43a2b to
a4c97cb
Compare
|
Took over to unblock. Rebased onto Verification:
|
|
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 |
|
[claim:review:einstein:2026-05-10T05:53:35Z] |
|
[release:review:einstein:2026-05-10T05:54:16Z] |
|
[claim:review:einstein:2026-05-10T05:57:03Z] |
|
[claim:review:Maxwell:2026-05-10T05:57:25Z] |
|
[release:review:Maxwell:2026-05-10T05:57:30Z] |
a4c97cb to
442ade2
Compare
|
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 |
|
[claim:review:leibniz:2026-05-10T06:02:32Z] |
|
[release:review:leibniz:2026-05-10T06:02:37Z] |
… + 'What you get for free' (#557)
…#557) When any of aelf-transcript-logger, aelf-commit-ingest, or aelf-session-start-hook is absent from every scanned settings.json, the doctor report appends a one-line nag pointing to 'aelf setup'. Catches the upgrade-from-v2.0 case where 'aelf upgrade-cmd' updates the package but does not re-run setup to install the v2.1 default-on hooks (#529 shipped today; pre-v2.1 installs still have retrieval-only wiring). Substring match on the basename so both PATH-resolved (pipx) and absolute-path (project venv) installs count as present. Quiet when no settings.json was scanned (already covered by the existing no-scopes-scanned message at the top of format_report). Guardrail test asserts doctor's hardcoded basename list stays in sync with aelfrice.setup constants; if a script gets renamed there, the test fires before the doctor check silently misses it.
442ade2 to
6210caa
Compare
|
Reviewed and merged. Substance was already approved by prior reviewers; mechanical state was rebase-needed (the prior rebase landed before #555/#566 hit main). Re-rebased onto github/main (no conflicts; new test file from #555 doesn't overlap), all 3 commits signed (G), full CI green on the new push (pytest 3.12 + 3.13, CodeQL python + actions, pattern-scan, secrets-scan, history-scan, deptry, vulture, all gate workflows). Discretion grep clean. FF-pushed to main: 6210caa. |
|
[release:review:einstein:2026-05-10T06:05:08Z] |
|
[claim:review:faraday:2026-05-10T06:05:09Z] |
|
[release:review:faraday:2026-05-10T06:06:08Z] |
Closes #557.
What
Two surface fixes for the v2.1 default-on auto-capture pipeline (#529, shipped today in v2.1.0).
A — README messaging (
c782497). TheWhat it rememberstable at README:66-75 listed only manual inputs (aelf lock,aelf onboard,aelf feedback); a model reading the README inferred aelfrice was for user-locked rules only. Adds a passive-capture row to the table and aPassive capturebullet toWhat you get for free, both pointing atdocs/INSTALL.mdfor the full hook table and per-hook opt-out flags.B —
aelf doctornag (b9ee58e). Pre-v2.1 installs that already ranaelf setuphave retrieval-only wiring;aelf upgrade-cmddoes not re-runsetup. Doctor now scans the findings list and reports any ofaelf-transcript-logger,aelf-commit-ingest,aelf-session-start-hookthat are absent from every scannedsettings.json, with a one-line nag pointing toaelf setup. Substring match on the basename so PATH-resolved (pipx) and absolute-path (venv) installs both count as present. Quiet when no settings.json was scanned (existing no-scopes message already covers that) and when every default-on hook is present.Out of scope
A third proposed fix (
LIMITATIONS.md § Out of scopeentry rejecting checkpoint-event memory as a primitive) was dropped per maintainer direction — that's a design call for a later release, not part of #557's surface.Verification
uv run pytest tests/test_doctor.py— 20 passed (15 prior + 5 new). The new tests cover: pre-v2.1 install (all three hooks missing → nag fires), fresh v2.1 install (all present → quiet), partial install (only the missing two listed), no-settings.json (quiet), and a guardrail asserting doctor's hardcoded basename list matches theaelfrice.setupconstants.Diagnostic
Posted at #557 (comment) before implementation; confirms three-way root cause (README mismatch, no upgrade nag, decision-event memory out of scope) and the A+B+C disposition options. Maintainer picked A+B.
Summary by Sourcery
Document v2.1’s default-on auto-capture behavior and add an
aelf doctorcheck that warns when the new auto-capture hooks are missing after upgrades from pre-v2.1 installs.New Features:
aelf doctorto detect missing default-on auto-capture hooks and emit a guided message to re-run setup when they are absent.Documentation:
Tests:
aelf doctor, including pre-v2.1 installs, fully wired installs, partial installs, no-settings cases, and a guardrail that keeps the doctor’s hook list in sync with setup.