Skip to content

Fix ten data-loss and wrong-output bugs - #423

Merged
dovvnloading merged 11 commits into
mainfrom
fix/data-loss-and-wrong-output
Oct 30, 2026
Merged

dovvnloading merged 11 commits into
mainfrom
fix/data-loss-and-wrong-output

Conversation

@dovvnloading

Copy link
Copy Markdown
Owner

Problem

Ten defects that either lose user data or present wrong output. Each one was reproduced against main before it was fixed.

  1. knowledge.db backups. prune_backups parsed knowledge.db backups under the chats.db prefix. The "one snapshot per day, forever" policy silently kept only the 10 most recent.
  2. Reply lost when its message is deleted. If the user deleted their message while the reply was generating, a reply that succeeded was reported as "AI response failed".
  3. HTML node edits. Edits lived only in component state. Panning the node off-screen discarded them, and they were never saved.
  4. Group drag froze the canvas. Dragging a container or frame that had members stopped the canvas applying scene updates. New nodes never appeared, streamed replies never updated, and deleted nodes stayed on screen.
  5. llama.cpp model pin ignored. A node pinned to one configured llama.cpp model was answered by the other one and badged as the pinned one.
  6. Truncated streams committed. Gemini, OpenAI-compatible and Ollama streams that stopped without a finish marker (a proxy, a gracefully closed connection) committed half a reply as complete.
  7. Infinite token counts. A token count of 1e999 (JSON for infinity) raised OverflowError at the done event and discarded an already-streamed reply.
  8. Undo then Send. Undoing a Send left the continuation pointer on the removed node, so the next Send became a detached root.
  9. Reopened chats. The continuation pointer was never saved, so the first Send after reopening a chat also became a detached root.
  10. Images in the context budget. Images were budgeted at about 48 tokens instead of about 1,600, so image-heavy histories silently overflowed the context window.

Change

One commit per fix:

  • Backups: retention parses under the prefix it listed with; the private parser no longer has a default prefix to fall back to.
  • Infinite numbers: OverflowError is tolerated wherever untrusted numbers are coerced to int: normalize_usage, the model catalog, and session_load's budget, counter and session-total fields. A corrupted saved chat now opens instead of failing.
  • llama.cpp pin: the pin resolves to its configured file, and the provider runs that file for every task. describe_active_model / active_provider_model accept the resolved ref.
  • Stream completion: Gemini, OpenAI-compatible and Ollama now require their own finish marker and raise the same unexpected-end error Anthropic does, so the partial lands flagged incomplete. The OpenAI/Gemini test fakes now send the finish marker real servers send.
  • Image budgeting: images are priced by the documented vision formula from their header dimensions, and trim_history reports exactly the tokens it returns. The summary cache keys images by digest, and the context meter adds the same per-image figure. Audio attachments are still budgeted by their path; that needs per-provider rates and is a separate change.
  • Chat pointers: Command snapshots last_chat_node_id and final_deliverable_node_id, so undo/redo restore them and Mark Final is a real undoable command. remove_nodes re-points like delete_chat_node, and the pointer is saved and restored like final_deliverable_node_id.
  • Replies: send, artifact and gitlink callbacks are a silent no-op when their node was deleted mid-flight. A gitlink apply no longer reports failure for files it already wrote. Reply provenance and cost use the request-time pair, resolved through the same pin rungs as the dispatch.
  • Group drag: member changes carry the drag flag of the change that produced them.
  • HTML / note drafts: a new undoable scene/setHtmlContent intent. The card commits on Render, blur and Ctrl+S, ignores the echo of its own commit, and shows an undo. The note editor also flushes its draft on Ctrl+S, because the save key bypasses the while-typing gate.

Test plan

  • Every fix has regression tests. Each new test was run against the pre-fix code and fails there.
  • Backend, matching CI (Python 3.12, lockfile dependencies, CI=1): 3,493 passed, 20 skipped, coverage 86.6%. Perf suite 8 passed. ruff and mypy (361 files) clean.
  • npm run check: codegen drift, tsc and ESLint clean (0 errors); vitest 2,250 passed; build ok; bundle gate ok (largest chunk 495,189 / 512,000).
  • Live, against the built app on a throwaway data dir: setHtmlContent stores the edit, undo restores the original, and redo re-applies it.
  • The group drag was not driven live: the verification browser could not composite frames, so React Flow never measured the nodes. It is covered by a hook test through the real middleware and onNodesChange, and by a canvas test that applies a scene update after a group-with-members drag.

dovvnloading and others added 11 commits October 24, 2026 10:18
prune_backups passed its prefix to list_backups but parsed every
timestamp with _parse_backup_timestamp's default, the chats.db prefix.
Every knowledge- backup therefore parsed as None: no day was ever
covered, and the daily-forever bucket deleted everything older than the
10 most recent snapshots. 14 knowledge- backups on 14 distinct days lost
the 4 oldest; the same fixture under chats- lost none.

Both retention loops now parse under the prefix list_backups matched on,
and the private parser no longer has a default to fall back to, so a
bare call fails at the call site instead of silently misreading. Tests
cover the distinct-days and same-day-burst policies for both prefixes and
a custom-prefix take_backup.

Co-Authored-By: Claude <[email protected]>
JSON 1e999 parses to float('inf'), and int(inf) raises OverflowError,
which none of the int coercions caught:

- normalize_usage runs at every provider's done event, after the reply
  has already streamed, so one infinite token count from a server or
  proxy discarded the whole reply.
- graphlink_model_catalog._as_int failed an Ollama model descriptor the
  same way.
- session_load's budget, counter, elapsedMs and total_session_tokens
  coercions failed the restorer (or the whole chat, for the session
  total) on a corrupted or hand-edited row, since chats.db rows are
  written with json.dumps' default allow_nan=True.

Each now treats OverflowError like the TypeError/ValueError it already
tolerated. Tests cover inf, -inf, nan, Decimal('Infinity') and huge
finite values for both coercers, and a saved plan, harness and chat
carrying infinite numbers.

Co-Authored-By: Claude <[email protected]>
A llama.cpp ModelRef pin was validated against the two configured
slots' basenames, then LlamaCppProvider was built with no model at all,
and every llama_cpp_runtime helper resolves the model by task. A chat
node pinned to the title slot's model was answered by the chat slot's
model and attributed to the pinned one, so a side-by-side comparison
compared a model with itself.

_provider_for_model_ref now resolves the pin to its configured full path
and LlamaCppProvider points both slots of its own settings copy at it,
so the client cache, reasoning detection and every task use the pinned
model. Two different files sharing the pinned basename are refused as
ambiguous; the same file reached through a link is not. The
describe_active_model provenance pair and AgentDispatcher's
active_provider_model accept the resolved ref, so the reply badge and
cost lookup can name the model that actually ran.

Co-Authored-By: Claude <[email protected]>
AnthropicProvider raises when a stream ends without message_stop, and
the dispatch layer then lands the streamed partial flagged incomplete.
Gemini, OpenAI-compatible and Ollama streams had no such check: a proxy
or a gracefully closed connection that ended the stream mid-answer
committed half a reply as the finished turn.

Each now records its provider's own finish marker - Gemini's
candidates[].finishReason, OpenAI's choices[].finish_reason (read before
the include_usage chunk with empty choices), Ollama's done:true or its
terminal tool_calls chunk - and raises the same unexpected-end error
when the stream stops without one. Tool-call-only turns and length
finishes still complete. The OpenAI and Gemini fakes in test_providers
and test_tool_calling now carry the finish marker every real server
sends.

Co-Authored-By: Claude <[email protected]>
Token counting replaced every image's bytes with a placeholder, so a
1.2 MB image was budgeted at 48 tokens against roughly 1,600 real ones.
trim_history is the chat's context gate, so a few images on a small
local window silently overflowed it.

graphlink_memory.message_tokens now prices each image_bytes part with
estimate_image_tokens, which reads the image header for its dimensions
and applies Anthropic's documented vision formula (with a conservative
fallback when the header is unreadable), and keeps the payload out of
the text count. trim_history's returned context_tokens is now exactly
the sum over the messages it returns; it used to keep counting a popped
leading assistant turn and could count the newest message twice.

Two consequences of dropping images sooner are handled here: the
summary cache keys an image by digest instead of by the repr of its
bytes (11 MB per image), and the context meter adds the same per-image
figure so it shows what the budget spends.

Co-Authored-By: Claude <[email protected]>
last_chat_node_id decides where the next Send continues, but Commands
only snapshotted nodes, edges, assets and pins:

- Undoing a Send left the pointer on the removed node, so the next Send
  became a detached root with no parent edge.
- Mark Final Deliverable changed nothing a Command could see, was dropped
  as a no-op, and Ctrl+Z undid the previous action instead.
- removeNodes left both pointers naming deleted nodes.
- The pointer was never saved, so the first Send after reopening a chat
  also started a new root.

Commands now capture both document-level chat pointers before and after
every mutator; undo restores them and redo re-applies the state undo
left. remove_nodes moves the continuation point to the nearest surviving
chat ancestor and clears a removed Final Deliverable mark, as
delete_chat_node already did. The pointer is saved and restored like
final_deliverable_node_id, and older saves load unchanged. Synthesizing
branches now moves the pointer inside its own recorded command.

Co-Authored-By: Claude <[email protected]>
- Send: if the user deleted their message while the reply was
  generating, the reply's command raised SceneError and the user saw
  "AI response failed" for a request that succeeded. It is now a silent
  no-op, as regenerate, conversation and image replies already were.
  The same guard covers artifact replies and gitlink runs, and a gitlink
  apply whose node was deleted no longer reports "apply failed" for
  files it had already written.
- Provenance: the reply badge and the token counter's cost were taken
  from the task's default model even when the node was pinned to
  another one, and _on_usage re-read the dispatcher when usage arrived,
  so a provider switch mid-stream re-priced the reply. Both now use the
  request-time pair, resolved through the same pin rungs the dispatch
  uses.
- A reply moves the continuation point inside its own recorded command,
  so redo puts it back on the reply.

send_message's completion callback moves to module level beside
regenerate's, keeping register_chat_intents under its 300-line cap.

Co-Authored-By: Claude <[email protected]>
Dragging a container or frame with members built the members' synthetic
position changes with a hardcoded dragging: true. The middleware also
runs on React Flow's drag-stop change, so the release batch ended with
member changes still flagged as dragging, and onNodesChange re-opened
the gesture it had just closed. draggingRef stayed set, and the
scene-sync effect skipped every later publish: new nodes never
appeared, streamed replies never updated and deleted nodes stayed on
screen until some non-group node was dragged.

Member changes now carry the drag flag of the change that produced
them, and the parameter is required so no caller can drop it. Tests
drive a group drag through the real middleware and onNodesChange, and
through the canvas, where a scene update after release is applied.

Co-Authored-By: Claude <[email protected]>
The HTML node's source textarea and preview lived only in component
state, with no way to reach the backend. Off-viewport culling unmounts
the card, so panning away discarded both, and because the stored source
never changed, edits were also lost on save and reload.

A new scene/setHtmlContent intent stores the source as an undoable
command, like setNoteContent. The card commits on Render, on blur and
on Ctrl+S, only when the source changed. It ignores the echo of its own
commit so the cursor never jumps, but shows a change it did not make,
such as an undo, when there is no uncommitted typing.

Ctrl+S is exempt from the while-typing shortcut gate, so it saved the
session without the draft in an open note or HTML editor. Both editors
now push their draft ahead of the save. The new intent is classified as
undoable in the undo gate and labelled "Edit HTML".

Co-Authored-By: Claude <[email protected]>
Both dependency audits now fail every PR, main's lockfiles included.

pip-audit: brotli 1.2.0, certifi 2026.6.17, pillow 12.3.0, pypdf 6.16.1
and urllib3 2.7.0 have published advisories. Regenerated with the
lockfile's own command, upgrading only those five (pip-compile
--generate-hashes --upgrade-package ...): brotli 1.2.1, certifi
2026.10.23, pillow 13.0.0, pypdf 6.21.2, urllib3 2.8.0. Pillow is a
major bump; the full suite passes on it unchanged.

npm audit (high and above): brace-expansion, fast-uri, source-map-js,
undici, vite and zod. All six are fixed within their current major
ranges by npm audit fix, so only package-lock.json changes; vite stays
on 6.x. The remaining low and moderate advisories need breaking
upgrades and are below the gate.

pip-audit: no known vulnerabilities. npm audit: no high or critical.
Full backend suite 3,493 passed (coverage 86.6%); npm run check passed,
2,250 tests, bundle sizes unchanged.

Co-Authored-By: Claude <[email protected]>
npm audit fix pruned two bundled dependencies of the optional
@tailwindcss/oxide-wasm32-wasi package (@emnapi/core and @emnapi/runtime
1.11.1) from package-lock.json. Windows never installs that wasm
fallback, so nothing failed locally, but npm ci on Linux rejects the
lockfile as out of sync and both the frontend and E2E jobs stopped at
their install step.

The package itself is unchanged at 4.3.3, so the two entries are
restored exactly as main's lockfile has them. npm ci now accepts the
lockfile, npm audit still reports no high or critical advisories, and
npm run check passes on a clean npm ci install.

Co-Authored-By: Claude <[email protected]>
@dovvnloading
dovvnloading merged commit 57806b4 into main Oct 30, 2026
5 checks passed
@dovvnloading
dovvnloading deleted the fix/data-loss-and-wrong-output branch October 30, 2026 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant