import_vcf built `text = data.get("vcf") or data.get("text") or ""`, so a
non-string JSON value (a number, list, etc.) stayed in place and the following
`text.strip()` raised AttributeError, returning HTTP 500. Coerce vcf/text/csv
with str() so non-string input degrades to the existing structured "no data"
response, matching the file's convention elsewhere.
Adds tests/test_contacts_import_nonstring.py covering non-string vcf, non-string
csv, and an empty body.
get_status() called get_avg_duration() unconditionally, and that helper globs
and JSON-parses every file under the research data dir. The SSE status stream
polls get_status() roughly once a second, so with a few saved reports each poll
re-read and re-parsed all of them, including for sessions that are not active
(the disk branch never even used the value).
Compute avg_duration only for active sessions and memoize it on the task entry,
so a long stream computes it once instead of on every poll. Behaviour is
unchanged: active streams still report avg_duration.
Adds tests/test_research_status_avg_duration.py: an inactive session does no
avg scan, and an active session computes it once across many polls.
When a lane reset fails to rewrite the recreated collection, the recovery path
re-adds the preserved rows. It read the embeddings with
`preserved.get("embeddings") or []` and gated the loop with
`if ids and docs and old_embeddings:`. chromadb returns embeddings as a numpy
ndarray, whose truth value is ambiguous, so both expressions raise ValueError
inside the except block — the restore is abandoned and every preserved row is
lost (the collection was already deleted), exactly when the code is trying to
avoid data loss.
Use an explicit `is None` check and `len(...)`, and convert ndarray batches to
lists before re-adding.
Adds tests/test_embedding_lane_ndarray_restore.py (preserved embeddings come
back as np.ndarray); existing test_embedding_lanes.py still passes.
extract_urls() stripped any trailing ')' unconditionally via
`re.sub(r'[.,;:!?\)]+$', '', url)`. That corrupts URLs that legitimately
end in a parenthesis — most commonly Wikipedia disambiguation links like
https://en.wikipedia.org/wiki/Python_(programming_language), which became
...Python_(programming_language and then 404 when fetched by the web/research
tools.
Strip trailing sentence punctuation as before, but only drop a ')' when it is
unbalanced (more ')' than '('), so a prose-glued "(see https://example.com)"
still loses its closing paren while balanced URLs keep theirs.
Added tests/test_extract_urls.py covering balanced, unbalanced, nested, and
trailing-punctuation cases.
fork_session passed each source message's metadata dict by reference into the
new session. add_message() -> _persist_message() stamps _db_id (and timestamp)
onto that dict in place, so persisting the fork overwrote the SOURCE messages'
_db_id with the forked rows' ids — silently breaking edit/delete-by-id on the
original conversation.
Copy the metadata dict per message so the fork and source no longer alias.
Adds tests/test_fork_session_metadata.py asserting the source session's
message metadata is unchanged after a fork.
PATCH /api/tokens/{id} unconditionally recomputed scopes from
payload.get("scopes"). On a rename — body {"name": "..."} with no "scopes"
key — that is None, so _normalize_scopes(None) returned the default ["chat"]
and the handler overwrote token.scopes, silently dropping every scope the
token had been granted (e.g. email:read, calendar:write).
Only write scopes when the request actually includes them, and return the
token's real stored scopes in the response (matching the GET /tokens display
shape) instead of the recomputed default.
tests/test_api_token_routes.py: add rename-preserves-scopes,
explicit-scopes-applied, and missing-token-404 cases for the PATCH handler.
PresetManager.save() used a plain open("w") + json.dump, which truncates
presets.json before writing the new content. A crash, power loss, or
serialization error mid-write leaves the file truncated/empty and every
saved preset is lost.
Route the write through core.atomic_io.atomic_write_json (tmp file +
os.replace), matching how the rest of the codebase persists JSON state.
The helper is imported lazily so this module stays free of the heavy core
package import graph at module load time.
Adds tests/test_preset_atomic_save.py covering the source contract, a
failed-write leaving the existing file intact, and a round trip.
* fix(caldav): don't prune the whole window when no objects could be parsed
The post-sync prune deletes local origin=="caldav" rows in the window whose UID
the server didn't just return. With an empty seen_uids it falls back to
`uid.isnot(None)` — a match-all delete. That's right when the calendar is
genuinely empty, but when the server returns objects and every one fails to
parse (malformed iCal / an icalendar error), seen_uids is empty only because
nothing could be read, so the match-all branch silently deletes every local
event in the 90-day-back/365-day-forward window.
Track whether any object failed to parse and gate the prune with a small pure
helper `_should_prune_window(seen_uids, parse_failed)`: prune when something was
read, or when the calendar is genuinely empty (no objects, no parse errors), but
never when objects came back unreadable.
Adds tests/test_caldav_prune_parse_failure.py for the three cases.
* fix(caldav): skip the prune on any parse failure, not just total
Review follow-up (#3454): _should_prune_window returned True whenever seen_uids
was non-empty, so a partial parse failure (say 48 of 50 objects parse) still
pruned the 2 unreadable-but-still-upstream events, because their UIDs were absent
from seen_uids. Any parse failure makes seen_uids an incomplete view of the
server, so pruning against it is unsafe whether the failure is total or partial.
Skip the prune on any parse failure (return not parse_failed); only prune on a
clean read (a genuinely empty window is still safe to prune). Tradeoff: one
permanently-unparseable event pauses deletion mirroring until it is fixed, which
is the safe direction (false-keep beats false-delete).
Replace the now-incorrect "partial failure still prunes" assertion with a
partial-failure regression: one object parses, one fails, so the prune is
skipped and the unparsed event's local copy is not deleted.
---------
Co-authored-by: Kenny Van de Maele <kenny@kvandemaele.be>
* fix(memory): only delete memories the model explicitly drops in tidy
The AI memory-tidy path computed deletions as the complement of the model's
`keep` list (`if mid not in keep_ids: continue`). When the model returned a
valid response that simply omitted some existing ids — a common LLM lapse — every
omitted memory was silently deleted, even though it was neither a duplicate nor
listed in `drop`.
Honor the explicit `drop` set instead: delete only ids the model dropped (minus
any it saw only truncated), and preserve everything else, still applying cleaned
text/category from `keep`.
Adds tests/test_consolidate_memory_explicit_drops.py: a memory the model omits
from both keep and drop survives; an explicitly dropped one is removed.
* refactor(memory): remove now-dead keep_ids from tidy
After deletion switched to drop_ids and text/category rewrites to cleaned_by_id,
keep_ids was written but never read. Remove the init, the .add(mid) in the keep
loop, and the truncated .update() (its truncated-protection is already covered by
`drop_ids -= truncated_ids`). Pure deletion, no behavior change; tests stay green.
Addresses review feedback on #3455.
---------
Co-authored-by: Kenny Van de Maele <kenny@kvandemaele.be>
maybe_extract_skill() sliced the LLM response from the first '{' to the
last '}'. When a model emits a stray brace in prose before the real
object (e.g. "uses {placeholder} then {...}"), the slice starts at the
prose brace, json.loads fails, and a valid skill is silently dropped.
Factor parsing into _extract_json_object(), which tries the whole
(de-fenced) string first and then each '{' start position, returning the
first candidate that parses to a JSON object.
Adds tests/test_skill_extractor_json.py.
Mazen Tamer Salah