feat(doctor): nag when per-project DBs are on pre-v1.x schema (#589) - #599
Merged
robotrocketscience merged 6 commits intoMay 10, 2026
Merged
Conversation
|
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 (4)
✨ 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 |
Reviewer's GuideExtends Sequence diagram for aelf doctor legacy-schema DB detectionsequenceDiagram
actor User
participant CLI as aelf_cli
participant Doctor as doctor_diagnose
participant LegacyScanner as _check_legacy_schema_dbs
participant SQLite as sqlite3
User->>CLI: invoke aelf doctor
CLI->>Doctor: diagnose(aelfrice_projects_dir=None)
Doctor->>LegacyScanner: _check_legacy_schema_dbs(projects_dir=_AELFRICE_PROJECTS_DIR)
alt projects_dir exists
loop for each projects_dir/*/memory.db
LegacyScanner->>SQLite: connect(file:memory.db?mode=ro)
SQLite-->>LegacyScanner: connection or error
alt connection ok
LegacyScanner->>SQLite: PRAGMA table_info(beliefs)
SQLite-->>LegacyScanner: beliefs columns
alt beliefs table missing
LegacyScanner-->>LegacyScanner: skip DB
else beliefs table present
alt origin column present
LegacyScanner-->>LegacyScanner: skip DB (modern schema)
else origin column absent
LegacyScanner->>SQLite: SELECT COUNT(*) FROM beliefs
SQLite-->>LegacyScanner: row_count
alt row_count > 0
LegacyScanner-->>LegacyScanner: compute idle_days from mtime
LegacyScanner-->>Doctor: append LegacySchemaDB(path,row_count,idle_days)
else row_count == 0
LegacyScanner-->>LegacyScanner: skip empty DB
end
end
end
else connection error
LegacyScanner-->>LegacyScanner: skip unreadable DB
end
end
else projects_dir missing
LegacyScanner-->>Doctor: []
end
Doctor-->>CLI: DoctorReport with legacy_schema_dbs
CLI->>CLI: format_report(report)
CLI->>CLI: _format_legacy_schema_section(report,lines)
alt legacy_schema_dbs nonempty
CLI-->>User: text report including legacy-schema per-project DBs block
else no legacy_schema_dbs
CLI-->>User: text report without legacy-schema block
end
Class diagram for DoctorReport legacy schema trackingclassDiagram
class LegacySchemaDB {
+Path path
+int row_count
+int idle_days
}
class DoctorReport {
+list~LegacySchemaDB~ legacy_schema_dbs
+list~CommandFinding~ findings
+list~str~ missing_auto_capture_hooks
+list~str~ missing_runtime_deps
+list~str~ hook_failures_tail
+property broken
+property ok
}
DoctorReport "1" o-- "*" LegacySchemaDB
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
robotrocketscience
added
author-Godel
6 tasks
![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 left some high level feedback:
- In
_check_legacy_schema_dbsyou broadly swallow any exception (both on connect and during queries); consider narrowing the exception types or at least logging/debug-logging failures so corrupted or unreadable DBs are diagnosable instead of silently skipped. - The
glob('*/memory.db')scan in_check_legacy_schema_dbsassumes a single-level project ID directory layout; if that ever changes, this will quietly stop detecting some DBs, so it might be safer to either make the pattern configurable or explicitly validate the directory structure before scanning.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_check_legacy_schema_dbs` you broadly swallow any exception (both on connect and during queries); consider narrowing the exception types or at least logging/debug-logging failures so corrupted or unreadable DBs are diagnosable instead of silently skipped.
- The `glob('*/memory.db')` scan in `_check_legacy_schema_dbs` assumes a single-level project ID directory layout; if that ever changes, this will quietly stop detecting some DBs, so it might be safer to either make the pattern configurable or explicitly validate the directory structure before scanning.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
[claim:review:godel:2026-05-10T15:31:25Z] |
Owner
Author
|
[release:review:godel:2026-05-10T15:31:50Z] |
Owner
Author
|
[claim:review:noether:2026-05-10T15:32:53Z] |
|
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 |
TDD-first: add test_legacy_schema_detected, test_legacy_schema_report_block_present, test_legacy_schema_report_quiet_when_zero, test_legacy_schema_quiet_when_projects_dir_missing, and test_legacy_schema_idle_days_in_report against the not-yet-implemented _check_legacy_schema_dbs function. Tests fail at import until the next commit lands the implementation.
Add LegacySchemaDB dataclass, _AELFRICE_PROJECTS_DIR constant, and _check_legacy_schema_dbs() that enumerates ~/.aelfrice/projects/*/memory.db, opens each read-only, checks PRAGMA table_info(beliefs) for the `origin` column, and flags DBs with rows but no origin column as pre-v1.x legacy. Empty DBs and DBs with the modern schema are silently skipped. Connection errors skip silently (doctor is read-only diagnostic). Wire into DoctorReport.legacy_schema_dbs field and populate it in diagnose() alongside missing_auto_capture_hooks. Add aelfrice_projects_dir kwarg to diagnose() so tests can redirect the scan path away from the user's real ~/.aelfrice/projects.
Add _format_legacy_schema_section() mirroring the #557 _format_missing_auto_capture_section() pattern: quiet when legacy_schema_dbs is empty, otherwise appends a nag block listing each flagged DB with row count and idle days, plus the fix: line pointing at `aelf migrate`. Wire the call into format_report() adjacent to the existing _format_missing_auto_capture_section() call.
Replace help=argparse.SUPPRESS with a real one-line help string so the subcommand appears in `aelf --help` output. `aelf doctor` now points users at `aelf migrate` when legacy-schema DBs are detected; the command must be discoverable from `--help` for that fix line to be actionable.
Add a 'Legacy-schema detection' subsection under 'Hooks installed by aelf setup' showing the nag-block format, when it fires (quiet for zero matches), and the aelf migrate invocation to resolve each flagged DB. Parallel to the v2.1 hook-docs paragraph added by #557.
Add [Unreleased] ### Added bullet documenting the new legacy-schema per-project DB scan in aelf doctor and the aelf migrate unhide.
robotrocketscience
force-pushed
the
feat/issue-589-legacy-schema-doctor-nag
branch
from
May 10, 2026 15:36
5b67fdc to
ef74142
Compare
Owner
Author
ReviewRecommendation: approve. Rebased onto current Code review
Tests
Verification
ActionReleasing claim. The FF push to |
Owner
Author
|
[release:review:noether:2026-05-10T15:37:47Z] |
Owner
Author
|
[claim:review:noether:2026-05-10T15:39:30Z] |
Owner
Author
|
[release:review:noether:2026-05-10T15:39:53Z] |
Owner
Author
|
[claim:review:planck:2026-05-10T15:39:55Z] |
Owner
Author
|
[release:review:planck:2026-05-10T15:40:55Z] |
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 #589.
What lands
Extends
aelf doctorwith a per-project-DB schema check that mirrors the existing #557 auto-capture-hook nag pattern. When pre-v1.x per-project DBs (noorigincolumn onbeliefs) are detected under~/.aelfrice/projects/*/memory.db, the doctor report appends alegacy-schema per-project DBs detectedblock listing each with row count + idle days.aelf migrateis unhidden fromargparse.SUPPRESSso the fix line is discoverable.Operator decisions (resolved on the issue before this branch was claimed):
~/.aelfrice/projects/*/memory.db, not current-project-only.aelf migrate: yes —cli.py:4014help=argparse.SUPPRESS→ real help string.What you get
Block is quiet when zero legacy DBs are found (parity with #557 quietness rules).
Atomic commits
All signed.
Acceptance criteria
aelf doctorenumerates per-project DBs and identifies any withbeliefsrows but noorigincolumnlegacy-schema per-project DBs detectedblock listing each with row count + idle daysaelf migrateunhidden from argparsedocs/INSTALL.md§ "Hooks installed byaelf setup" (parallel to v2.1 hook docs)tests/test_doctor.py::test_legacy_schema_detected— temp legacy DB → block present; modern DB → block absentOut of scope
Verification
uv run pytest -x -q→ 3287 passed, 53 skipped (62.68s). Targeted: 25 doctor tests, all green. Discretion grep ongithub/main..HEADclean.Summary by Sourcery
Add
aelf doctorsupport for detecting legacy per-project databases using a pre-v1.x schema and surface a migration hint via a visibleaelf migratecommand.New Features:
aelf doctorto scan per-project SQLite databases for legacy schemas lacking theorigincolumn and include findings in the doctor report when present.aelf migratesubcommand in the CLI help to guide users in migrating legacy per-project databases.Enhancements:
Documentation:
aelf doctorand how to migrate affected databases in INSTALL.md.aelf migrate.Tests: