core: preserve genie runtime prompt context - #87
Conversation
Switches the `Contribution / PR body checklist` workflow trigger from `pull_request` to `pull_request_target` so the check fires on every PR — including PRs whose head branch was created before the workflow itself existed. Before this fix, `on: pull_request:` resolved the workflow file from the PR's head ref. PRs opened before #88 landed (#78, #83, #87) therefore had no `.github/workflows/contribution.yml` on their head and the workflow never scheduled — they showed 5 checks instead of 6. `pull_request_target` reads the workflow file from the **base** ref instead. Once this lands on `main`: - Every new PR fires the check immediately. - Existing open PRs fire the check on the next push to head / edit to body / reopen — no rebase required on the contributor's branch. Safe to use `pull_request_target` here even though it runs with secret access. The job only reads `${{ github.event.pull_request.body }}` and `${{ github.event.pull_request.title }}` from the event payload — never checks out PR code, never executes PR-controlled scripts. The `permissions:` block stays at `contents: read` + `pull-requests: read` so even the granted access is minimal. The check is absent on this PR's own CI run by design: main still has `pull_request` (the old trigger), and this PR's head has `pull_request_target` only — neither side matches, so the job doesn't fire. Self-verification will land on the next PR after this one merges, plus an empty-commit demonstration on PR #87 immediately after. All 5 other CI checks green on `9a5135c` (fmt, clippy, test, aarch64 cross-compile, `--no-default-features`).
ai-hpc
left a comment
ai-hpc
left a comment
There was a problem hiding this comment.
Closes #85 with the structurally-right fix, plus a bonus that closes the chat-blocks-health symptom that wasn't a filed issue but was the real reason dashboard polls felt frozen during chat. Two commits:
Commit 1 — runtime prompt compaction (a26a25a)
Replaces the original PR #74 "compact to a single user turn + hardcoded blurb" with the design the PR description actually claimed:
- Preserves all system messages (tool manifest + household-preference injection) so the LLM knows what tools exist and what facts to recall. The reason #85 fired in the first place.
- Keeps the latest user turn untouched.
- Walks older non-system history backwards from the latest turn and retains as many older user/assistant pairs as fit in the byte budget. This is the "system blurb + recent N turns" shape the PR #74 body had described but didn't implement.
- Raises
GENIE_RUNTIME_MAX_BODY_BYTESfrom4 KB → 24 KBto match the 8192-token INT8-KV runtime context from #76 (the rough-budget concern I flagged on #85). NewGENIE_RUNTIME_BODY_OVERHEAD_BYTES = 768reservation for JSON envelope + serialization overhead, so the budget math doesn't quietly overshoot. - Falls back gracefully: if there's no user turn at all, returns the system messages alone (test
genie_runtime_compaction_falls_back_to_latest_non_system_messagenow asserts this preserves both system context and the latest non-system turn).
estimate_message_bytes uses role.len() + content.len() + 32 — the +32 is a JSON-envelope nudge that matches the actual serialized overhead per message reasonably well. Cheap to compute, and the budget is enforced inclusively (> body_budget breaks the loop) so we stay under the cap.
The Jetson acceptance the author noted in the PR body is the smoking gun: /api/chat returned Your name is Jared with tool=memory_recall — which is exactly what #85's reproduction was missing. Before this PR, the LLM had no way to know memory_recall existed because the tool manifest was getting compacted away.
Commit 2 — concurrent request handling (a77d67b)
Quieter but useful. Changes ChatServer::serve from sequential accept().await; handle(...).await to tokio::task::LocalSet::spawn_local so multiple requests can be in-flight on the single OS thread. Adds a chat_turn_lock: Mutex<()> that explicitly serializes only the chat-turn endpoints (POST /api/chat, POST /api/chat/stream, POST /v1/chat/completions). /api/health, /api/services, /api/memories, runtime contract, history list, etc. now run concurrently with an in-flight chat turn.
Net effect: the dashboard's 5s polling against /api/health and /api/services stays responsive even when an LLM call is taking 4-8 seconds. Previously the whole HTTP server was blocked behind the LLM call (the old comment claimed "LLM calls are seconds, HTTP is microseconds" — true but irrelevant when the queue is serialized).
The change is correctly scoped — chat turns still serialize via chat_turn_lock, so concurrent chat requests from two users still queue up (which is what we want; the LLM backend can only handle one prompt at a time anyway and parallel chat requests would just thrash the KV cache).
Knock-on refactor that makes the diff cleaner:
- Inline
(method, path)tuple matching at every route →classify_route()returning aRequestRoute<'a>enum. Routes are now named (RequestRoute::Chat,::Health, etc.) and the dispatch table reads top-to-bottom without re-spelling method/path tuples. - 4× repeated
if matches!(request_origin, Unknown) { Api } else { request_origin }ternary →normalized_origin(request_origin)helper. with_chat_turn_lock(lock, fut)helper encapsulates the "lock then await" pattern for chat-turn endpoints.
ChatServer::serve signature change from &self to self (because the body is Rc::new(self) for spawn_local) — this is a breaking change for any external caller, but the only call sites are in crates/genie-core/src/main.rs (4 mutually-exclusive branches that each move once). Verified via the cargo clippy + test (--no-default-features) and cargo test checks both being green.
The tests are updated correctly:
genie_runtime_profile_compacts_large_core_promptnow expects 2 messages (system + user) instead of 1; verifies "memory_recall" and "household context" survive into the wire payload.- New
genie_runtime_profile_keeps_runtime_prompt_under_expanded_budgetcovers the "real-sized 10 KB prompt should NOT compact at all under the new 24 KB budget" case — this is the test that would catch a regression where someone shrinksGENIE_RUNTIME_MAX_BODY_BYTESback toward 4 KB. genie_runtime_compaction_falls_back_to_latest_non_system_messageupdated for the 2-message output shape.
Worth flagging, neither blocking:
- Token-budget is byte-based, not token-based.
GENIE_RUNTIME_MAX_BODY_BYTES = 24 KBis a rough proxy for "fits in 8192 token context after JSON overhead". For typical English chat text (~4 bytes/token) that's ~6k tokens before overhead, comfortable. For CJK / code-heavy content (denser bytes/token) it could over-fit. Right path eventually is to tokenize on the client side or probe runtime capacity. The current static value is defensible for an alpha. chat_turn_lockis a process-wide singleton, so even semantically-different chat endpoints (web/api/chatvs OpenAI-compatible/v1/chat/completions) serialize behind each other. That's the right call given the single shared LLM backend, but worth noting if anyone later wants per-conversation parallelism.LocalSet+Rcis a single-thread design. That's appropriate for a Jetson appliance with <10 concurrent users, but if someone ever wants to push this to many-thread / many-tenant deployments, the Mutex needs to becometokio::sync::Mutex(it already is) and theRcneeds to becomeArc. Worth a one-line note inserve's doc comment, but the doc comment already says "current-thread runtime" which is honest enough.
All 6 CI checks green on a77d67b (fmt, clippy, test, aarch64 cross-compile, --no-default-features, PR body checklist). Going in.
|
Merged at |
Raises the `genie-core` release binary size budget from the alpha-era `5.0 MB` to `6.0 MB`. The 5 MB ceiling was set when `genie-core` was much smaller; legitimate growth since then (LLM backend facade #35-#43, voice cargo feature #57, telegram voice in/out #53/#64, runtime_mode module #72, LocalSet concurrent server #87, per-call STT nonce #68) pushed the binary to `5.07 MB`, making the test a known-flaky drag on every recent PR. The new ceiling is intentionally tight: ~0.93 MB of headroom over the current 5.07 MB. That's enough to absorb the next legitimate growth bump but small enough that a future PR adding ~1 MB of dependencies or modules will trip the assert and force a deliberate raise-or-shrink decision — exactly what a size budget should do. Implementation: - Extracted `RELEASE_BINARY_SIZE_BUDGET_MB: f64 = 6.0` constant with a `///` doc comment explaining the alpha-era origin, the load-bearing growth, and the "keep it tight to force deliberate decision" principle. - Assertion message now echoes the budget back to the failure output, so future contributors hitting the assert know exactly which constant to inspect without grepping. All 6 CI checks green on `ea85c53` (fmt, clippy, test, aarch64 cross-compile, `--no-default-features`, PR body checklist).
2 participants
Summary
Fixes #85
Testing
Real Behavior Proof
/api/chatreturnedYour name is Jaredwithtool=memory_recallafter deploying this branch.Jetson