Repository navigation
feat(memory): add memory consolidation to synthesize fragmented facts - #3996
Conversation
When a fact category accumulates many individual entries, the LLM reviews them during the normal memory-update call (same invocation, no extra API cost) and decides whether groups of related facts can be synthesized into a single richer fact. This completes the memory lifecycle: extraction → guaranteed injection → staleness review → consolidation. - Select fragmented categories by min-facts threshold, surface the most fragmented groups first; prompt-layer caps aligned with apply-layer guardrails so the LLM never sees groups it cannot act on - Cap consolidated confidence at source maximum to prevent inflation; reject results below fact_confidence_threshold - Double-consume protection prevents a fact from being merged into multiple consolidation targets - Feature-gated at both prompt and apply time with per-cycle safety caps - Add 26 tests covering candidate selection, normalization, apply guardrails, and prompt integration
willem-bd
left a comment
There was a problem hiding this comment.
Reviewed the memory-consolidation changes. Apply-time guardrails (source-ID existence, no-overlap, size cap, confidence cap, threshold floor) look solid and the brace-escaping in both prompts is correct. Six findings below, most-severe first. The two actionable items are #1 (dedupe source IDs) and #2 (consolidation-vs-trim ordering) — both real data-integrity gaps.
| source_ids = entry.get("sourceIds") | ||
| if not isinstance(source_ids, list) or not source_ids: | ||
| continue | ||
| clean_ids = [sid for sid in source_ids if isinstance(sid, str) and sid] |
There was a problem hiding this comment.
Duplicate sourceIds bypass the "≥2 distinct sources" guardrail.
clean_ids filters out non-string/empty IDs but doesn't deduplicate. An LLM response like {"sourceIds": ["f1", "f1"], ...} passes the len(clean_ids) < 2 check here and the 2 <= len(source_ids) <= max_sources check at apply time. At apply, ids_consumed.update(["f1","f1"]) collapses to {"f1"}, so exactly one fact is removed and one consolidated fact is written — effectively rewriting a single fact's content/category through the consolidation path, which the normal ingestion path never permits (it dedups by content and never mutates existing facts).
Suggested fix — dedupe and reject when fewer than 2 unique IDs remain:
clean_ids = list(dict.fromkeys(sid for sid in source_ids if isinstance(sid, str) and sid))|
|
||
| if ids_consumed: | ||
| current_memory["facts"] = [f for f in current_memory.get("facts", []) if f.get("id") not in ids_consumed] | ||
| current_memory["facts"].extend(new_consolidated) |
There was a problem hiding this comment.
The max_facts trim at the end of _apply_updates can drop a freshly-consolidated fact after its sources were deleted here — unrecoverable loss.
Consolidation removes the N source facts (line 1007) and appends the consolidated fact (line 1008) before new facts are added and before the final max_facts trim. The consolidated fact's confidence is capped at max_source_conf (the max of its sources); if new facts arrive at higher confidence and push the count over max_facts, the stable confidence-descending sort in the trim can drop the consolidated fact — but its N sources are already gone. Net: all N originals and their synthesized replacement vanish, with no log line.
Consider running consolidation after the trim, or excluding source == "consolidation" facts from the trim for the cycle they were created.
| conf = float(raw_conf) if isinstance(raw_conf, (int, float)) and not isinstance(raw_conf, bool) else 0.0 | ||
| content = str(fact.get("content", "")) | ||
| lines.append(f'- [{fid} | {conf:.2f}] "{content}"') | ||
| parts.append(f'<consolidation_candidates category="{cat}" count="{len(group)}">\n' + "\n".join(lines) + "\n</consolidation_candidates>") |
There was a problem hiding this comment.
count="{len(group)}" advertises the full category size, but only group[:max_sources] facts are listed above.
When a category holds more than consolidation_max_sources facts (e.g. 20 facts, max_sources=8), the LLM is told count=20 while seeing only 8 IDs — contradicting this function's own docstring ("the LLM is never shown groups it cannot act on"). No data corruption (apply guardrails reject hallucinated IDs), but the LLM may waste output on IDs it can't see or attempt consolidations that get silently dropped.
Fix: use min(len(group), max_sources) for count, or add a note like shown 8 of 20.
| if isinstance(cat, str) and cat.strip(): | ||
| by_category.setdefault(cat.strip(), []).append(fact) | ||
| threshold = config.consolidation_min_facts | ||
| return {cat: group for cat, group in by_category.items() if len(group) >= threshold} |
There was a problem hiding this comment.
No protected-category exemption, unlike the sibling staleness feature.
_select_stale_candidates skips categories in staleness_protected_categories (default ["correction"] — "explicit user feedback [that] should not be auto-pruned"), but this selector groups every category unconditionally. If a protected category ever reaches consolidation_min_facts, consolidation actively surfaces it for merging — the exact category the system is designed to preserve.
Low likelihood at default min_facts=8 (corrections are rare), but the contract should be honored in both features. A one-line fix: skip categories in config.staleness_protected_categories (or add a parallel consolidation_protected_categories).
| "sourceIds": clean_ids, | ||
| "consolidated": { | ||
| "content": content.strip(), | ||
| "category": consolidated.get("category", "context") if isinstance(consolidated.get("category"), str) else "context", |
There was a problem hiding this comment.
Consolidated category isn't stripped/emptiness-checked, unlike the newFacts path (_normalize_memory_update_fact).
A value like " " or " preference " is persisted verbatim, while the rest of the system strips and defaults empty categories. A padded stored category can later miss category-based matching (e.g. guaranteed-injection slots), diverging from how every other fact is normalized.
Strip + default empty to match _normalize_memory_update_fact.
| "confidence": fact_confidence, | ||
| "createdAt": now, | ||
| "source": "consolidation", | ||
| "consolidatedFrom": list(source_ids), |
There was a problem hiding this comment.
sourceError metadata is not propagated into the consolidated fact.
This new_fact dict copies content/category/confidence/createdAt/source/consolidatedFrom, but facts may carry sourceError (the "what went wrong" guidance attached to corrections). Consolidating such facts loses that metadata silently — against the consolidation prompt's own "preserve ALL key details" rule.
Propagate sourceError when any source fact carries it.
Six fixes based on maintainer review of bytedance#3996: 1. Deduplicate sourceIds in normalization — ["f1","f1"] previously bypassed the ≥2-distinct-sources check; dict.fromkeys collapses it to ["f1"] which is correctly rejected. 2. Run consolidation after max_facts trim — previously, sources were deleted then the merged fact could be evicted by the trim, leaving no record of either. Moving consolidation last ensures source facts exist in the post-trim index before removal. 3. Fix count= attribute in consolidation prompt — advertised the full category size but listed only max_sources IDs; now uses min(len(group), max_sources) to match what the LLM can act on. 4. Exempt staleness_protected_categories from consolidation candidates — mirrors the existing staleness-review contract so correction facts are never surfaced for merging. 5. Strip and default category in consolidation normalization — " " or " preference " are now normalised, matching _normalize_memory_update_fact. 6. Propagate sourceError from source facts into consolidated fact — correction context is no longer silently lost on merge.
|
Thank you for the thorough and detailed review — the findings are all spot-on! All six issues have been addressed in the latest commit:
Regression tests for all six cases have been added to Thanks again for the careful read! |
willem-bd
left a comment
There was a problem hiding this comment.
Thanks @sontianye. Following up on the earlier review — the duplicate-IDs, post-trim ordering, count attribute, category-strip, and sourceError points are all addressed in this head (with regression tests). Two net-new items remain: the protected-category guardrail is enforced at selection time but not at apply time (defense-in-depth gap vs the staleness pass), and two apply-time guardrails lack direct tests. Details inline.
|
|
||
| # Guardrail: all source IDs must exist in the post-trim index | ||
| # and not already consumed by an earlier merge this cycle. | ||
| if any(sid in ids_consumed or sid not in fact_index for sid in source_ids): |
There was a problem hiding this comment.
[P1] Protected categories can still be consolidated at apply time (defense-in-depth gap vs staleness)
- Location:
updater.py:1019(consolidation apply guardrail loop); contrast staleness apply atupdater.py:909-917. - Problem: Following up on the earlier protected-category point —
_select_consolidation_candidatesnow excludesstaleness_protected_categoriesat prompt time, but the apply path does not re-check. The guardrails here only verify source existence, non-consumption, size, content, confidence, and threshold. The prompt still embeds the fullcurrent_memorydump ({current_memory}), so the LLM sees correction facts even though they aren't surfaced as candidates. An LLM slip that proposes a correction fact as a consolidation source is accepted here — silently deleting/rewriting an explicit user correction. - Evidence: The staleness pass closes exactly this hole by intersecting removals with the candidate set at apply time —
candidate_ids = {f["id"] for f in _select_stale_candidates(current_memory, config)}(updater.py:909-917) — so protected/non-aged facts can never be deleted "regardless of model behavior." The consolidation loop has no equivalent.test_protected_category_not_selectedonly asserts candidate selection, not apply, so the gap is untested. - Suggested fix: mirror staleness — build
allowed_idsfrom_select_consolidation_candidates(current_memory, config)at apply time and reject any decision whosesourceIdsaren't all in it; or at minimum reject sources whosefact_index[sid]["category"]is instaleness_protected_categories. - Test: feed
_apply_updatesafactsToConsolidateentry whosesourceIdsinclude a correction fact and assert it is not merged (and the correction survives).
There was a problem hiding this comment.
Thanks again for the precise analysis!
P1 — apply-time protected-category guardrail:
Mirrored the staleness-pass pattern exactly. is now built from _select_consolidation_candidates(current_memory, config) at the top of the consolidation apply block, and the inner guard checks sid not in allowed_source_ids alongside the existing checks. A protected or below-threshold fact proposed by the LLM is rejected regardless of model behaviour — same defense-in-depth guarantee the staleness pass provides.
Test added: test_protected_category_rejected_at_apply_time — feeds factsToConsolidate targeting correction facts into _apply_updates with the category below the selection threshold and asserts all correction facts survive untouched.
| # Falls back to max_source_conf when the field is absent or malformed. | ||
| raw_llm_conf = consolidated.get("confidence") | ||
| if isinstance(raw_llm_conf, (int, float)) and not isinstance(raw_llm_conf, bool) and math.isfinite(float(raw_llm_conf)): | ||
| fact_confidence = min(float(raw_llm_conf), max_source_conf) |
There was a problem hiding this comment.
[P2] Two apply-time guardrails lack direct tests
- Location:
updater.py:1037(confidence cap) and theif config.consolidation_enabled:apply gate (~line 996). - Problem: Two guardrails introduced by this PR have no test coverage:
- Confidence cap —
fact_confidence = min(float(raw_llm_conf), max_source_conf)prevents consolidation from inflating confidence. No test asserts that an LLM returning confidence1.0over sources maxing at0.8is capped to0.8(nor that a sub-threshold capped result is rejected). - Apply-time
consolidation_enabled=False— the real safety net when the flag is toggled mid-flight. There's a prompt-time "omitted when disabled" test, but no test that feedsfactsToConsolidateinto_apply_updateswith the flag off and asserts nothing is merged.
- Confidence cap —
- Evidence: scanned
test_memory_consolidation.py— apply tests cover max_groups, nonexistent IDs, over-max-sources, double-consume, post-trim rejection, and sourceError; none exercise the confidence-cap value or the apply-time disabled gate. - Suggested fix: add a test for the confidence cap (LLM conf > source max -> capped; capped value < threshold -> rejected) and one for apply-time disabled (
factsToConsolidatepresent,consolidation_enabled=False-> no merge).
There was a problem hiding this comment.
P2 — tests for confidence cap and apply-time disabled gate:
Both tests are now in TestReviewerFindings:
-
test_confidence_cap_and_threshold_gate— asserts that an LLM returning confidence1.0over sources maxing at0.75is capped to0.75; also asserts that when the capped value (0.65) falls belowfact_confidence_thresholdthe merge is rejected and both source facts survive. -
test_apply_gate_consolidation_disabled— passesfactsToConsolidateinto_apply_updateswithconsolidation_enabled=Falseand asserts nothing is merged (both source facts survive untouched).
Total tests: 35, all passing.
P1: mirror the staleness-pass defense-in-depth pattern — build allowed_source_ids from _select_consolidation_candidates at apply time so a protected-category or below-threshold fact proposed by the LLM is rejected regardless of model behavior. P2a: test that LLM-returned confidence is capped at max source confidence and that a capped result below fact_confidence_threshold is rejected. P2b: test that factsToConsolidate with consolidation_enabled=False is a no-op at apply time (35 tests, all pass).
willem-bd
left a comment
There was a problem hiding this comment.
Thanks @sontianye. The apply-time protected-category guardrail and the two missing tests from the prior review are addressed in af7d44c — the allowed_source_ids intersection mirrors staleness, and the confidence-cap / disabled-gate tests cover both guardrails. Looks good. A few minor items remain; posting them as follow-up nits.
|
|
||
| # ── Memory consolidation ──────────────────────────────────────────── | ||
| consolidation_enabled: bool = Field( | ||
| default=True, |
There was a problem hiding this comment.
[P2] consolidation_enabled defaults to true for a lossy, irreversible mutation
- Location:
memory_config.py:137-145 - Problem: Consolidation deletes source facts and replaces them with an LLM-synthesized fact.
consolidatedFrompreserves the source IDs but not their original content, so a bad merge is real data loss with no in-memory rollback (the atomic temp-file+rename gives a consistent single write, not history). Shipping this ON by default for every deployment — on arisk:highPR — means the first bad consolidation a user hits is unrecoverable from memory alone. - Suggested fix: consider defaulting
false(opt-in) for one release, or document the audit/rollback story (e.g. memory-file backups) before flipping the default. The flag itself is fine; this is only about the default. - Test: n/a (config default).
There was a problem hiding this comment.
Good call — agreed. Consolidation is a lossy, irreversible mutation: source content is permanently replaced and only the source IDs are preserved in consolidatedFrom. Shipping that on by default is the wrong risk posture for a new feature.
Changed consolidation_enabled default to false in memory_config.py, and updated config.example.yaml with a note explaining the trade-off so operators know exactly what they are opting into when they flip it.
| for fact in group[:max_sources]: | ||
| fid = fact.get("id", "?") | ||
| raw_conf = fact.get("confidence", 0.0) | ||
| conf = float(raw_conf) if isinstance(raw_conf, (int, float)) and not isinstance(raw_conf, bool) else 0.0 |
There was a problem hiding this comment.
[P2] Confidence coercion defaults diverge: 0.5 at apply vs 0.0 in the prompt formatter
- Location:
updater.py:577-578(_build_consolidation_section) vs_coerce_source_confidence(~updater.py:102) - Problem:
_coerce_source_confidence(used at apply time for the confidence cap) defaults a missing/invalid source confidence to0.5, but this prompt formatter defaults it to0.0. A fact stored withconfidence: nullis therefore shown to the LLM as0.00in the consolidation section yet treated as0.5when capping the merged fact's confidence — same concept, two defaults, so a fact can read low-value in the prompt while still lifting the capped result. - Suggested fix: reuse
_coerce_source_confidence(fact)here instead of the inline coercion, so display and apply agree. - Test: assert a null-confidence source renders with the same value used in the cap.
There was a problem hiding this comment.
Correct — the divergence was real. _build_consolidation_section used an inline float(raw_conf) if ... else 0.0 that defaulted missing/invalid confidence to 0.0, while _coerce_source_confidence (used at apply time) defaults to 0.5. A null-confidence fact would show 0.00 to the LLM while the cap treated it as 0.5.
Fixed by replacing the inline coercion with a direct call to _coerce_source_confidence(fact) — same function, same default, prompt and apply now agree. A regression test (test_null_confidence_renders_consistently_with_cap) verifies both sides use 0.50.
| "content": content.strip(), | ||
| "category": consolidated.get("category", "context"), | ||
| "confidence": fact_confidence, | ||
| "createdAt": now, |
There was a problem hiding this comment.
[P2] Consolidated fact uses now, resetting the staleness clock
- Location:
updater.py:1061 - Problem: The consolidated fact is stamped
createdAt: now, so even when its sources were aged (stale candidates), the synthesized fact won't be eligible for staleness review for anotherstaleness_age_days. Consolidation can effectively "launder" aged info into a fresh fact that evades the staleness pass. - Suggested fix: likely acceptable since the LLM just reviewed the sources during consolidation — but make it a conscious choice. If aged info should stay reviewable, carry forward the newest source's
createdAt(or add aconsolidatedAtalongsidecreatedAt). - Test: assert the consolidated
createdAtpolicy is intentional (eithernowor newest-source) under a stale-source scenario.
There was a problem hiding this comment.
Made it a conscious choice: the consolidated fact now carries the newest source's createdAt rather than now.
The semantic: the fact's age should reflect when the underlying information was last recorded, not when synthesis happened. This means aged information that happens to get consolidated stays eligible for the next staleness pass — the clock is never reset. consolidatedAt: now is added alongside as an explicit audit field, so the merge timestamp is preserved without affecting staleness arithmetic.
A regression test (test_consolidated_created_at_tracks_newest_source) asserts createdAt == max(source.createdAt) and that consolidatedAt > max(source.createdAt).
1. Default consolidation_enabled=False — consolidation is lossy (source content is permanently replaced, only consolidatedFrom IDs preserved); new lossy features default to off. config.example.yaml updated to match. 2. Unify confidence coercion between prompt and apply — _build_consolidation_section now calls _coerce_source_confidence(fact) instead of an inline 0.0-default coercion, so a null-confidence fact renders with 0.50 in the LLM prompt and is capped at 0.50 at apply time (same value, same function). 3. Preserve staleness clock on merge — consolidated fact now carries the newest source's createdAt (not now) so aged information does not gain a fresh staleness-review window just by being consolidated; consolidatedAt is added as an explicit audit field. Three regression tests added (default=false, null-confidence consistency, createdAt policy); all guardrail tests now set consolidation_enabled=True explicitly so they test the guardrail, not the feature flag. 38 tests pass.
willem-bd
left a comment
There was a problem hiding this comment.
Thanks for this — really nicely structured. The consolidation pass faithfully mirrors the staleness-review pattern: candidates are recomputed at apply time and used as an allowlist, so hallucinated IDs / protected categories / over-merging / confidence inflation are all rejected regardless of model behavior. Default-off is the right call for a lossy mutation, the post-trim ordering correctly prevents the only real data-loss vector, and the test suite (38 tests) is thorough.
Posting a few inline comments below — mostly docs and minor hardening. The one non-doc item I'd address before merge is the createdAt handling.
Two notes that don't attach to a single line:
- PR description says
consolidation_enableddefaults totrue, but the code /config.example.yaml/test_consolidation_enabled_defaults_to_falseall default tofalse. The code is the safer choice; the description is stale and should be corrected so reviewers don't conclude this mutates memory on first deploy. - The core risk to sign off on is the acknowledged irreversibility:
consolidatedFromkeeps source IDs but not their text, so a bad synthesis permanently destroys detail. The default-off / opt-in framing handles this correctly — just flagging it as the gate for ever flipping the default.
| 3. Background thread invokes LLM to extract context updates and facts, using the stored `user_id` (not the contextvar, which is unavailable on timer threads) | ||
| 4. Applies updates atomically (temp file + rename) with cache invalidation, skipping duplicate fact content before append | ||
| 5. **Staleness pass** (same LLM invocation as step 3, no extra API call): when `staleness_review_enabled` is `true` and at least `staleness_min_candidates` aged facts exist, `_select_stale_candidates` selects facts older than `staleness_age_days` that are not in `staleness_protected_categories` (default: `correction`), surfaces them in the prompt, and the LLM judges each as KEEP or REMOVE. `_apply_updates` enforces the guardrail unconditionally at apply time: it intersects the LLM-returned removal set with `_select_stale_candidates` output before applying the per-cycle cap (`staleness_max_removals_per_cycle`), so protected and non-aged facts can never be deleted regardless of model behavior or the feature flag setting. | ||
| 5b. **Consolidation pass** (same LLM invocation as step 3, no extra API call): when `consolidation_enabled` is `true` and at least one category holds `consolidation_min_facts` or more facts, `_select_consolidation_candidates` identifies fragmented categories and surfaces at most `consolidation_max_groups_per_cycle` of them (largest first) in the prompt. The LLM decides which groups to merge and proposes a synthesised fact per group. `_apply_updates` enforces guardrails: source IDs must exist and must not overlap across groups, group size is capped at `consolidation_max_sources`, the merged fact's confidence cannot exceed the source maximum, and facts below `fact_confidence_threshold` are not written. |
There was a problem hiding this comment.
Doc gap (safety property). This step enumerates the apply-time guardrails but omits a key one: protected categories (staleness_protected_categories, default correction) are excluded from consolidation candidates — enforced both in _select_consolidation_candidates and the apply-time allowlist (allowed_source_ids). "Explicit user feedback is never silently merged away" is the central safety property here, so worth calling it out, mirroring how the staleness step documents its own protected-category exclusion.
There was a problem hiding this comment.
Good catch — the protected-category exclusion is the central safety property of this feature and deserved explicit documentation. Expanded the guardrails comment to say:
must be in
allowed_source_ids— the set built from_select_consolidation_candidates, which excludes categories instaleness_protected_categories(default:correction). This mirrors the staleness apply-time check and ensures explicit user feedback is never silently merged away regardless of model behaviour.
| # reflects the age of the underlying information, not when | ||
| # synthesis happened. consolidatedAt records the merge time | ||
| # for audit without resetting staleness eligibility. | ||
| source_created_at = max(fact_index[sid].get("createdAt") or now for sid in source_ids) |
There was a problem hiding this comment.
This string max() over createdAt is the one spot that doesn't get the "corrupted / manually-edited memory file" hardening that _coerce_source_confidence was added for. Two concerns:
- If a file stores a numeric
createdAt(e.g. a unix timestamp),max([str, int])raisesTypeErrorand aborts the whole memory update. - String comparison assumes a uniform
Zsuffix; a file mixingZand+00:00orders wrong lexicographically (same instant → theZone is picked as "newer"), skewing the merged fact's staleness clock.
The system already has _parse_fact_datetime for correct chronological comparison — routing through it (or a _coerce-style guard) would make this crash-proof and consistent with how staleness reasons about age.
There was a problem hiding this comment.
Both concerns are real and fixed. Replaced the string max() with a datetime-aware comparison via _parse_fact_datetime, mirroring exactly how staleness computes age:
_fallback_dt = _parse_fact_datetime(now) or datetime.now(UTC)
_source_dts = [_parse_fact_datetime(fact_index[sid].get("createdAt") or "") or _fallback_dt for sid in source_ids]
source_created_at = max(_source_dts).isoformat().removesuffix("+00:00") + "Z"A numeric createdAt is caught by _parse_fact_datetime's TypeError guard and falls back to _fallback_dt; mixed Z/+00:00 formats are now compared as aware datetime objects rather than as strings. The existing test_consolidated_created_at_tracks_newest_source still exercises this path.
| continue | ||
|
|
||
| source_confidences = [_coerce_source_confidence(fact_index[sid]) for sid in source_ids] | ||
| max_source_conf = min(max(source_confidences), 1.0) |
There was a problem hiding this comment.
_coerce_source_confidence already clamps to [0, 1], so the outer min(..., 1.0) can never bind — dead code that slightly mis-states intent (it reads as if the clamp were needed). Nit: either drop it or leave a note that it's purely defensive.
There was a problem hiding this comment.
Correct — dead code removed. _coerce_source_confidence already guarantees its return is in [0.0, 1.0] via the final max(0.0, min(val, 1.0)), so max(source_confidences) ≤ 1.0 by contract and the outer min(..., 1.0) could never bind. Changed to plain max(source_confidences) with a comment explaining the invariant.
| _norm_conf = 0.9 | ||
| else: | ||
| _f = float(_raw_conf) | ||
| _norm_conf = _f if math.isfinite(_f) else 0.9 |
There was a problem hiding this comment.
Two small nits on the consolidated-confidence path:
- Unlike
_normalize_memory_update_fact, this keeps out-of-range values (e.g.1.5) — only finiteness / non-bool is checked. Not exploitable today because the apply-time cap pins it tomax_source_conf <= 1.0, but if that cap is ever loosened the divergence becomes a foot-gun. - The null/bool/non-finite coercion now lives in four places (
_validate_confidence,_coerce_source_confidence, here, and the apply-time check at ~L1046)._coerce_source_confidencecould subsume the latter two.
There was a problem hiding this comment.
Both points addressed:
-
Out-of-range values — added
max(0.0, min(float(raw_llm_conf), 1.0))clamping before applying the source cap, so values like1.5are clipped to1.0first. Safe today via the cap, but now defensively correct if the cap is ever relaxed. -
Refactoring — kept the coercion inline rather than folding it into
_coerce_source_confidence. The two contexts have genuinely different fallback semantics: source facts default to0.5(uncertain but plausible, and the system can't improve on that); the LLM's returned confidence defaults tomax_source_conf(the natural conservative ceiling). A combined helper would need a caller-supplied default parameter, making it less readable than the two explicit call sites. The duplication is minimal (one conditional expression each).
| if isinstance(raw_llm_conf, (int, float)) and not isinstance(raw_llm_conf, bool) and math.isfinite(float(raw_llm_conf)): | ||
| fact_confidence = min(float(raw_llm_conf), max_source_conf) | ||
| else: | ||
| fact_confidence = max_source_conf |
There was a problem hiding this comment.
Test gap. This fallback branch (LLM omits / malforms confidence) isn't directly exercised — test_confidence_cap_and_threshold_gate only covers the present-confidence path. Since this sits on the loss-protection side, an explicit test (e.g. LLM returns no confidence field → result uses max_source_conf) would be worthwhile.
There was a problem hiding this comment.
Added test_confidence_fallback_to_max_source_when_llm_omits_field: sources at 0.85/0.75, LLM returns a consolidated dict with no confidence key, asserts merged[0]["confidence"] == pytest.approx(0.85) (the max_source_conf fallback).
| fid = fact.get("id", "?") | ||
| conf = _coerce_source_confidence(fact) | ||
| content = str(fact.get("content", "")) | ||
| lines.append(f'- [{fid} | {conf:.2f}] "{content}"') |
There was a problem hiding this comment.
Raw fact content is injected inside "...". Content containing a literal " or a closing </consolidation_candidates> tag could confuse the model. This matches the existing staleness section (same pattern), so it's not a regression — but given the recent prompt-injection hardening focus (b36d719), worth a deliberate decision on whether memory-fact content warrants the same neutralization.
There was a problem hiding this comment.
Noted — and deliberately left as-is for this PR. The pattern (raw fact content inside "..." in prompt XML) is identical to the existing staleness section, so this is not a regression. A proper fix would require escaping all fact content used across both sections, which is a meaningful separate change. Happy to follow up in a dedicated PR once this one lands.
1. createdAt max via _parse_fact_datetime — replaces string max() which
crashes on non-string createdAt (numeric unix timestamps) and sorts
Z/+00:00 mixed formats incorrectly. Mirrors how staleness computes age.
2. Remove dead min(..., 1.0) — _coerce_source_confidence already clamps
each source confidence to [0, 1], so max(source_confidences) ≤ 1.0
by contract; the outer min could never bind.
3. Clamp raw_llm_conf to [0, 1] before applying the source cap — out-of-
range values like 1.5 are safe today (pinned by the cap) but defensively
clamped first so the invariant holds even if the cap is ever loosened.
4. Doc: expand the apply-time guardrails comment to call out the protected-
category exclusion via allowed_source_ids — this is the central safety
property ("explicit user feedback is never silently merged away").
5. Test: add test_confidence_fallback_to_max_source_when_llm_omits_field
covering the else-branch (LLM omits confidence → uses max_source_conf).
6. Fix lint: reorder imports in test file (stdlib before third-party).
39 tests, all pass.
…y rebuilt 488ec17 -> 9097642. Migration-free (head stays 0003), no compose/Dockerfile/ frontend changes, zero conflicts, all carried patches intact. Notable: bytedance#3996 adds memory consolidation, which is LOSSY (source fact content is permanently replaced, only consolidatedFrom IDs survive). It defaults to false and we do not set the key, so we inherit false — verified against the new image, and the code gates it at both prompt-build and apply time. Keep it off. bytedance#4019 auto-promote is gated on tool_search.enabled (false here). bytedance#4012 fixes the redis stream bridge (we run in-memory). bytedance#4018 scrubs MYSQL_PWD/REDISCLI_AUTH from the skill env. Verified 3a/3b/3c incl. a live Aio bash-smoke, since bytedance#4009/bytedance#4017 touch the tool-output path the LGI pipeline depends on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
) * fix(security): html-escape fact content in memory prompt sections Raw memory fact content was injected verbatim into prompt XML — a fact containing a literal `"` could break the `"..."` delimiter, and a closing tag like `</consolidation_candidates>` could prematurely end the XML block, both potentially confusing the model. Apply `html.escape()` to `content` in `_build_staleness_section` and `_build_consolidation_section`, and to `cat` in the consolidation section's XML attribute. Tests added for both sections covering special characters, XML tag injection, and attribute injection. Follow-up to #3996 as noted by reviewer willem-bd. * fix(security): address reviewer follow-ups on html-escaping PR - Escape `cat` in _build_staleness_section for symmetry with the consolidation section (both sections now consistently html-escape all LLM-derived category values that appear in the prompt) - Add comment at current_memory=json.dumps() documenting the conscious accept: json.dumps leaves < > & unescaped; lower-risk than staleness/consolidation (read-only context, not delete/merge instructions); fix at fact-content insert time if revisited - Add test for category escaping in the staleness section * fix(security): reference tracking issue #4044 in conscious-accept comment * style: compress conscious-accept comment to two lines
…DeerMem; phase-2 host hooks Port upstream bytedance#3996 (memory consolidation) and bytedance#3993 (staleness KeyError fix) from origin/MemoryManager into the pluggable, self-contained DeerMem structure (backends/deermem/deermem/), adapted to the DI MemoryUpdater (config injected, not get_memory_config globals): - DeerMemConfig: add consolidation_enabled (opt-in, default false) / consolidation_min_facts / consolidation_max_groups_per_cycle / consolidation_max_sources - prompt.py: factsToConsolidate JSON field + {consolidation_section} placeholder + CONSOLIDATION_PROMPT constant - updater.py: _coerce_source_confidence / _select_consolidation_candidates / _build_consolidation_section module helpers (matching the existing _select_stale_candidates style); consolidation normalization in _normalize_memory_update_data; consolidation apply in _apply_updates (after max_facts trim, with apply-time guardrails mirroring staleness); staleness KeyError fix (f["id"] -> f.get("id") is not None) applied to both the staleness guardrail and the consolidation allowed_source_ids comprehension - config.example.yaml: consolidation section under memory.backend_config - tests/test_memory_consolidation.py: 40 DI-adapted tests (running, not skipped) incl. the staleness KeyError regression Also includes in-flight phase-2 host-integration work: storage_path semantics (any absolute/relative value = root dir) and host-default tracing_callback / should_keep_hidden_message hooks injected into backend_config by the factory. Co-Authored-By: Claude <noreply@anthropic.com>
Integrates the self-contained DeerMem restructure (pluggable backends + DI
MemoryUpdater) with the upstream MemoryManager line.
Memory-subsystem conflicts resolved in favor of MemoryManager-lyf ('keep my
version'): core/updater.py, core/prompt.py, tests/test_memory_consolidation.py
taken from lyf (restructured layout + ported consolidation bytedance#3996 and staleness
KeyError fix bytedance#3993); config.example.yaml memory section kept as lyf's
backend_config form (non-memory upstream yaml changes - MCP/boxlite - preserved,
duplicate top-level consolidation block removed); memory_config.py kept stripped
(consolidation lives on DeerMemConfig only).
Non-memory upstream commits (MCP routing hints, boxlite warm-pool, Helm chart,
sandbox env scrubbing, etc.) merge cleanly.
Verified: test_memory_consolidation (40) + test_deermem_self_contained +
test_memory_manager_pluggable + test_memory_prompt_injection = 84 passed.
Co-Authored-By: Claude <noreply@anthropic.com>
…bytedance#3996) * feat(memory): add memory consolidation to synthesize fragmented facts When a fact category accumulates many individual entries, the LLM reviews them during the normal memory-update call (same invocation, no extra API cost) and decides whether groups of related facts can be synthesized into a single richer fact. This completes the memory lifecycle: extraction → guaranteed injection → staleness review → consolidation. - Select fragmented categories by min-facts threshold, surface the most fragmented groups first; prompt-layer caps aligned with apply-layer guardrails so the LLM never sees groups it cannot act on - Cap consolidated confidence at source maximum to prevent inflation; reject results below fact_confidence_threshold - Double-consume protection prevents a fact from being merged into multiple consolidation targets - Feature-gated at both prompt and apply time with per-cycle safety caps - Add 26 tests covering candidate selection, normalization, apply guardrails, and prompt integration * fix(memory): address consolidation correctness issues from PR review Six fixes based on maintainer review of bytedance#3996: 1. Deduplicate sourceIds in normalization — ["f1","f1"] previously bypassed the ≥2-distinct-sources check; dict.fromkeys collapses it to ["f1"] which is correctly rejected. 2. Run consolidation after max_facts trim — previously, sources were deleted then the merged fact could be evicted by the trim, leaving no record of either. Moving consolidation last ensures source facts exist in the post-trim index before removal. 3. Fix count= attribute in consolidation prompt — advertised the full category size but listed only max_sources IDs; now uses min(len(group), max_sources) to match what the LLM can act on. 4. Exempt staleness_protected_categories from consolidation candidates — mirrors the existing staleness-review contract so correction facts are never surfaced for merging. 5. Strip and default category in consolidation normalization — " " or " preference " are now normalised, matching _normalize_memory_update_fact. 6. Propagate sourceError from source facts into consolidated fact — correction context is no longer silently lost on merge. * fix(memory): add apply-time guardrails and tests for consolidation P1: mirror the staleness-pass defense-in-depth pattern — build allowed_source_ids from _select_consolidation_candidates at apply time so a protected-category or below-threshold fact proposed by the LLM is rejected regardless of model behavior. P2a: test that LLM-returned confidence is capped at max source confidence and that a capped result below fact_confidence_threshold is rejected. P2b: test that factsToConsolidate with consolidation_enabled=False is a no-op at apply time (35 tests, all pass). * fix(memory): address three correctness issues from second review round 1. Default consolidation_enabled=False — consolidation is lossy (source content is permanently replaced, only consolidatedFrom IDs preserved); new lossy features default to off. config.example.yaml updated to match. 2. Unify confidence coercion between prompt and apply — _build_consolidation_section now calls _coerce_source_confidence(fact) instead of an inline 0.0-default coercion, so a null-confidence fact renders with 0.50 in the LLM prompt and is capped at 0.50 at apply time (same value, same function). 3. Preserve staleness clock on merge — consolidated fact now carries the newest source's createdAt (not now) so aged information does not gain a fresh staleness-review window just by being consolidated; consolidatedAt is added as an explicit audit field. Three regression tests added (default=false, null-confidence consistency, createdAt policy); all guardrail tests now set consolidation_enabled=True explicitly so they test the guardrail, not the feature flag. 38 tests pass. * fix(memory): harden createdAt comparison and confidence handling 1. createdAt max via _parse_fact_datetime — replaces string max() which crashes on non-string createdAt (numeric unix timestamps) and sorts Z/+00:00 mixed formats incorrectly. Mirrors how staleness computes age. 2. Remove dead min(..., 1.0) — _coerce_source_confidence already clamps each source confidence to [0, 1], so max(source_confidences) ≤ 1.0 by contract; the outer min could never bind. 3. Clamp raw_llm_conf to [0, 1] before applying the source cap — out-of- range values like 1.5 are safe today (pinned by the cap) but defensively clamped first so the invariant holds even if the cap is ever loosened. 4. Doc: expand the apply-time guardrails comment to call out the protected- category exclusion via allowed_source_ids — this is the central safety property ("explicit user feedback is never silently merged away"). 5. Test: add test_confidence_fallback_to_max_source_when_llm_omits_field covering the else-branch (LLM omits confidence → uses max_source_conf). 6. Fix lint: reorder imports in test file (stdlib before third-party). 39 tests, all pass. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
…m backend (#4122) * feat(memory): pluggable + self-contained memory system (MemoryManager plan phases 1 & 2) Phase 1 — Pluggable (steps 0-10): - ABC MemoryManager (9 methods) + singleton factory + drop-in backend discovery - DeerMem default backend with core/ (storage/queue/updater/prompt/message_processing) - NoopMemoryManager backend (proves pluggability) - All call sites (middleware/hook/prompt/gateway/client/app) routed through manager - hasattr capability probing for DeerMem-internal methods (no hard imports) - MemoryConfig gains manager_class field; shared vs DeerMem-private annotated Phase 2 — Self-contained DeerMem (steps 11-18): - backend_config passthrough + DeerMemConfig (all DeerMem-private fields moved off MemoryConfig) - DI: DeerMem owns storage/queue/updater/llm as instance attributes (no global singletons) - Storage independence: core/paths.py with own root (~/.deermem or ), factory auto-injects deer-flow's runtime_home() as absolute base_dir (zero-config) - LLM independence: core/llm.py via langchain init_chat_model (no create_chat_model) - Trace independence: optional tracing_callback replaces inject_langfuse_metadata/request_trace_context - Message processing independence: hide_from_ui default-skip + optional should_keep_hidden_message hook - Internal imports → relative (only deer_mem.py ABC import is host-relative) - Carrier (deer_mem.py adapter) / portable (deermem/ config+core) split - New tests: test_deermem_self_contained + test_memory_manager_pluggable; all memory tests migrated - Other-agent demo: samples/other_agent_demo/ + automated portability test - config.example.yaml memory section updated to phase-2 schema * feat(memory): port consolidation + staleness fix into self-contained DeerMem; phase-2 host hooks Port upstream #3996 (memory consolidation) and #3993 (staleness KeyError fix) from origin/MemoryManager into the pluggable, self-contained DeerMem structure (backends/deermem/deermem/), adapted to the DI MemoryUpdater (config injected, not get_memory_config globals): - DeerMemConfig: add consolidation_enabled (opt-in, default false) / consolidation_min_facts / consolidation_max_groups_per_cycle / consolidation_max_sources - prompt.py: factsToConsolidate JSON field + {consolidation_section} placeholder + CONSOLIDATION_PROMPT constant - updater.py: _coerce_source_confidence / _select_consolidation_candidates / _build_consolidation_section module helpers (matching the existing _select_stale_candidates style); consolidation normalization in _normalize_memory_update_data; consolidation apply in _apply_updates (after max_facts trim, with apply-time guardrails mirroring staleness); staleness KeyError fix (f["id"] -> f.get("id") is not None) applied to both the staleness guardrail and the consolidation allowed_source_ids comprehension - config.example.yaml: consolidation section under memory.backend_config - tests/test_memory_consolidation.py: 40 DI-adapted tests (running, not skipped) incl. the staleness KeyError regression Also includes in-flight phase-2 host-integration work: storage_path semantics (any absolute/relative value = root dir) and host-default tracing_callback / should_keep_hidden_message hooks injected into backend_config by the factory. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): add noop backend template and backends guide - backends/noop/: complete drop-in template (config.py with zero deer-flow imports, noop_manager.py with a 6-step new-backend walkthrough in its docstring, commented optional fact-CRUD capabilities). - backends/README.md: which files to touch when adding/swapping a backend, the 5-item backend contract, and common pitfalls. - manager.py: generalize backend examples in comments (drop mem0-specific references). Co-Authored-By: Claude <noreply@anthropic.com> * fix(frontend): guard formatTimeAgo against invalid timestamps Return a neutral placeholder when the input date is invalid (e.g. an empty lastUpdated from a backend with no memories) instead of throwing 'Invalid time value' from date-fns. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): wire tool-driven memory mode through the MemoryManager ABC tools.py (memory_search/add/update/delete) now calls get_memory_manager() instead of the removed host memory module, so tool mode (memory.mode: tool) works for any backend. DeerMem.search is implemented (case-insensitive substring match, ranked by confidence) as a stand-in for the planned semantic retrieval; noop.search returns [] (unchanged). Fact-CRUD tools use getattr+callable probing -- backends lacking those ops (noop) get a clear JSON error instead of crashing. Tests: test_memory_tools rewired to mock the manager (handler tests) + TestModeGating retained; test_memory_search now covers DeerMem.search; pluggable stubs test updated (search no longer a stub). Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (import sorting, type annotation quotes, E402 in skipped tests) * docs: restore explanatory comments in config.example.yaml memory section * fix(security): port html-escape memory facts fix (#4097) to vendored DeerMem prompt.py * fix(memory): address review + port dropped upstream memory fixes Review blockers (vendored DeerMem): - #4044 restore _escape_memory_for_prompt (current_memory blob in MEMORY_UPDATE_PROMPT) - prevents </current_memory> breakout - #4028 html.escape staleness-section cat/content in _build_staleness_section - #4119 add _escape_summary for injection-path summaries (Work/Personal/ Current Focus/Recent/Earlier/Background) - default-model silent no-op: factory injects host default chat model via a new host_llm slot (create_chat_model(name=None)); DeerMem prefers host_llm over build_llm(model). Zero-config extraction works out of the box again - MemoryConfigResponse: fix stale docstring (backend-agnostic shape; DeerMem knobs live under backend_config, not top-level - restoring flat would re-couple the API to DeerMem). Frontend audited: does not read /memory/config - _host_default_tracing_callback: restore langfuse assistant_id/environment - search: push category onto the ABC signature; DeerMem filters BEFORE the top_k slice (was filtered client-side after slicing -> starved results) - _do_update_memory_sync: split into wrapper+impl; bind trace_id into the request-trace ContextVar on the Timer/executor worker via a new trace_context_manager host hook (None trace_id left unbound - no fabrication) - client.py fact-CRUD now passes user_id (was writing to the global bucket while get_memory reads per-user) - _resolve_manager_class: fail-fast (raise ValueError) on an unresolved explicit manager_class instead of silently falling back to DeerMem (memory is persistent state - a wrong store is a silent data-integrity footgun) Upstream memory fixes dropped by the host->vendored rename conflict, re-ported to backends/deermem/deermem/core/ (+ deer_mem.py): - #4073 queue busy-timer-spin -> _reprocess_pending flag (core/queue.py) - #4074 null source.confidence in staleness -> _coerce_source_confidence (core/updater.py: _build_staleness_section + _apply_updates stale sort) - #4075 factsToRemove is optional (drop from _REQUIRED_MEMORY_UPDATE_TOP_LEVEL_KEYS) - #4076 null confidence in search ranking -> _coerce_source_confidence (deer_mem.py DeerMem.search) host_llm + trace_context_manager are host-injected via backend_config (factory in manager.py), keeping backends/deermem/ at exactly one `from deerflow` line (the ABC contract) - portability test preserved. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (F541 f-string without placeholders, E501 line too long) * fix(memory): restore hide_from_ui clarification preservation, expose mode Two memory-system fixes (F541/E501 lint was already fixed on this branch): - filter_messages_for_memory: restore default preservation of well-formed human_input_response clarification answers (v2 regression). The self-containment refactor made the bare function skip ALL hide_from_ui when no hook was passed, but upstream preserves well-formed clarification responses by default (test_hide_from_ui_human_input_response_is_preserved). Inline a host-agnostic _is_human_clarification_response mirror of read_human_input_response as the default keep-decision; the host-injected should_keep_hidden_message hook still overrides (production path unchanged). Portable package stays zero `from deerflow`. - /memory/config: expose `mode` (middleware|tool) in MemoryConfigResponse + the config/status endpoints + client.get_memory_config. mode is a host- shared, behavior-determining field missing from the response projection. Sync tests (mock .mode; e2e assert mode present). - Align manager_class field docstring with fail-fast behavior. Tests: filter/self-contained/portability (35) + memory-config (4) pass; ruff clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): resolve ruff format failures in memory module + tests `make lint` runs `ruff format --check` in addition to `ruff check`; 8 memory files had pending format changes -- 7 pre-existing (deer_mem, updater, tools, test_memory_queue/router/search/tools) + message_processing from the hide_from_ui fix. Apply `ruff format`: whitespace/wrapping only, no logic change. 109 memory tests pass; ruff check + format --check both clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - legacy field migration, fact_id contract, path/docs Address willem-bd's review on PR head bc8bf0d (risk:high, persistent state): - config: auto-migrate pre-abstraction top-level memory.* DeerMem fields (storage_path, max_facts, debounce_seconds, model_name, token_counting, staleness_*, consolidation_*) into backend_config on load + warn, so an upgrade does NOT silently revert customized settings (was: silent extra='ignore' drop). model_name -> backend_config.model.model. Unknown top-level keys warned. - factory: resolve a relative backend_config.storage_path against runtime_home() (base_dir-relative, CWD-independent) to preserve pre-abstraction semantics; paths.py stays portable (no runtime_home import). - tools: memory_add uses the fact_id returned directly by create_fact instead of re-deriving it via content-key matching (coupled the tool to the backend's content normalization; could misreport a storage cap). create_fact now returns (memory_data, fact_id); gateway/client/tool updated. Fix terse {"error":"content"} -> {"error":"empty content"}. - app.py: update stale token_counting=="char" warm-up comment to point at manager.warm (DeerMem.warm re-checks char and returns early). - router: comment explaining reload_memory silent fallback vs fact 501 asymmetry (read-only degrade vs write fail-loud). - CHANGELOG: document breaking changes (/memory/config + client.get_memory_config shape flat->backend_config; custom storage_class path moved + __init__ must accept config) and the legacy-field auto-migration. - tests: add regression test pinning the per-user memory path ({storage_path}/users/{safe_user_id}/memory.json == host make_safe_user_id) across the abstraction; update create_fact mocks for (memory_data, fact_id). Tests: 273 passed (memory suite); ruff check + format clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - storage_path, max_facts, tracing, parsing Six review findings (willem-bd), each verified against upstream: - storage_path semantics (file -> root dir): migration drops file-style (.json) legacy values with a warning; factory raises if storage_path resolves to an existing file (avoid silent NotADirectoryError write failure). CHANGELOG + config.example.yaml comment updated. - create_memory_fact enforces max_facts again (via _trim_facts_to_max) and returns (memory, None) when the cap evicts the new fact; memory_add tool reports "not stored", client raises ValueError, POST /memory/facts -> 409. - max_facts trim uses _coerce_source_confidence (was raw f.get("confidence", 0) -> TypeError on non-float imported/legacy confidence, swallowed as silent update failure). - memory-tracing assistant_id restored to "memory_agent" (was "lead-agent" copy-paste; matches upstream + DeerMem run_name). - _is_human_clarification_response cross-checked against read_human_input_response (drift guard test). - empty-string legacy values skipped silently in migration (narrow fix, not broad "if not value" which would skip explicit bool False). 8 new regression tests. make lint + 406 memory tests pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address internal review - storage fail-fast, build_llm degrade, config warn, noop template Addresses 4 findings from the PR #4122 internal supplemental review (parallel to willem-bd's review, no overlap): - create_storage fail-fast: a misspelled/unimportable storage_class now raises ValueError instead of silently falling back to FileMemoryStorage. Memory is persistent state, so a wrong store is a data-integrity footgun; mirrors the existing manager_class resolution policy. (storage.py) - noop template create_fact signature: the commented template used keyword-only `content` and returned a bare dict, while DeerMem's actual create_fact takes positional `content` and returns tuple[dict, str|None] (the memory_add tool passes content positionally; gateway/client/tools all tuple-unpack). A backend copied from the template would 500 on fact-CRUD. Template fixed; delete_fact/update_fact templates left (callers compatible). (noop_manager.py) - build_llm graceful degrade: wrap init_chat_model in try/except, degrade to None + WARNING on failure (mirroring _host_default_llm) so a misconfigured explicit model does not crash app startup -- non-LLM memory ops still work and an update raises at runtime with the error logged. (llm.py) - from_backend_config unknown-key warning: log a WARNING for unknown backend_config keys (mirrors the host layer's load_memory_config_from_dict) so a typo like `storage_pat` does not silently fall back to the default and write memory to an unintended location. (config.py) Tests: rewrote 3 create_storage fallback tests to expect ValueError; added 4 tests (build_llm zero-config/degrade, from_backend_config warn/silent). make lint green; full memory suite passes. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: lllyfff <2281215061@qq.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: lllyfff <122260771+lllyfff@users.noreply.github.com>
Consolidation (bytedance#3996) and per-fact expected_valid_days (bytedance#4143) were both authored by the same contributor but never connected: the consolidated new_fact carried the newest source's createdAt but no expected_valid_days, so _effective_fact_staleness_age fell back to the global staleness_age_days. A merge of several 200-day-old stable facts (each evd=3650) would land with no evd, read as a 90-day window, and re-enter the staleness candidate set on the very next cycle - the merge discarded the lifetime signal of the underlying information and contradicted consolidation's premise (these are stable, related facts worth synthesising). Fix: the merged fact inherits expected_valid_days set so it is re-reviewed at the EARLIEST source review deadline (min(createdAt + expected_valid_days) across sources, relative to the merged fact's createdAt = the newest source's). A merge combines details from every source, so a volatile sub-detail (evd=7) must not inherit a stable source's 3650-day window and escape staleness review for years - staleness KEEP/REMOVE is the only path that re-validates a merged fact, so biasing toward the soonest deadline keeps uncertain merges re-checked sooner. A source already past its deadline yields a minimal positive window (review next cycle) rather than the global fallback, which would defer an overdue review. Capped at the creation-time staleness_max_lifetime_multiplier like any new fact. Omitted when no source carries a valid evd (legacy facts fall back to the global age at read time, matching pre-feature behaviour). DRY: extract _read_expected_valid_days(fact) -> int | None, the shared type rule (int/float, reject bool, coerce to int BEFORE the > 0 guard) previously inlined in four places - _normalize_memory_update_fact, _effective_fact_staleness_age, the newFact creation cap, and consolidation inheritance. All four call the single helper. Coercing before the guard matters for values in (0, 1): 0.5 passes a raw > 0 check but truncates to 0, which would violate the helper's "positive int or None" contract; the order now matches the original _normalize_memory_update_fact rule. No prompt/schema change: consolidation's prompt does not surface source evd to the LLM, so asking the model to assign a merged lifetime would be guessing without signal. Source inheritance is deterministic and always available. Tests: consolidation evd cases now use time-stable createdAt (relative to now via a _days_ago helper) covering - earliest-deadline selection, creation-cap clamp, omit when no source evd, volatile source governs the deadline (and re-enters staleness next cycle), overdue source clamps to a minimal window, float coercion. Plus TestReadExpectedValidDays / TestEffectiveFactStalenessAge regression cases for the (0, 1) coercion-order fix.
Consolidation (bytedance#3996) and per-fact expected_valid_days (bytedance#4143) were both authored by the same contributor but never connected: the consolidated new_fact carried the newest source's createdAt but no expected_valid_days, so _effective_fact_staleness_age fell back to the global staleness_age_days. A merge of several 200-day-old stable facts (each evd=3650) would land with no evd, read as a 90-day window, and re-enter the staleness candidate set on the very next cycle - the merge discarded the lifetime signal of the underlying information and contradicted consolidation's premise (these are stable, related facts worth synthesising). Fix: the merged fact inherits expected_valid_days set so it is re-reviewed at the EARLIEST source review deadline (min(createdAt + expected_valid_days) across sources, relative to the merged fact's createdAt = the newest source's). A merge combines details from every source, so a volatile sub-detail (evd=7) must not inherit a stable source's 3650-day window and escape staleness review for years - staleness KEEP/REMOVE is the only path that re-validates a merged fact, so biasing toward the soonest deadline keeps uncertain merges re-checked sooner. A source already past its deadline yields a minimal positive window (review next cycle) rather than the global fallback, which would defer an overdue review. Capped at the creation-time staleness_max_lifetime_multiplier like any new fact. Omitted when no source carries a valid evd (legacy facts fall back to the global age at read time, matching pre-feature behaviour). DRY: extract _read_expected_valid_days(fact) -> int | None, the shared type rule (int/float, reject bool, coerce to int BEFORE the > 0 guard) previously inlined in four places - _normalize_memory_update_fact, _effective_fact_staleness_age, the newFact creation cap, and consolidation inheritance. All four call the single helper. Coercing before the guard matters for values in (0, 1): 0.5 passes a raw > 0 check but truncates to 0, which would violate the helper's "positive int or None" contract; the order now matches the original _normalize_memory_update_fact rule. No prompt/schema change: consolidation's prompt does not surface source evd to the LLM, so asking the model to assign a merged lifetime would be guessing without signal. Source inheritance is deterministic and always available. Tests: consolidation evd cases now use time-stable createdAt (relative to now via a _days_ago helper) covering - earliest-deadline selection, creation-cap clamp, omit when no source evd, volatile source governs the deadline (and re-enters staleness next cycle), overdue source clamps to a minimal window, float coercion. Plus TestReadExpectedValidDays / TestEffectiveFactStalenessAge regression cases for the (0, 1) coercion-order fix.
…ces (#4225) * fix(memory): consolidated facts inherit expected_valid_days from sources Consolidation (#3996) and per-fact expected_valid_days (#4143) were both authored by the same contributor but never connected: the consolidated new_fact carried the newest source's createdAt but no expected_valid_days, so _effective_fact_staleness_age fell back to the global staleness_age_days. A merge of several 200-day-old stable facts (each evd=3650) would land with no evd, read as a 90-day window, and re-enter the staleness candidate set on the very next cycle - the merge discarded the lifetime signal of the underlying information and contradicted consolidation's premise (these are stable, related facts worth synthesising). Fix: the merged fact inherits expected_valid_days set so it is re-reviewed at the EARLIEST source review deadline (min(createdAt + expected_valid_days) across sources, relative to the merged fact's createdAt = the newest source's). A merge combines details from every source, so a volatile sub-detail (evd=7) must not inherit a stable source's 3650-day window and escape staleness review for years - staleness KEEP/REMOVE is the only path that re-validates a merged fact, so biasing toward the soonest deadline keeps uncertain merges re-checked sooner. A source already past its deadline yields a minimal positive window (review next cycle) rather than the global fallback, which would defer an overdue review. Capped at the creation-time staleness_max_lifetime_multiplier like any new fact. Omitted when no source carries a valid evd (legacy facts fall back to the global age at read time, matching pre-feature behaviour). DRY: extract _read_expected_valid_days(fact) -> int | None, the shared type rule (int/float, reject bool, coerce to int BEFORE the > 0 guard) previously inlined in four places - _normalize_memory_update_fact, _effective_fact_staleness_age, the newFact creation cap, and consolidation inheritance. All four call the single helper. Coercing before the guard matters for values in (0, 1): 0.5 passes a raw > 0 check but truncates to 0, which would violate the helper's "positive int or None" contract; the order now matches the original _normalize_memory_update_fact rule. No prompt/schema change: consolidation's prompt does not surface source evd to the LLM, so asking the model to assign a merged lifetime would be guessing without signal. Source inheritance is deterministic and always available. Tests: consolidation evd cases now use time-stable createdAt (relative to now via a _days_ago helper) covering - earliest-deadline selection, creation-cap clamp, omit when no source evd, volatile source governs the deadline (and re-enters staleness next cycle), overdue source clamps to a minimal window, float coercion. Plus TestReadExpectedValidDays / TestEffectiveFactStalenessAge regression cases for the (0, 1) coercion-order fix. * fix(memory): reject non-finite expected_valid_days before int coercion The shared _read_expected_valid_days helper (introduced when consolidating the evd type rule across four call sites) coerces with int(raw) before the > 0 guard - reversing the original _normalize_memory_update_fact order so that a fractional 0.5 does not leak as 0. But int(raw) raises for non-finite floats: int(nan) raises ValueError and int(inf)/int(-inf) raise OverflowError. Python's JSON decoder accepts NaN / Infinity as floats by default, so a single malformed expected_valid_days in a hand-edited memory.json would abort staleness selection or consolidation instead of falling back to the global lifetime. On main, _effective_fact_staleness_age checked raw > 0 first, so NaN fell back safely (nan > 0 is false) - but inf did NOT (inf > 0 is true, so main also crashed on inf). This helper is now the persisted-fact read path for both staleness and consolidation, so the regression (and the pre-existing inf crash) must be closed here. Fix: require math.isfinite(float(raw)) before the int() coercion, then keep the existing positivity check and fallback. NaN / +/-inf all return None, so callers fall back to the global staleness_age_days. Normal int/float values (including large ones) are unaffected - isfinite is a no-op for them. Tests: - TestReadExpectedValidDays.test_rejects_non_finite_values - NaN, inf, -inf return None (not raise). - TestEffectiveFactStalenessAge.test_falls_back_for_non_finite_values - the persisted-fact read path returns the global age for each, no raise. - test_consolidation_with_non_finite_source_evd_does_not_raise - end-to-end: a NaN-evd source merged with a stable source does not abort consolidation; the NaN source's effective lifetime falls back to the global 90 and its deadline participates in the earliest-deadline computation. * test(memory): hoist _select_stale_candidates import + tidy deadline docstring Two review nits from the latest pass: - `_select_stale_candidates` was imported inline inside three test methods; hoisted to the module-level import block so the dependency is declared once. - `test_consolidated_evd_volatile_source_with_equal_created_at_future_deadline` had an abandoned calculation in its comment ("3 + 7 = 10 ... minus 3 already elapsed = 7? No:") that could mislead future readers into thinking elapsed-since-creation factors into the inherited window. Collapsed to a single clear line stating the window is relative to the merged createdAt, regardless of the source's current age. * fix(memory): reject huge-int expected_valid_days above timedelta.max.days _read_expected_valid_days routed every numeric value through float(raw) for the math.isfinite guard, but Python's JSON decoder parses an integer literal with no decimal point as an arbitrary-precision int (unlike 1e400, which decodes to float inf). So a hand-edited memory.json carrying "expected_valid_days": 10**400 raised OverflowError in float(raw) before math.isfinite was ever called - exactly the malformed-field-aborts-everything scenario the helper's docstring claims to prevent. The earlier non-finite fix only closed the float cases (NaN / +/-inf / 1e400). A huge int below the float limit but above timedelta.max.days (e.g. 10**12) would pass the helper and raise OverflowError downstream in timedelta(days=evd) during staleness selection or consolidation - the same crash fancyboi999 flagged for extend_by_days, just reached via a stored evd. Fix: branch on type so an int never passes through float() (matching the reviewer's suggestion), AND cap the returned int at timedelta.max.days (999999999) so the downstream timedelta(days=evd) call cannot overflow either. The float branch keeps the isfinite + int() coercion. Both branches share the 0 < evd <= timedelta.max.days positivity/range check. Normal values are unaffected - any legitimate expected_valid_days is far below the cap (the config ceiling staleness_max_extension_days tops out at 36500). Tests (all three layers): - TestReadExpectedValidDays.test_rejects_huge_int_above_timedelta_max - 10**400, 10**12, 10**9, timedelta.max.days+1 return None; timedelta.max.days itself is accepted. - TestEffectiveFactStalenessAge.test_falls_back_for_huge_int_above_timedelta_max - the persisted-fact read path returns the global age, no raise. - test_consolidation_with_huge_int_source_evd_does_not_raise[1e400|1e12|1e9] - parametrized end-to-end: a huge-int-evd source merged with a stable source does not abort consolidation; the bad source falls back to the global 90. * fix(memory): guard datetime arithmetic, not just timedelta construction The huge-int fix capped _read_expected_valid_days at timedelta.max.days, but that only proves timedelta(days=evd) can be constructed - adding it to a real fact timestamp still overflows datetime.max. @fancyboi999 reproduced it: a source with expected_valid_days=timedelta.max.days raises "OverflowError: date value out of range" at dt + timedelta(...) in the new consolidation deadline calculation. capping at timedelta.max.days was another patch chasing the next overflow boundary, not a real close. Root cause: the helper was doing datetime-range validation, but the safe bound depends on the datetime the evd is added to, not on the evd alone. So the responsibility moves to the arithmetic site, with try/except as the terminal guard - no concrete upper bound to be wrong about. Changes: - _read_expected_valid_days returns any positive int (huge ints included, not routed through float). Its job is type/positivity validation only. - New _safe_add_days(dt, days) -> datetime | None wraps dt + timedelta(days), returning None on OverflowError/ValueError. This is the terminal guard - there is no further boundary to overflow because try/except catches any datetime-range failure regardless of magnitude. - _select_stale_candidates uses _safe_add_days(now, -effective_age); a None result means the window is unrepresentably large, so the fact cannot yet be stale and is skipped (not selected). - Consolidation computes each source's deadline via _safe_add_days; a source whose deadline overflows falls back to the global staleness_age_days deadline (same treatment as a legacy no-evd source), so one malformed field cannot abort the merge. Normal values are unaffected - any legitimate expected_valid_days is far below the overflow boundary (the config ceiling staleness_max_extension_days tops out at 36500). Tests: - TestSafeAddDays: normal/negative shifts; 10**400/10**12/10**9 return None; timedelta.max.days (the exact reproduced value) returns None, not raises. - TestSelectStaleCandidates.test_huge_evd_does_not_abort_selection: a fact with a huge evd is skipped, not selected, and selection does not raise. - test_consolidation_with_huge_int_source_evd_does_not_raise now parametrized over [1e400, 1e12, 1e9, timedelta.max.days] - the last is the value that constructs a valid timedelta but overflows datetime arithmetic. - Helper/read-path tests updated to assert huge ints are returned as-is (the overflow guard is no longer in the helper).
…bytedance#3996) * feat(memory): add memory consolidation to synthesize fragmented facts When a fact category accumulates many individual entries, the LLM reviews them during the normal memory-update call (same invocation, no extra API cost) and decides whether groups of related facts can be synthesized into a single richer fact. This completes the memory lifecycle: extraction → guaranteed injection → staleness review → consolidation. - Select fragmented categories by min-facts threshold, surface the most fragmented groups first; prompt-layer caps aligned with apply-layer guardrails so the LLM never sees groups it cannot act on - Cap consolidated confidence at source maximum to prevent inflation; reject results below fact_confidence_threshold - Double-consume protection prevents a fact from being merged into multiple consolidation targets - Feature-gated at both prompt and apply time with per-cycle safety caps - Add 26 tests covering candidate selection, normalization, apply guardrails, and prompt integration * fix(memory): address consolidation correctness issues from PR review Six fixes based on maintainer review of bytedance#3996: 1. Deduplicate sourceIds in normalization — ["f1","f1"] previously bypassed the ≥2-distinct-sources check; dict.fromkeys collapses it to ["f1"] which is correctly rejected. 2. Run consolidation after max_facts trim — previously, sources were deleted then the merged fact could be evicted by the trim, leaving no record of either. Moving consolidation last ensures source facts exist in the post-trim index before removal. 3. Fix count= attribute in consolidation prompt — advertised the full category size but listed only max_sources IDs; now uses min(len(group), max_sources) to match what the LLM can act on. 4. Exempt staleness_protected_categories from consolidation candidates — mirrors the existing staleness-review contract so correction facts are never surfaced for merging. 5. Strip and default category in consolidation normalization — " " or " preference " are now normalised, matching _normalize_memory_update_fact. 6. Propagate sourceError from source facts into consolidated fact — correction context is no longer silently lost on merge. * fix(memory): add apply-time guardrails and tests for consolidation P1: mirror the staleness-pass defense-in-depth pattern — build allowed_source_ids from _select_consolidation_candidates at apply time so a protected-category or below-threshold fact proposed by the LLM is rejected regardless of model behavior. P2a: test that LLM-returned confidence is capped at max source confidence and that a capped result below fact_confidence_threshold is rejected. P2b: test that factsToConsolidate with consolidation_enabled=False is a no-op at apply time (35 tests, all pass). * fix(memory): address three correctness issues from second review round 1. Default consolidation_enabled=False — consolidation is lossy (source content is permanently replaced, only consolidatedFrom IDs preserved); new lossy features default to off. config.example.yaml updated to match. 2. Unify confidence coercion between prompt and apply — _build_consolidation_section now calls _coerce_source_confidence(fact) instead of an inline 0.0-default coercion, so a null-confidence fact renders with 0.50 in the LLM prompt and is capped at 0.50 at apply time (same value, same function). 3. Preserve staleness clock on merge — consolidated fact now carries the newest source's createdAt (not now) so aged information does not gain a fresh staleness-review window just by being consolidated; consolidatedAt is added as an explicit audit field. Three regression tests added (default=false, null-confidence consistency, createdAt policy); all guardrail tests now set consolidation_enabled=True explicitly so they test the guardrail, not the feature flag. 38 tests pass. * fix(memory): harden createdAt comparison and confidence handling 1. createdAt max via _parse_fact_datetime — replaces string max() which crashes on non-string createdAt (numeric unix timestamps) and sorts Z/+00:00 mixed formats incorrectly. Mirrors how staleness computes age. 2. Remove dead min(..., 1.0) — _coerce_source_confidence already clamps each source confidence to [0, 1], so max(source_confidences) ≤ 1.0 by contract; the outer min could never bind. 3. Clamp raw_llm_conf to [0, 1] before applying the source cap — out-of- range values like 1.5 are safe today (pinned by the cap) but defensively clamped first so the invariant holds even if the cap is ever loosened. 4. Doc: expand the apply-time guardrails comment to call out the protected- category exclusion via allowed_source_ids — this is the central safety property ("explicit user feedback is never silently merged away"). 5. Test: add test_confidence_fallback_to_max_source_when_llm_omits_field covering the else-branch (LLM omits confidence → uses max_source_conf). 6. Fix lint: reorder imports in test file (stdlib before third-party). 39 tests, all pass. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
…tedance#4028) * fix(security): html-escape fact content in memory prompt sections Raw memory fact content was injected verbatim into prompt XML — a fact containing a literal `"` could break the `"..."` delimiter, and a closing tag like `</consolidation_candidates>` could prematurely end the XML block, both potentially confusing the model. Apply `html.escape()` to `content` in `_build_staleness_section` and `_build_consolidation_section`, and to `cat` in the consolidation section's XML attribute. Tests added for both sections covering special characters, XML tag injection, and attribute injection. Follow-up to bytedance#3996 as noted by reviewer willem-bd. * fix(security): address reviewer follow-ups on html-escaping PR - Escape `cat` in _build_staleness_section for symmetry with the consolidation section (both sections now consistently html-escape all LLM-derived category values that appear in the prompt) - Add comment at current_memory=json.dumps() documenting the conscious accept: json.dumps leaves < > & unescaped; lower-risk than staleness/consolidation (read-only context, not delete/merge instructions); fix at fact-content insert time if revisited - Add test for category escaping in the staleness section * fix(security): reference tracking issue bytedance#4044 in conscious-accept comment * style: compress conscious-accept comment to two lines
…m backend (bytedance#4122) * feat(memory): pluggable + self-contained memory system (MemoryManager plan phases 1 & 2) Phase 1 — Pluggable (steps 0-10): - ABC MemoryManager (9 methods) + singleton factory + drop-in backend discovery - DeerMem default backend with core/ (storage/queue/updater/prompt/message_processing) - NoopMemoryManager backend (proves pluggability) - All call sites (middleware/hook/prompt/gateway/client/app) routed through manager - hasattr capability probing for DeerMem-internal methods (no hard imports) - MemoryConfig gains manager_class field; shared vs DeerMem-private annotated Phase 2 — Self-contained DeerMem (steps 11-18): - backend_config passthrough + DeerMemConfig (all DeerMem-private fields moved off MemoryConfig) - DI: DeerMem owns storage/queue/updater/llm as instance attributes (no global singletons) - Storage independence: core/paths.py with own root (~/.deermem or ), factory auto-injects deer-flow's runtime_home() as absolute base_dir (zero-config) - LLM independence: core/llm.py via langchain init_chat_model (no create_chat_model) - Trace independence: optional tracing_callback replaces inject_langfuse_metadata/request_trace_context - Message processing independence: hide_from_ui default-skip + optional should_keep_hidden_message hook - Internal imports → relative (only deer_mem.py ABC import is host-relative) - Carrier (deer_mem.py adapter) / portable (deermem/ config+core) split - New tests: test_deermem_self_contained + test_memory_manager_pluggable; all memory tests migrated - Other-agent demo: samples/other_agent_demo/ + automated portability test - config.example.yaml memory section updated to phase-2 schema * feat(memory): port consolidation + staleness fix into self-contained DeerMem; phase-2 host hooks Port upstream bytedance#3996 (memory consolidation) and bytedance#3993 (staleness KeyError fix) from origin/MemoryManager into the pluggable, self-contained DeerMem structure (backends/deermem/deermem/), adapted to the DI MemoryUpdater (config injected, not get_memory_config globals): - DeerMemConfig: add consolidation_enabled (opt-in, default false) / consolidation_min_facts / consolidation_max_groups_per_cycle / consolidation_max_sources - prompt.py: factsToConsolidate JSON field + {consolidation_section} placeholder + CONSOLIDATION_PROMPT constant - updater.py: _coerce_source_confidence / _select_consolidation_candidates / _build_consolidation_section module helpers (matching the existing _select_stale_candidates style); consolidation normalization in _normalize_memory_update_data; consolidation apply in _apply_updates (after max_facts trim, with apply-time guardrails mirroring staleness); staleness KeyError fix (f["id"] -> f.get("id") is not None) applied to both the staleness guardrail and the consolidation allowed_source_ids comprehension - config.example.yaml: consolidation section under memory.backend_config - tests/test_memory_consolidation.py: 40 DI-adapted tests (running, not skipped) incl. the staleness KeyError regression Also includes in-flight phase-2 host-integration work: storage_path semantics (any absolute/relative value = root dir) and host-default tracing_callback / should_keep_hidden_message hooks injected into backend_config by the factory. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): add noop backend template and backends guide - backends/noop/: complete drop-in template (config.py with zero deer-flow imports, noop_manager.py with a 6-step new-backend walkthrough in its docstring, commented optional fact-CRUD capabilities). - backends/README.md: which files to touch when adding/swapping a backend, the 5-item backend contract, and common pitfalls. - manager.py: generalize backend examples in comments (drop mem0-specific references). Co-Authored-By: Claude <noreply@anthropic.com> * fix(frontend): guard formatTimeAgo against invalid timestamps Return a neutral placeholder when the input date is invalid (e.g. an empty lastUpdated from a backend with no memories) instead of throwing 'Invalid time value' from date-fns. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): wire tool-driven memory mode through the MemoryManager ABC tools.py (memory_search/add/update/delete) now calls get_memory_manager() instead of the removed host memory module, so tool mode (memory.mode: tool) works for any backend. DeerMem.search is implemented (case-insensitive substring match, ranked by confidence) as a stand-in for the planned semantic retrieval; noop.search returns [] (unchanged). Fact-CRUD tools use getattr+callable probing -- backends lacking those ops (noop) get a clear JSON error instead of crashing. Tests: test_memory_tools rewired to mock the manager (handler tests) + TestModeGating retained; test_memory_search now covers DeerMem.search; pluggable stubs test updated (search no longer a stub). Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (import sorting, type annotation quotes, E402 in skipped tests) * docs: restore explanatory comments in config.example.yaml memory section * fix(security): port html-escape memory facts fix (bytedance#4097) to vendored DeerMem prompt.py * fix(memory): address review + port dropped upstream memory fixes Review blockers (vendored DeerMem): - bytedance#4044 restore _escape_memory_for_prompt (current_memory blob in MEMORY_UPDATE_PROMPT) - prevents </current_memory> breakout - bytedance#4028 html.escape staleness-section cat/content in _build_staleness_section - bytedance#4119 add _escape_summary for injection-path summaries (Work/Personal/ Current Focus/Recent/Earlier/Background) - default-model silent no-op: factory injects host default chat model via a new host_llm slot (create_chat_model(name=None)); DeerMem prefers host_llm over build_llm(model). Zero-config extraction works out of the box again - MemoryConfigResponse: fix stale docstring (backend-agnostic shape; DeerMem knobs live under backend_config, not top-level - restoring flat would re-couple the API to DeerMem). Frontend audited: does not read /memory/config - _host_default_tracing_callback: restore langfuse assistant_id/environment - search: push category onto the ABC signature; DeerMem filters BEFORE the top_k slice (was filtered client-side after slicing -> starved results) - _do_update_memory_sync: split into wrapper+impl; bind trace_id into the request-trace ContextVar on the Timer/executor worker via a new trace_context_manager host hook (None trace_id left unbound - no fabrication) - client.py fact-CRUD now passes user_id (was writing to the global bucket while get_memory reads per-user) - _resolve_manager_class: fail-fast (raise ValueError) on an unresolved explicit manager_class instead of silently falling back to DeerMem (memory is persistent state - a wrong store is a silent data-integrity footgun) Upstream memory fixes dropped by the host->vendored rename conflict, re-ported to backends/deermem/deermem/core/ (+ deer_mem.py): - bytedance#4073 queue busy-timer-spin -> _reprocess_pending flag (core/queue.py) - bytedance#4074 null source.confidence in staleness -> _coerce_source_confidence (core/updater.py: _build_staleness_section + _apply_updates stale sort) - bytedance#4075 factsToRemove is optional (drop from _REQUIRED_MEMORY_UPDATE_TOP_LEVEL_KEYS) - bytedance#4076 null confidence in search ranking -> _coerce_source_confidence (deer_mem.py DeerMem.search) host_llm + trace_context_manager are host-injected via backend_config (factory in manager.py), keeping backends/deermem/ at exactly one `from deerflow` line (the ABC contract) - portability test preserved. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (F541 f-string without placeholders, E501 line too long) * fix(memory): restore hide_from_ui clarification preservation, expose mode Two memory-system fixes (F541/E501 lint was already fixed on this branch): - filter_messages_for_memory: restore default preservation of well-formed human_input_response clarification answers (v2 regression). The self-containment refactor made the bare function skip ALL hide_from_ui when no hook was passed, but upstream preserves well-formed clarification responses by default (test_hide_from_ui_human_input_response_is_preserved). Inline a host-agnostic _is_human_clarification_response mirror of read_human_input_response as the default keep-decision; the host-injected should_keep_hidden_message hook still overrides (production path unchanged). Portable package stays zero `from deerflow`. - /memory/config: expose `mode` (middleware|tool) in MemoryConfigResponse + the config/status endpoints + client.get_memory_config. mode is a host- shared, behavior-determining field missing from the response projection. Sync tests (mock .mode; e2e assert mode present). - Align manager_class field docstring with fail-fast behavior. Tests: filter/self-contained/portability (35) + memory-config (4) pass; ruff clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): resolve ruff format failures in memory module + tests `make lint` runs `ruff format --check` in addition to `ruff check`; 8 memory files had pending format changes -- 7 pre-existing (deer_mem, updater, tools, test_memory_queue/router/search/tools) + message_processing from the hide_from_ui fix. Apply `ruff format`: whitespace/wrapping only, no logic change. 109 memory tests pass; ruff check + format --check both clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - legacy field migration, fact_id contract, path/docs Address willem-bd's review on PR head bc8bf0d (risk:high, persistent state): - config: auto-migrate pre-abstraction top-level memory.* DeerMem fields (storage_path, max_facts, debounce_seconds, model_name, token_counting, staleness_*, consolidation_*) into backend_config on load + warn, so an upgrade does NOT silently revert customized settings (was: silent extra='ignore' drop). model_name -> backend_config.model.model. Unknown top-level keys warned. - factory: resolve a relative backend_config.storage_path against runtime_home() (base_dir-relative, CWD-independent) to preserve pre-abstraction semantics; paths.py stays portable (no runtime_home import). - tools: memory_add uses the fact_id returned directly by create_fact instead of re-deriving it via content-key matching (coupled the tool to the backend's content normalization; could misreport a storage cap). create_fact now returns (memory_data, fact_id); gateway/client/tool updated. Fix terse {"error":"content"} -> {"error":"empty content"}. - app.py: update stale token_counting=="char" warm-up comment to point at manager.warm (DeerMem.warm re-checks char and returns early). - router: comment explaining reload_memory silent fallback vs fact 501 asymmetry (read-only degrade vs write fail-loud). - CHANGELOG: document breaking changes (/memory/config + client.get_memory_config shape flat->backend_config; custom storage_class path moved + __init__ must accept config) and the legacy-field auto-migration. - tests: add regression test pinning the per-user memory path ({storage_path}/users/{safe_user_id}/memory.json == host make_safe_user_id) across the abstraction; update create_fact mocks for (memory_data, fact_id). Tests: 273 passed (memory suite); ruff check + format clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - storage_path, max_facts, tracing, parsing Six review findings (willem-bd), each verified against upstream: - storage_path semantics (file -> root dir): migration drops file-style (.json) legacy values with a warning; factory raises if storage_path resolves to an existing file (avoid silent NotADirectoryError write failure). CHANGELOG + config.example.yaml comment updated. - create_memory_fact enforces max_facts again (via _trim_facts_to_max) and returns (memory, None) when the cap evicts the new fact; memory_add tool reports "not stored", client raises ValueError, POST /memory/facts -> 409. - max_facts trim uses _coerce_source_confidence (was raw f.get("confidence", 0) -> TypeError on non-float imported/legacy confidence, swallowed as silent update failure). - memory-tracing assistant_id restored to "memory_agent" (was "lead-agent" copy-paste; matches upstream + DeerMem run_name). - _is_human_clarification_response cross-checked against read_human_input_response (drift guard test). - empty-string legacy values skipped silently in migration (narrow fix, not broad "if not value" which would skip explicit bool False). 8 new regression tests. make lint + 406 memory tests pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address internal review - storage fail-fast, build_llm degrade, config warn, noop template Addresses 4 findings from the PR bytedance#4122 internal supplemental review (parallel to willem-bd's review, no overlap): - create_storage fail-fast: a misspelled/unimportable storage_class now raises ValueError instead of silently falling back to FileMemoryStorage. Memory is persistent state, so a wrong store is a data-integrity footgun; mirrors the existing manager_class resolution policy. (storage.py) - noop template create_fact signature: the commented template used keyword-only `content` and returned a bare dict, while DeerMem's actual create_fact takes positional `content` and returns tuple[dict, str|None] (the memory_add tool passes content positionally; gateway/client/tools all tuple-unpack). A backend copied from the template would 500 on fact-CRUD. Template fixed; delete_fact/update_fact templates left (callers compatible). (noop_manager.py) - build_llm graceful degrade: wrap init_chat_model in try/except, degrade to None + WARNING on failure (mirroring _host_default_llm) so a misconfigured explicit model does not crash app startup -- non-LLM memory ops still work and an update raises at runtime with the error logged. (llm.py) - from_backend_config unknown-key warning: log a WARNING for unknown backend_config keys (mirrors the host layer's load_memory_config_from_dict) so a typo like `storage_pat` does not silently fall back to the default and write memory to an unintended location. (config.py) Tests: rewrote 3 create_storage fallback tests to expect ValueError; added 4 tests (build_llm zero-config/degrade, from_backend_config warn/silent). make lint green; full memory suite passes. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: lllyfff <2281215061@qq.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: lllyfff <122260771+lllyfff@users.noreply.github.com>
…ces (bytedance#4225) * fix(memory): consolidated facts inherit expected_valid_days from sources Consolidation (bytedance#3996) and per-fact expected_valid_days (bytedance#4143) were both authored by the same contributor but never connected: the consolidated new_fact carried the newest source's createdAt but no expected_valid_days, so _effective_fact_staleness_age fell back to the global staleness_age_days. A merge of several 200-day-old stable facts (each evd=3650) would land with no evd, read as a 90-day window, and re-enter the staleness candidate set on the very next cycle - the merge discarded the lifetime signal of the underlying information and contradicted consolidation's premise (these are stable, related facts worth synthesising). Fix: the merged fact inherits expected_valid_days set so it is re-reviewed at the EARLIEST source review deadline (min(createdAt + expected_valid_days) across sources, relative to the merged fact's createdAt = the newest source's). A merge combines details from every source, so a volatile sub-detail (evd=7) must not inherit a stable source's 3650-day window and escape staleness review for years - staleness KEEP/REMOVE is the only path that re-validates a merged fact, so biasing toward the soonest deadline keeps uncertain merges re-checked sooner. A source already past its deadline yields a minimal positive window (review next cycle) rather than the global fallback, which would defer an overdue review. Capped at the creation-time staleness_max_lifetime_multiplier like any new fact. Omitted when no source carries a valid evd (legacy facts fall back to the global age at read time, matching pre-feature behaviour). DRY: extract _read_expected_valid_days(fact) -> int | None, the shared type rule (int/float, reject bool, coerce to int BEFORE the > 0 guard) previously inlined in four places - _normalize_memory_update_fact, _effective_fact_staleness_age, the newFact creation cap, and consolidation inheritance. All four call the single helper. Coercing before the guard matters for values in (0, 1): 0.5 passes a raw > 0 check but truncates to 0, which would violate the helper's "positive int or None" contract; the order now matches the original _normalize_memory_update_fact rule. No prompt/schema change: consolidation's prompt does not surface source evd to the LLM, so asking the model to assign a merged lifetime would be guessing without signal. Source inheritance is deterministic and always available. Tests: consolidation evd cases now use time-stable createdAt (relative to now via a _days_ago helper) covering - earliest-deadline selection, creation-cap clamp, omit when no source evd, volatile source governs the deadline (and re-enters staleness next cycle), overdue source clamps to a minimal window, float coercion. Plus TestReadExpectedValidDays / TestEffectiveFactStalenessAge regression cases for the (0, 1) coercion-order fix. * fix(memory): reject non-finite expected_valid_days before int coercion The shared _read_expected_valid_days helper (introduced when consolidating the evd type rule across four call sites) coerces with int(raw) before the > 0 guard - reversing the original _normalize_memory_update_fact order so that a fractional 0.5 does not leak as 0. But int(raw) raises for non-finite floats: int(nan) raises ValueError and int(inf)/int(-inf) raise OverflowError. Python's JSON decoder accepts NaN / Infinity as floats by default, so a single malformed expected_valid_days in a hand-edited memory.json would abort staleness selection or consolidation instead of falling back to the global lifetime. On main, _effective_fact_staleness_age checked raw > 0 first, so NaN fell back safely (nan > 0 is false) - but inf did NOT (inf > 0 is true, so main also crashed on inf). This helper is now the persisted-fact read path for both staleness and consolidation, so the regression (and the pre-existing inf crash) must be closed here. Fix: require math.isfinite(float(raw)) before the int() coercion, then keep the existing positivity check and fallback. NaN / +/-inf all return None, so callers fall back to the global staleness_age_days. Normal int/float values (including large ones) are unaffected - isfinite is a no-op for them. Tests: - TestReadExpectedValidDays.test_rejects_non_finite_values - NaN, inf, -inf return None (not raise). - TestEffectiveFactStalenessAge.test_falls_back_for_non_finite_values - the persisted-fact read path returns the global age for each, no raise. - test_consolidation_with_non_finite_source_evd_does_not_raise - end-to-end: a NaN-evd source merged with a stable source does not abort consolidation; the NaN source's effective lifetime falls back to the global 90 and its deadline participates in the earliest-deadline computation. * test(memory): hoist _select_stale_candidates import + tidy deadline docstring Two review nits from the latest pass: - `_select_stale_candidates` was imported inline inside three test methods; hoisted to the module-level import block so the dependency is declared once. - `test_consolidated_evd_volatile_source_with_equal_created_at_future_deadline` had an abandoned calculation in its comment ("3 + 7 = 10 ... minus 3 already elapsed = 7? No:") that could mislead future readers into thinking elapsed-since-creation factors into the inherited window. Collapsed to a single clear line stating the window is relative to the merged createdAt, regardless of the source's current age. * fix(memory): reject huge-int expected_valid_days above timedelta.max.days _read_expected_valid_days routed every numeric value through float(raw) for the math.isfinite guard, but Python's JSON decoder parses an integer literal with no decimal point as an arbitrary-precision int (unlike 1e400, which decodes to float inf). So a hand-edited memory.json carrying "expected_valid_days": 10**400 raised OverflowError in float(raw) before math.isfinite was ever called - exactly the malformed-field-aborts-everything scenario the helper's docstring claims to prevent. The earlier non-finite fix only closed the float cases (NaN / +/-inf / 1e400). A huge int below the float limit but above timedelta.max.days (e.g. 10**12) would pass the helper and raise OverflowError downstream in timedelta(days=evd) during staleness selection or consolidation - the same crash fancyboi999 flagged for extend_by_days, just reached via a stored evd. Fix: branch on type so an int never passes through float() (matching the reviewer's suggestion), AND cap the returned int at timedelta.max.days (999999999) so the downstream timedelta(days=evd) call cannot overflow either. The float branch keeps the isfinite + int() coercion. Both branches share the 0 < evd <= timedelta.max.days positivity/range check. Normal values are unaffected - any legitimate expected_valid_days is far below the cap (the config ceiling staleness_max_extension_days tops out at 36500). Tests (all three layers): - TestReadExpectedValidDays.test_rejects_huge_int_above_timedelta_max - 10**400, 10**12, 10**9, timedelta.max.days+1 return None; timedelta.max.days itself is accepted. - TestEffectiveFactStalenessAge.test_falls_back_for_huge_int_above_timedelta_max - the persisted-fact read path returns the global age, no raise. - test_consolidation_with_huge_int_source_evd_does_not_raise[1e400|1e12|1e9] - parametrized end-to-end: a huge-int-evd source merged with a stable source does not abort consolidation; the bad source falls back to the global 90. * fix(memory): guard datetime arithmetic, not just timedelta construction The huge-int fix capped _read_expected_valid_days at timedelta.max.days, but that only proves timedelta(days=evd) can be constructed - adding it to a real fact timestamp still overflows datetime.max. @fancyboi999 reproduced it: a source with expected_valid_days=timedelta.max.days raises "OverflowError: date value out of range" at dt + timedelta(...) in the new consolidation deadline calculation. capping at timedelta.max.days was another patch chasing the next overflow boundary, not a real close. Root cause: the helper was doing datetime-range validation, but the safe bound depends on the datetime the evd is added to, not on the evd alone. So the responsibility moves to the arithmetic site, with try/except as the terminal guard - no concrete upper bound to be wrong about. Changes: - _read_expected_valid_days returns any positive int (huge ints included, not routed through float). Its job is type/positivity validation only. - New _safe_add_days(dt, days) -> datetime | None wraps dt + timedelta(days), returning None on OverflowError/ValueError. This is the terminal guard - there is no further boundary to overflow because try/except catches any datetime-range failure regardless of magnitude. - _select_stale_candidates uses _safe_add_days(now, -effective_age); a None result means the window is unrepresentably large, so the fact cannot yet be stale and is skipped (not selected). - Consolidation computes each source's deadline via _safe_add_days; a source whose deadline overflows falls back to the global staleness_age_days deadline (same treatment as a legacy no-evd source), so one malformed field cannot abort the merge. Normal values are unaffected - any legitimate expected_valid_days is far below the overflow boundary (the config ceiling staleness_max_extension_days tops out at 36500). Tests: - TestSafeAddDays: normal/negative shifts; 10**400/10**12/10**9 return None; timedelta.max.days (the exact reproduced value) returns None, not raises. - TestSelectStaleCandidates.test_huge_evd_does_not_abort_selection: a fact with a huge evd is skipped, not selected, and selection does not raise. - test_consolidation_with_huge_int_source_evd_does_not_raise now parametrized over [1e400, 1e12, 1e9, timedelta.max.days] - the last is the value that constructs a valid timedelta but overflows datetime arithmetic. - Helper/read-path tests updated to assert huge ints are returned as-is (the overflow guard is no longer in the helper).
…bytedance#3996) * feat(memory): add memory consolidation to synthesize fragmented facts When a fact category accumulates many individual entries, the LLM reviews them during the normal memory-update call (same invocation, no extra API cost) and decides whether groups of related facts can be synthesized into a single richer fact. This completes the memory lifecycle: extraction → guaranteed injection → staleness review → consolidation. - Select fragmented categories by min-facts threshold, surface the most fragmented groups first; prompt-layer caps aligned with apply-layer guardrails so the LLM never sees groups it cannot act on - Cap consolidated confidence at source maximum to prevent inflation; reject results below fact_confidence_threshold - Double-consume protection prevents a fact from being merged into multiple consolidation targets - Feature-gated at both prompt and apply time with per-cycle safety caps - Add 26 tests covering candidate selection, normalization, apply guardrails, and prompt integration * fix(memory): address consolidation correctness issues from PR review Six fixes based on maintainer review of bytedance#3996: 1. Deduplicate sourceIds in normalization — ["f1","f1"] previously bypassed the ≥2-distinct-sources check; dict.fromkeys collapses it to ["f1"] which is correctly rejected. 2. Run consolidation after max_facts trim — previously, sources were deleted then the merged fact could be evicted by the trim, leaving no record of either. Moving consolidation last ensures source facts exist in the post-trim index before removal. 3. Fix count= attribute in consolidation prompt — advertised the full category size but listed only max_sources IDs; now uses min(len(group), max_sources) to match what the LLM can act on. 4. Exempt staleness_protected_categories from consolidation candidates — mirrors the existing staleness-review contract so correction facts are never surfaced for merging. 5. Strip and default category in consolidation normalization — " " or " preference " are now normalised, matching _normalize_memory_update_fact. 6. Propagate sourceError from source facts into consolidated fact — correction context is no longer silently lost on merge. * fix(memory): add apply-time guardrails and tests for consolidation P1: mirror the staleness-pass defense-in-depth pattern — build allowed_source_ids from _select_consolidation_candidates at apply time so a protected-category or below-threshold fact proposed by the LLM is rejected regardless of model behavior. P2a: test that LLM-returned confidence is capped at max source confidence and that a capped result below fact_confidence_threshold is rejected. P2b: test that factsToConsolidate with consolidation_enabled=False is a no-op at apply time (35 tests, all pass). * fix(memory): address three correctness issues from second review round 1. Default consolidation_enabled=False — consolidation is lossy (source content is permanently replaced, only consolidatedFrom IDs preserved); new lossy features default to off. config.example.yaml updated to match. 2. Unify confidence coercion between prompt and apply — _build_consolidation_section now calls _coerce_source_confidence(fact) instead of an inline 0.0-default coercion, so a null-confidence fact renders with 0.50 in the LLM prompt and is capped at 0.50 at apply time (same value, same function). 3. Preserve staleness clock on merge — consolidated fact now carries the newest source's createdAt (not now) so aged information does not gain a fresh staleness-review window just by being consolidated; consolidatedAt is added as an explicit audit field. Three regression tests added (default=false, null-confidence consistency, createdAt policy); all guardrail tests now set consolidation_enabled=True explicitly so they test the guardrail, not the feature flag. 38 tests pass. * fix(memory): harden createdAt comparison and confidence handling 1. createdAt max via _parse_fact_datetime — replaces string max() which crashes on non-string createdAt (numeric unix timestamps) and sorts Z/+00:00 mixed formats incorrectly. Mirrors how staleness computes age. 2. Remove dead min(..., 1.0) — _coerce_source_confidence already clamps each source confidence to [0, 1], so max(source_confidences) ≤ 1.0 by contract; the outer min could never bind. 3. Clamp raw_llm_conf to [0, 1] before applying the source cap — out-of- range values like 1.5 are safe today (pinned by the cap) but defensively clamped first so the invariant holds even if the cap is ever loosened. 4. Doc: expand the apply-time guardrails comment to call out the protected- category exclusion via allowed_source_ids — this is the central safety property ("explicit user feedback is never silently merged away"). 5. Test: add test_confidence_fallback_to_max_source_when_llm_omits_field covering the else-branch (LLM omits confidence → uses max_source_conf). 6. Fix lint: reorder imports in test file (stdlib before third-party). 39 tests, all pass. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
…tedance#4028) * fix(security): html-escape fact content in memory prompt sections Raw memory fact content was injected verbatim into prompt XML — a fact containing a literal `"` could break the `"..."` delimiter, and a closing tag like `</consolidation_candidates>` could prematurely end the XML block, both potentially confusing the model. Apply `html.escape()` to `content` in `_build_staleness_section` and `_build_consolidation_section`, and to `cat` in the consolidation section's XML attribute. Tests added for both sections covering special characters, XML tag injection, and attribute injection. Follow-up to bytedance#3996 as noted by reviewer willem-bd. * fix(security): address reviewer follow-ups on html-escaping PR - Escape `cat` in _build_staleness_section for symmetry with the consolidation section (both sections now consistently html-escape all LLM-derived category values that appear in the prompt) - Add comment at current_memory=json.dumps() documenting the conscious accept: json.dumps leaves < > & unescaped; lower-risk than staleness/consolidation (read-only context, not delete/merge instructions); fix at fact-content insert time if revisited - Add test for category escaping in the staleness section * fix(security): reference tracking issue bytedance#4044 in conscious-accept comment * style: compress conscious-accept comment to two lines
…m backend (bytedance#4122) * feat(memory): pluggable + self-contained memory system (MemoryManager plan phases 1 & 2) Phase 1 — Pluggable (steps 0-10): - ABC MemoryManager (9 methods) + singleton factory + drop-in backend discovery - DeerMem default backend with core/ (storage/queue/updater/prompt/message_processing) - NoopMemoryManager backend (proves pluggability) - All call sites (middleware/hook/prompt/gateway/client/app) routed through manager - hasattr capability probing for DeerMem-internal methods (no hard imports) - MemoryConfig gains manager_class field; shared vs DeerMem-private annotated Phase 2 — Self-contained DeerMem (steps 11-18): - backend_config passthrough + DeerMemConfig (all DeerMem-private fields moved off MemoryConfig) - DI: DeerMem owns storage/queue/updater/llm as instance attributes (no global singletons) - Storage independence: core/paths.py with own root (~/.deermem or ), factory auto-injects deer-flow's runtime_home() as absolute base_dir (zero-config) - LLM independence: core/llm.py via langchain init_chat_model (no create_chat_model) - Trace independence: optional tracing_callback replaces inject_langfuse_metadata/request_trace_context - Message processing independence: hide_from_ui default-skip + optional should_keep_hidden_message hook - Internal imports → relative (only deer_mem.py ABC import is host-relative) - Carrier (deer_mem.py adapter) / portable (deermem/ config+core) split - New tests: test_deermem_self_contained + test_memory_manager_pluggable; all memory tests migrated - Other-agent demo: samples/other_agent_demo/ + automated portability test - config.example.yaml memory section updated to phase-2 schema * feat(memory): port consolidation + staleness fix into self-contained DeerMem; phase-2 host hooks Port upstream bytedance#3996 (memory consolidation) and bytedance#3993 (staleness KeyError fix) from origin/MemoryManager into the pluggable, self-contained DeerMem structure (backends/deermem/deermem/), adapted to the DI MemoryUpdater (config injected, not get_memory_config globals): - DeerMemConfig: add consolidation_enabled (opt-in, default false) / consolidation_min_facts / consolidation_max_groups_per_cycle / consolidation_max_sources - prompt.py: factsToConsolidate JSON field + {consolidation_section} placeholder + CONSOLIDATION_PROMPT constant - updater.py: _coerce_source_confidence / _select_consolidation_candidates / _build_consolidation_section module helpers (matching the existing _select_stale_candidates style); consolidation normalization in _normalize_memory_update_data; consolidation apply in _apply_updates (after max_facts trim, with apply-time guardrails mirroring staleness); staleness KeyError fix (f["id"] -> f.get("id") is not None) applied to both the staleness guardrail and the consolidation allowed_source_ids comprehension - config.example.yaml: consolidation section under memory.backend_config - tests/test_memory_consolidation.py: 40 DI-adapted tests (running, not skipped) incl. the staleness KeyError regression Also includes in-flight phase-2 host-integration work: storage_path semantics (any absolute/relative value = root dir) and host-default tracing_callback / should_keep_hidden_message hooks injected into backend_config by the factory. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): add noop backend template and backends guide - backends/noop/: complete drop-in template (config.py with zero deer-flow imports, noop_manager.py with a 6-step new-backend walkthrough in its docstring, commented optional fact-CRUD capabilities). - backends/README.md: which files to touch when adding/swapping a backend, the 5-item backend contract, and common pitfalls. - manager.py: generalize backend examples in comments (drop mem0-specific references). Co-Authored-By: Claude <noreply@anthropic.com> * fix(frontend): guard formatTimeAgo against invalid timestamps Return a neutral placeholder when the input date is invalid (e.g. an empty lastUpdated from a backend with no memories) instead of throwing 'Invalid time value' from date-fns. Co-Authored-By: Claude <noreply@anthropic.com> * feat(memory): wire tool-driven memory mode through the MemoryManager ABC tools.py (memory_search/add/update/delete) now calls get_memory_manager() instead of the removed host memory module, so tool mode (memory.mode: tool) works for any backend. DeerMem.search is implemented (case-insensitive substring match, ranked by confidence) as a stand-in for the planned semantic retrieval; noop.search returns [] (unchanged). Fact-CRUD tools use getattr+callable probing -- backends lacking those ops (noop) get a clear JSON error instead of crashing. Tests: test_memory_tools rewired to mock the manager (handler tests) + TestModeGating retained; test_memory_search now covers DeerMem.search; pluggable stubs test updated (search no longer a stub). Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (import sorting, type annotation quotes, E402 in skipped tests) * docs: restore explanatory comments in config.example.yaml memory section * fix(security): port html-escape memory facts fix (bytedance#4097) to vendored DeerMem prompt.py * fix(memory): address review + port dropped upstream memory fixes Review blockers (vendored DeerMem): - bytedance#4044 restore _escape_memory_for_prompt (current_memory blob in MEMORY_UPDATE_PROMPT) - prevents </current_memory> breakout - bytedance#4028 html.escape staleness-section cat/content in _build_staleness_section - bytedance#4119 add _escape_summary for injection-path summaries (Work/Personal/ Current Focus/Recent/Earlier/Background) - default-model silent no-op: factory injects host default chat model via a new host_llm slot (create_chat_model(name=None)); DeerMem prefers host_llm over build_llm(model). Zero-config extraction works out of the box again - MemoryConfigResponse: fix stale docstring (backend-agnostic shape; DeerMem knobs live under backend_config, not top-level - restoring flat would re-couple the API to DeerMem). Frontend audited: does not read /memory/config - _host_default_tracing_callback: restore langfuse assistant_id/environment - search: push category onto the ABC signature; DeerMem filters BEFORE the top_k slice (was filtered client-side after slicing -> starved results) - _do_update_memory_sync: split into wrapper+impl; bind trace_id into the request-trace ContextVar on the Timer/executor worker via a new trace_context_manager host hook (None trace_id left unbound - no fabrication) - client.py fact-CRUD now passes user_id (was writing to the global bucket while get_memory reads per-user) - _resolve_manager_class: fail-fast (raise ValueError) on an unresolved explicit manager_class instead of silently falling back to DeerMem (memory is persistent state - a wrong store is a silent data-integrity footgun) Upstream memory fixes dropped by the host->vendored rename conflict, re-ported to backends/deermem/deermem/core/ (+ deer_mem.py): - bytedance#4073 queue busy-timer-spin -> _reprocess_pending flag (core/queue.py) - bytedance#4074 null source.confidence in staleness -> _coerce_source_confidence (core/updater.py: _build_staleness_section + _apply_updates stale sort) - bytedance#4075 factsToRemove is optional (drop from _REQUIRED_MEMORY_UPDATE_TOP_LEVEL_KEYS) - bytedance#4076 null confidence in search ranking -> _coerce_source_confidence (deer_mem.py DeerMem.search) host_llm + trace_context_manager are host-injected via backend_config (factory in manager.py), keeping backends/deermem/ at exactly one `from deerflow` line (the ABC contract) - portability test preserved. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve lint errors (F541 f-string without placeholders, E501 line too long) * fix(memory): restore hide_from_ui clarification preservation, expose mode Two memory-system fixes (F541/E501 lint was already fixed on this branch): - filter_messages_for_memory: restore default preservation of well-formed human_input_response clarification answers (v2 regression). The self-containment refactor made the bare function skip ALL hide_from_ui when no hook was passed, but upstream preserves well-formed clarification responses by default (test_hide_from_ui_human_input_response_is_preserved). Inline a host-agnostic _is_human_clarification_response mirror of read_human_input_response as the default keep-decision; the host-injected should_keep_hidden_message hook still overrides (production path unchanged). Portable package stays zero `from deerflow`. - /memory/config: expose `mode` (middleware|tool) in MemoryConfigResponse + the config/status endpoints + client.get_memory_config. mode is a host- shared, behavior-determining field missing from the response projection. Sync tests (mock .mode; e2e assert mode present). - Align manager_class field docstring with fail-fast behavior. Tests: filter/self-contained/portability (35) + memory-config (4) pass; ruff clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): resolve ruff format failures in memory module + tests `make lint` runs `ruff format --check` in addition to `ruff check`; 8 memory files had pending format changes -- 7 pre-existing (deer_mem, updater, tools, test_memory_queue/router/search/tools) + message_processing from the hide_from_ui fix. Apply `ruff format`: whitespace/wrapping only, no logic change. 109 memory tests pass; ruff check + format --check both clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - legacy field migration, fact_id contract, path/docs Address willem-bd's review on PR head bc8bf0d (risk:high, persistent state): - config: auto-migrate pre-abstraction top-level memory.* DeerMem fields (storage_path, max_facts, debounce_seconds, model_name, token_counting, staleness_*, consolidation_*) into backend_config on load + warn, so an upgrade does NOT silently revert customized settings (was: silent extra='ignore' drop). model_name -> backend_config.model.model. Unknown top-level keys warned. - factory: resolve a relative backend_config.storage_path against runtime_home() (base_dir-relative, CWD-independent) to preserve pre-abstraction semantics; paths.py stays portable (no runtime_home import). - tools: memory_add uses the fact_id returned directly by create_fact instead of re-deriving it via content-key matching (coupled the tool to the backend's content normalization; could misreport a storage cap). create_fact now returns (memory_data, fact_id); gateway/client/tool updated. Fix terse {"error":"content"} -> {"error":"empty content"}. - app.py: update stale token_counting=="char" warm-up comment to point at manager.warm (DeerMem.warm re-checks char and returns early). - router: comment explaining reload_memory silent fallback vs fact 501 asymmetry (read-only degrade vs write fail-loud). - CHANGELOG: document breaking changes (/memory/config + client.get_memory_config shape flat->backend_config; custom storage_class path moved + __init__ must accept config) and the legacy-field auto-migration. - tests: add regression test pinning the per-user memory path ({storage_path}/users/{safe_user_id}/memory.json == host make_safe_user_id) across the abstraction; update create_fact mocks for (memory_data, fact_id). Tests: 273 passed (memory suite); ruff check + format clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address PR review - storage_path, max_facts, tracing, parsing Six review findings (willem-bd), each verified against upstream: - storage_path semantics (file -> root dir): migration drops file-style (.json) legacy values with a warning; factory raises if storage_path resolves to an existing file (avoid silent NotADirectoryError write failure). CHANGELOG + config.example.yaml comment updated. - create_memory_fact enforces max_facts again (via _trim_facts_to_max) and returns (memory, None) when the cap evicts the new fact; memory_add tool reports "not stored", client raises ValueError, POST /memory/facts -> 409. - max_facts trim uses _coerce_source_confidence (was raw f.get("confidence", 0) -> TypeError on non-float imported/legacy confidence, swallowed as silent update failure). - memory-tracing assistant_id restored to "memory_agent" (was "lead-agent" copy-paste; matches upstream + DeerMem run_name). - _is_human_clarification_response cross-checked against read_human_input_response (drift guard test). - empty-string legacy values skipped silently in migration (narrow fix, not broad "if not value" which would skip explicit bool False). 8 new regression tests. make lint + 406 memory tests pass. Co-Authored-By: Claude <noreply@anthropic.com> * fix(memory): address internal review - storage fail-fast, build_llm degrade, config warn, noop template Addresses 4 findings from the PR bytedance#4122 internal supplemental review (parallel to willem-bd's review, no overlap): - create_storage fail-fast: a misspelled/unimportable storage_class now raises ValueError instead of silently falling back to FileMemoryStorage. Memory is persistent state, so a wrong store is a data-integrity footgun; mirrors the existing manager_class resolution policy. (storage.py) - noop template create_fact signature: the commented template used keyword-only `content` and returned a bare dict, while DeerMem's actual create_fact takes positional `content` and returns tuple[dict, str|None] (the memory_add tool passes content positionally; gateway/client/tools all tuple-unpack). A backend copied from the template would 500 on fact-CRUD. Template fixed; delete_fact/update_fact templates left (callers compatible). (noop_manager.py) - build_llm graceful degrade: wrap init_chat_model in try/except, degrade to None + WARNING on failure (mirroring _host_default_llm) so a misconfigured explicit model does not crash app startup -- non-LLM memory ops still work and an update raises at runtime with the error logged. (llm.py) - from_backend_config unknown-key warning: log a WARNING for unknown backend_config keys (mirrors the host layer's load_memory_config_from_dict) so a typo like `storage_pat` does not silently fall back to the default and write memory to an unintended location. (config.py) Tests: rewrote 3 create_storage fallback tests to expect ValueError; added 4 tests (build_llm zero-config/degrade, from_backend_config warn/silent). make lint green; full memory suite passes. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: lllyfff <2281215061@qq.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: lllyfff <122260771+lllyfff@users.noreply.github.com>
…ces (bytedance#4225) * fix(memory): consolidated facts inherit expected_valid_days from sources Consolidation (bytedance#3996) and per-fact expected_valid_days (bytedance#4143) were both authored by the same contributor but never connected: the consolidated new_fact carried the newest source's createdAt but no expected_valid_days, so _effective_fact_staleness_age fell back to the global staleness_age_days. A merge of several 200-day-old stable facts (each evd=3650) would land with no evd, read as a 90-day window, and re-enter the staleness candidate set on the very next cycle - the merge discarded the lifetime signal of the underlying information and contradicted consolidation's premise (these are stable, related facts worth synthesising). Fix: the merged fact inherits expected_valid_days set so it is re-reviewed at the EARLIEST source review deadline (min(createdAt + expected_valid_days) across sources, relative to the merged fact's createdAt = the newest source's). A merge combines details from every source, so a volatile sub-detail (evd=7) must not inherit a stable source's 3650-day window and escape staleness review for years - staleness KEEP/REMOVE is the only path that re-validates a merged fact, so biasing toward the soonest deadline keeps uncertain merges re-checked sooner. A source already past its deadline yields a minimal positive window (review next cycle) rather than the global fallback, which would defer an overdue review. Capped at the creation-time staleness_max_lifetime_multiplier like any new fact. Omitted when no source carries a valid evd (legacy facts fall back to the global age at read time, matching pre-feature behaviour). DRY: extract _read_expected_valid_days(fact) -> int | None, the shared type rule (int/float, reject bool, coerce to int BEFORE the > 0 guard) previously inlined in four places - _normalize_memory_update_fact, _effective_fact_staleness_age, the newFact creation cap, and consolidation inheritance. All four call the single helper. Coercing before the guard matters for values in (0, 1): 0.5 passes a raw > 0 check but truncates to 0, which would violate the helper's "positive int or None" contract; the order now matches the original _normalize_memory_update_fact rule. No prompt/schema change: consolidation's prompt does not surface source evd to the LLM, so asking the model to assign a merged lifetime would be guessing without signal. Source inheritance is deterministic and always available. Tests: consolidation evd cases now use time-stable createdAt (relative to now via a _days_ago helper) covering - earliest-deadline selection, creation-cap clamp, omit when no source evd, volatile source governs the deadline (and re-enters staleness next cycle), overdue source clamps to a minimal window, float coercion. Plus TestReadExpectedValidDays / TestEffectiveFactStalenessAge regression cases for the (0, 1) coercion-order fix. * fix(memory): reject non-finite expected_valid_days before int coercion The shared _read_expected_valid_days helper (introduced when consolidating the evd type rule across four call sites) coerces with int(raw) before the > 0 guard - reversing the original _normalize_memory_update_fact order so that a fractional 0.5 does not leak as 0. But int(raw) raises for non-finite floats: int(nan) raises ValueError and int(inf)/int(-inf) raise OverflowError. Python's JSON decoder accepts NaN / Infinity as floats by default, so a single malformed expected_valid_days in a hand-edited memory.json would abort staleness selection or consolidation instead of falling back to the global lifetime. On main, _effective_fact_staleness_age checked raw > 0 first, so NaN fell back safely (nan > 0 is false) - but inf did NOT (inf > 0 is true, so main also crashed on inf). This helper is now the persisted-fact read path for both staleness and consolidation, so the regression (and the pre-existing inf crash) must be closed here. Fix: require math.isfinite(float(raw)) before the int() coercion, then keep the existing positivity check and fallback. NaN / +/-inf all return None, so callers fall back to the global staleness_age_days. Normal int/float values (including large ones) are unaffected - isfinite is a no-op for them. Tests: - TestReadExpectedValidDays.test_rejects_non_finite_values - NaN, inf, -inf return None (not raise). - TestEffectiveFactStalenessAge.test_falls_back_for_non_finite_values - the persisted-fact read path returns the global age for each, no raise. - test_consolidation_with_non_finite_source_evd_does_not_raise - end-to-end: a NaN-evd source merged with a stable source does not abort consolidation; the NaN source's effective lifetime falls back to the global 90 and its deadline participates in the earliest-deadline computation. * test(memory): hoist _select_stale_candidates import + tidy deadline docstring Two review nits from the latest pass: - `_select_stale_candidates` was imported inline inside three test methods; hoisted to the module-level import block so the dependency is declared once. - `test_consolidated_evd_volatile_source_with_equal_created_at_future_deadline` had an abandoned calculation in its comment ("3 + 7 = 10 ... minus 3 already elapsed = 7? No:") that could mislead future readers into thinking elapsed-since-creation factors into the inherited window. Collapsed to a single clear line stating the window is relative to the merged createdAt, regardless of the source's current age. * fix(memory): reject huge-int expected_valid_days above timedelta.max.days _read_expected_valid_days routed every numeric value through float(raw) for the math.isfinite guard, but Python's JSON decoder parses an integer literal with no decimal point as an arbitrary-precision int (unlike 1e400, which decodes to float inf). So a hand-edited memory.json carrying "expected_valid_days": 10**400 raised OverflowError in float(raw) before math.isfinite was ever called - exactly the malformed-field-aborts-everything scenario the helper's docstring claims to prevent. The earlier non-finite fix only closed the float cases (NaN / +/-inf / 1e400). A huge int below the float limit but above timedelta.max.days (e.g. 10**12) would pass the helper and raise OverflowError downstream in timedelta(days=evd) during staleness selection or consolidation - the same crash fancyboi999 flagged for extend_by_days, just reached via a stored evd. Fix: branch on type so an int never passes through float() (matching the reviewer's suggestion), AND cap the returned int at timedelta.max.days (999999999) so the downstream timedelta(days=evd) call cannot overflow either. The float branch keeps the isfinite + int() coercion. Both branches share the 0 < evd <= timedelta.max.days positivity/range check. Normal values are unaffected - any legitimate expected_valid_days is far below the cap (the config ceiling staleness_max_extension_days tops out at 36500). Tests (all three layers): - TestReadExpectedValidDays.test_rejects_huge_int_above_timedelta_max - 10**400, 10**12, 10**9, timedelta.max.days+1 return None; timedelta.max.days itself is accepted. - TestEffectiveFactStalenessAge.test_falls_back_for_huge_int_above_timedelta_max - the persisted-fact read path returns the global age, no raise. - test_consolidation_with_huge_int_source_evd_does_not_raise[1e400|1e12|1e9] - parametrized end-to-end: a huge-int-evd source merged with a stable source does not abort consolidation; the bad source falls back to the global 90. * fix(memory): guard datetime arithmetic, not just timedelta construction The huge-int fix capped _read_expected_valid_days at timedelta.max.days, but that only proves timedelta(days=evd) can be constructed - adding it to a real fact timestamp still overflows datetime.max. @fancyboi999 reproduced it: a source with expected_valid_days=timedelta.max.days raises "OverflowError: date value out of range" at dt + timedelta(...) in the new consolidation deadline calculation. capping at timedelta.max.days was another patch chasing the next overflow boundary, not a real close. Root cause: the helper was doing datetime-range validation, but the safe bound depends on the datetime the evd is added to, not on the evd alone. So the responsibility moves to the arithmetic site, with try/except as the terminal guard - no concrete upper bound to be wrong about. Changes: - _read_expected_valid_days returns any positive int (huge ints included, not routed through float). Its job is type/positivity validation only. - New _safe_add_days(dt, days) -> datetime | None wraps dt + timedelta(days), returning None on OverflowError/ValueError. This is the terminal guard - there is no further boundary to overflow because try/except catches any datetime-range failure regardless of magnitude. - _select_stale_candidates uses _safe_add_days(now, -effective_age); a None result means the window is unrepresentably large, so the fact cannot yet be stale and is skipped (not selected). - Consolidation computes each source's deadline via _safe_add_days; a source whose deadline overflows falls back to the global staleness_age_days deadline (same treatment as a legacy no-evd source), so one malformed field cannot abort the merge. Normal values are unaffected - any legitimate expected_valid_days is far below the overflow boundary (the config ceiling staleness_max_extension_days tops out at 36500). Tests: - TestSafeAddDays: normal/negative shifts; 10**400/10**12/10**9 return None; timedelta.max.days (the exact reproduced value) returns None, not raises. - TestSelectStaleCandidates.test_huge_evd_does_not_abort_selection: a fact with a huge evd is skipped, not selected, and selection does not raise. - test_consolidation_with_huge_int_source_evd_does_not_raise now parametrized over [1e400, 1e12, 1e9, timedelta.max.days] - the last is the value that constructs a valid timedelta but overflows datetime arithmetic. - Helper/read-path tests updated to assert huge ints are returned as-is (the overflow guard is no longer in the helper).
Why
The memory system extracts facts atomically — each conversation adds individual entries like "uses React", "uses Python", "uses PostgreSQL". Over many sessions a single category can accumulate dozens of fragments that describe facets of the same picture. With
max_facts=100, fragmented low-value entries crowd out higher-value new facts and waste injection budget.This is the last piece of the memory lifecycle: extraction → guaranteed injection → staleness review → consolidation. The first three exist; consolidation lets the agent actively improve its own knowledge representation quality rather than only adding or removing facts.
What changed
consolidation_min_factsentries) during the normal memory-update LLM call — same invocation, zero extra API costconsolidation_max_groups_per_cyclegroups with at mostconsolidation_max_sourcesfacts each — aligned with apply-time guardrails so the LLM never sees groups it cannot act onfact_confidence_thresholdare rejected, double-consume protectionconsolidation_enabled(default:true)consolidation_enabled,consolidation_min_facts,consolidation_max_groups_per_cycle,consolidation_max_sourcesSurface area
Validation