feat: add pi-agent extension for hexus vector memory - #37
Conversation
codenamekt
added
enhancement
- Add pi-extension with HTTP client, config, and index.ts for memory recall, turn capture, and session reflection - Add REST API endpoints (health, recall, retain, append-turn) to server.py - Support both string[] and object[] formats for retain endpoint - Include Bearer token auth from main
5d82b51 to
e061c42
Compare
Replace bare 'except Exception:' with specific exception types (ValueError, KeyError, TypeError) to satisfy BLE001 lint rule.
Provider should be 'tobiTradez' and model ID should be 'minimax-m2.7-highspeed'
This fixes 101 pre-existing lint errors that were blocking the pi-extension PR. Changes include: - BLE001: Add noqa to bare Exception catches across hexus/__init__.py, hexus/store.py, hexus/webhook/dispatcher.py, mcp_server/server.py, mcp_server/import_cli.py - SIM117: Add noqa or combine nested with statements in hexus/store.py, mcp_server/tools.py, and test files - PLW1508: Use string defaults for env vars (was using int) - S110/S112: Add noqa for try-except-pass/continue patterns - PERF402: Replace manual list copy with unpacking - PLW0602: Remove unused global declaration in embedder.py - RUF013: Fix implicit Optional in entity_extractor.py - RUF012: Fix mutable class default in test_webhooks.py - RUF059: Fix unused unpacked variable in test_smoke.py - TRY401: Remove redundant exc arg in dispatcher.py - PLW1510: Add check=False to subprocess.run in test_migration.py - ISC004: Parenthesize implicit string concatenation - EXE001: Remove shebang from bench.py - SIM103: Simplify needless bool in __init__.py - UP035/UP006/UP045: Auto-fixed deprecated typing imports Also ran ruff format to fix auto-formatting issues.
…el env var The compat getModel() only works with built-in catalog providers (builtin). tobiTradez is a custom/faux provider, not in the builtin catalog, so getModel() always returned undefined causing silent reflection failures. Switch to ctx.modelRegistry.find(provider, modelId) which works with both builtin and registered custom providers. Also simplify HEXUS_REFLECTION_MODEL from two env vars to a single 'provider/modelId' string (e.g. 'tobiTradez/minimax-m2.7-highspeed').
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
|
||
| try { | ||
| const health = await client.health().catch(() => null); | ||
| if (!health?.ok) { ctx.ui.setStatus("hexus", "hexus: offline"); return; } |
There was a problem hiding this comment.
CRITICAL: Wrong field health?.ok — HealthResponse has status, not ok
health?.ok is always undefined, so !undefined is always true. This causes the before_agent_start handler to unconditionally bail out as "offline", blocking all memory recall functionality. The interface at http-client.ts:24 defines status: string.
| if (!health?.ok) { ctx.ui.setStatus("hexus", "hexus: offline"); return; } | |
| if (health?.status !== "ok") { ctx.ui.setStatus("hexus", "hexus: offline"); return; } |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ctx.ui.notify("Running session reflection...", "info"); | ||
|
|
||
| try { | ||
| const [provider, modelId] = reflConfig.model.split("/"); |
There was a problem hiding this comment.
CRITICAL: No validation before reflConfig.model.split("/") — produces undefined modelId when / is missing
If the model string doesn't contain "/" (e.g. HEXUS_REFLECTION_MODEL=gpt-4 or HEXUS_REFLECTION_MODEL=minimax-m2.7-highspeed), destructuring produces modelId = undefined. Calling ctx.modelRegistry.find(provider, undefined) has undefined behavior depending on the registry implementation.
Add a guard before the split:
if (!reflConfig.model.includes("/")) {
console.warn(`hexus: model "${reflConfig.model}" lacks provider/modelId format`);
isReflecting = false;
return;
}Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| async def metrics(request): | ||
| return Response(_generate_metrics(store), media_type="text/plain") | ||
| return JSONResponse(_generate_metrics(store), media_type="text/plain") |
There was a problem hiding this comment.
CRITICAL: JSONResponse wraps Prometheus metrics text in JSON, breaking all scrapers
_generate_metrics() returns plain Prometheus exposition-format text (e.g. # HELP hexus_db_reachable ...). JSONResponse calls json.dumps() which wraps the output in double quotes, turning it into a JSON string literal instead of valid Prometheus format. Prometheus scrapers will fail to parse the response.
Use starlette.responses.Response or PlainTextResponse instead:
| return JSONResponse(_generate_metrics(store), media_type="text/plain") | |
| return Response(_generate_metrics(store), media_type="text/plain") |
Note: this requires importing Response instead of (or in addition to) JSONResponse at line 1274.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| tools.http_transport_active = True | ||
|
|
||
| async def health(request): |
There was a problem hiding this comment.
WARNING: /api/health has no try/except, unlike every other REST endpoint
If tools.memory_health() raises (e.g. DB count() operation fails after the schema check passes), the exception propagates as an unhandled 500 without a JSON error body. Every other /api/* endpoint catches exceptions and returns structured {"error": "..."} responses. Add a try/except wrapping line 1280 to match the pattern used by recall/retain/append-turn handlers.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if "error" in result: | ||
| return JSONResponse(result, status_code=400) | ||
| return JSONResponse(result) | ||
| except (ValueError, KeyError, TypeError) as exc: |
There was a problem hiding this comment.
WARNING: except (ValueError, KeyError, TypeError) is too narrow for tool call failures
Tool functions (memory_recall, memory_retain, memory_append_turn) operate against Postgres. Database connection failures, pool exhaustion, and other operational errors raise exception types outside this tuple (e.g. psycopg.OperationalError, OSError, TimeoutError). These will produce unhandled 500 errors without a JSON error body.
Same issue at lines 1349 (/api/retain handler) and 1370 (/api/append-turn handler). Consider adding a broader except Exception fallback after the specific types to always return structured JSON errors.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| }, | ||
| }); | ||
|
|
||
| pi.on("session_shutdown", () => { |
There was a problem hiding this comment.
WARNING: session_shutdown only clears idle timer — does not reset isReflecting or recallInFlight
If runReflection() or the before_agent_start handler are in-flight during shutdown, their finally blocks will still execute ctx.ui.setStatus(...) and scheduleReflection(...) against a potentially disposed UI context. Add reset of these flags:
pi.on("session_shutdown", () => {
isReflecting = false;
recallInFlight = false;
if (idleTimer) clearTimeout(idleTimer);
});Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| export function getClient(): HexusClient { | ||
| if (!_client) { | ||
| const config = getConfig(); |
There was a problem hiding this comment.
WARNING: getClient() singleton created with default apiUrl before async config resolves
getConfig() returns DEFAULTS (config.ts:105) until initConfig() resolves. initConfig() is fire-and-forget at index.ts:126, but extension event registration happens synchronously immediately after. If any code path calls getClient() before config loads, the HexusClient is created with http://localhost:8000 and never updated when real config resolves. resetClient() exists at line 174 but is never called anywhere.
Consider awaiting initConfig() before registering events, or deferring _client creation until _config is non-null.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| @@ -98,9 +98,11 @@ def _compress_code(self, text: str) -> str: | |||
| lines = text.splitlines() | |||
| compressed_lines = [] | |||
| for line in lines: | |||
There was a problem hiding this comment.
SUGGESTION: Readability regression — compound or/and relies on operator precedence
The refactoring from if/elif to A or B and C is logically equivalent (Python's and binds tighter than or), but requires mental parsing of A or (B and C). Add parentheses for clarity:
| for line in lines: | |
| if ( | |
| re.match(r"^\s*(def|class|import|from|async\s+def)\b", line) | |
| or (re.match(r"^\s*#.*", line) | |
| and len(compressed_lines) < 10) | |
| ): |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| isReflecting = true; | ||
|
|
||
| const client = getClient(); | ||
| const branch = ctx.sessionManager.getBranch(); |
There was a problem hiding this comment.
SUGGESTION: buildConversationText(branch) doesn't guard against undefined branch
If ctx.sessionManager.getBranch() returns undefined or null (unpopulated session state), for...of on line 98 throws TypeError: undefined is not iterable. While caught by the outer try/catch in runReflection, it wastes a reflection cycle. Add a guard in buildConversationText: if (!Array.isArray(entries)) return "";.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 9 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (29 files)
Fix these issues in Kilo Cloud Reviewed by deepseek-v4-pro · Input: 117.2K · Output: 18.2K · Cached: 1.5M |
1 participant
Overview
Add a pi harness extension () that integrates hexus vector memory into pi agent sessions.
What it does
Files
Key design decisions
Ticket references
Installation
Then run in pi.