Fix importance scheduler and startup timeouts - #65
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe importance scheduler now uses ChangesScheduler updates
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ServerMode
participant DelayedStartupTask
participant backup_cron
participant importance_scheduler
ServerMode->>DelayedStartupTask: Schedule startup task
DelayedStartupTask->>DelayedStartupTask: Wait five seconds
DelayedStartupTask->>backup_cron: Start scheduler
DelayedStartupTask->>importance_scheduler: Start scheduler
DelayedStartupTask->>ServerMode: Log startup completion
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThe PR updates importance scheduling to use the production database identifiers and delays background-service startup by five seconds.
Confidence Score: 3/5The PR is not safe to merge until dashboard startup is moved into a running event loop and the scheduler test schemas are updated. Dashboard mode currently fails before Uvicorn starts, while the scheduler identifier changes leave its in-memory tests querying columns and tables that their fixtures do not define. Files Needing Attention: mcp_server/server.py, lifecycle/importance_scheduler.py
|
| Filename | Overview |
|---|---|
| lifecycle/importance_scheduler.py | Corrects production schema identifiers, but the corresponding test fixtures still define the old column and table names. |
| mcp_server/server.py | Delays background tasks successfully inside async lifespan, but dashboard mode calls asyncio.create_task before Uvicorn starts an event loop. |
Sequence Diagram
sequenceDiagram
participant Main
participant Dashboard as _run_with_dashboard
participant Asyncio
participant Uvicorn
Main->>Dashboard: synchronous call
Dashboard->>Asyncio: create_task(_delayed_start())
Asyncio-->>Dashboard: RuntimeError: no running event loop
Note over Dashboard,Uvicorn: uvicorn.run() is never reached
Prompt To Fix All With AI
### Issue 1
mcp_server/server.py:189
**Dashboard task lacks event loop**
When dashboard mode starts through the synchronous entrypoint, `asyncio.create_task()` executes before `uvicorn.run()` creates an event loop, causing `RuntimeError: no running event loop` and preventing the server from starting.
### Issue 2
lifecycle/importance_scheduler.py:99
**Scheduler fixtures retain old schema**
When the scheduler tests run, this query selects `entry_id` from a fixture that still defines `core_memory.id`, causing `sqlite3.OperationalError: no such column: entry_id`; the retrieval-signal fixture likewise writes to `audit_trail` while the changed code reads `audit_log`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix: importance scheduler table names an..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async def _delayed_start(): | ||
| await asyncio.sleep(5) | ||
| backup_cron.start() | ||
| importance_scheduler.start() |
There was a problem hiding this comment.
When dashboard mode starts through the synchronous entrypoint, asyncio.create_task() executes before uvicorn.run() creates an event loop, causing RuntimeError: no running event loop and preventing the server from starting.
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp_server/server.py
Line: 189
Comment:
**Dashboard task lacks event loop**
When dashboard mode starts through the synchronous entrypoint, `asyncio.create_task()` executes before `uvicorn.run()` creates an event loop, causing `RuntimeError: no running event loop` and preventing the server from starting.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| rows = await ( | ||
| await conn.execute( | ||
| """SELECT id, "key", value, importance, memory_kind | ||
| """SELECT entry_id, "key", value, importance, memory_kind |
There was a problem hiding this comment.
When the scheduler tests run, this query selects entry_id from a fixture that still defines core_memory.id, causing sqlite3.OperationalError: no such column: entry_id; the retrieval-signal fixture likewise writes to audit_trail while the changed code reads audit_log.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lifecycle/importance_scheduler.py
Line: 99
Comment:
**Scheduler fixtures retain old schema**
When the scheduler tests run, this query selects `entry_id` from a fixture that still defines `core_memory.id`, causing `sqlite3.OperationalError: no such column: entry_id`; the retrieval-signal fixture likewise writes to `audit_trail` while the changed code reads `audit_log`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.2 participants
Addresses 'no such table: audit_trail' and 'no such column: id' regressions. Delays background tasks by 5s to ensure clean MCP handshake.
Summary by CodeRabbit
Bug Fixes
Reliability