Skip to content

feat(memory): add memory consolidation to synthesize fragmented facts - #3996

Merged
WillemJiang merged 6 commits into
bytedance:mainfrom
sontianye:feat/memory-consolidation
Jul 10, 2026
Merged

WillemJiang merged 6 commits into
bytedance:mainfrom
sontianye:feat/memory-consolidation

Conversation

@sontianye

Copy link
Copy Markdown
Contributor

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

  • Memory updater now detects fragmented categories (≥ consolidation_min_facts entries) during the normal memory-update LLM call — same invocation, zero extra API cost
  • The LLM reviews each group and decides whether related facts can be synthesized into a single richer fact (e.g., 5 tech-stack facts → 1 "full-stack engineer: React + Python + PostgreSQL")
  • Prompt surfaces the most fragmented categories first, capped at consolidation_max_groups_per_cycle groups with at most consolidation_max_sources facts each — aligned with apply-time guardrails so the LLM never sees groups it cannot act on
  • Apply-time guardrails: source IDs must exist and not overlap across groups, consolidated confidence capped at source maximum (prevents inflation), results below fact_confidence_threshold are rejected, double-consume protection
  • Feature-gated at both prompt and apply time via consolidation_enabled (default: true)
  • New config fields: consolidation_enabled, consolidation_min_facts, consolidation_max_groups_per_cycle, consolidation_max_sources
  • 26 tests covering candidate selection, normalization, apply guardrails, and prompt integration

Surface area

  • Agents / LangGraph — memory updater prompt + apply logic

Validation

cd backend && make lint && ruff format --check
cd backend && pytest tests/test_memory_consolidation.py tests/test_memory_staleness_review.py
# 61 passed, 0 failed

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
@github-actions github-actions Bot added area:agents Agents, subagents, graph wiring, prompts, langgraph.json area:backend Gateway / runtime / core backend under backend/ area:docs Documentation and Markdown only needs-validation Touches front/back contract surface; needs real-path validation risk:high High risk: backend API, agents, sandbox, auth, deps, CI size/XL PR changes 700+ lines labels Jul 8, 2026

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@WillemJiang WillemJiang added this to the 2.1.0 milestone Jul 8, 2026
@WillemJiang WillemJiang added the reviewing A maintainer is reviewing this PR label Jul 8, 2026
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.
@sontianye

Copy link
Copy Markdown
Contributor Author

Thank you for the thorough and detailed review — the findings are all spot-on!

All six issues have been addressed in the latest commit:

  1. Duplicate sourceIds — added dict.fromkeys dedup so ["f1","f1"] collapses to ["f1"] and is correctly rejected.
  2. Consolidation-vs-trim ordering — moved the consolidation block to run after the max_facts trim, so evicted source facts are absent from fact_index and rejected by the existence guardrail before any deletion happens.
  3. count= mismatch — changed to min(len(group), max_sources) so the LLM is only told about the IDs it can actually reference.
  4. Protected-category exemption — _select_consolidation_candidates now skips categories in staleness_protected_categories, honouring the same contract as the staleness feature.
  5. Category not stripped — added strip() + empty→"context" fallback in the normalization block, matching the pattern in _normalize_memory_update_fact.
  6. sourceError not propagated — collected sourceError from all source facts (deduped), and carried it into the consolidated fact.

Regression tests for all six cases have been added to test_memory_consolidation.py.

Thanks again for the careful read!

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 at updater.py:909-917.
  • Problem: Following up on the earlier protected-category point — _select_consolidation_candidates now excludes staleness_protected_categories at 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 full current_memory dump ({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_selected only asserts candidate selection, not apply, so the gap is untested.
  • Suggested fix: mirror staleness — build allowed_ids from _select_consolidation_candidates(current_memory, config) at apply time and reject any decision whose sourceIds aren't all in it; or at minimum reject sources whose fact_index[sid]["category"] is in staleness_protected_categories.
  • Test: feed _apply_updates a factsToConsolidate entry whose sourceIds include a correction fact and assert it is not merged (and the correction survives).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Two apply-time guardrails lack direct tests

  • Location: updater.py:1037 (confidence cap) and the if config.consolidation_enabled: apply gate (~line 996).
  • Problem: Two guardrails introduced by this PR have no test coverage:
    1. Confidence cap — fact_confidence = min(float(raw_llm_conf), max_source_conf) prevents consolidation from inflating confidence. No test asserts that an LLM returning confidence 1.0 over sources maxing at 0.8 is capped to 0.8 (nor that a sub-threshold capped result is rejected).
    2. 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 feeds factsToConsolidate into _apply_updates with the flag off and asserts nothing is merged.
  • 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 (factsToConsolidate present, consolidation_enabled=False -> no merge).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 — tests for confidence cap and apply-time disabled gate:

Both tests are now in TestReviewerFindings:

  1. test_confidence_cap_and_threshold_gate — asserts that an LLM returning confidence 1.0 over sources maxing at 0.75 is capped to 0.75; also asserts that when the capped value (0.65) falls below fact_confidence_threshold the merge is rejected and both source facts survive.

  2. test_apply_gate_consolidation_disabled — passes factsToConsolidate into _apply_updates with consolidation_enabled=False and 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 willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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. consolidatedFrom preserves 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 a risk:high PR — 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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 to 0.5, but this prompt formatter defaults it to 0.0. A fact stored with confidence: null is therefore shown to the LLM as 0.00 in the consolidation section yet treated as 0.5 when 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 another staleness_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 a consolidatedAt alongside createdAt).
  • Test: assert the consolidated createdAt policy is intentional (either now or newest-source) under a stale-source scenario.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

sontianye and others added 2 commits July 9, 2026 17:56
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 willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_enabled defaults to true, but the code / config.example.yaml / test_consolidation_enabled_defaults_to_false all default to false. 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: consolidatedFrom keeps 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.

Comment thread backend/AGENTS.md
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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 in staleness_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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. If a file stores a numeric createdAt (e.g. a unix timestamp), max([str, int]) raises TypeError and aborts the whole memory update.
  2. String comparison assumes a uniform Z suffix; a file mixing Z and +00:00 orders wrong lexicographically (same instant → the Z one 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small nits on the consolidated-confidence path:

  1. 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 to max_source_conf <= 1.0, but if that cap is ever loosened the divergence becomes a foot-gun.
  2. 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_confidence could subsume the latter two.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both points addressed:

  1. Out-of-range values — added max(0.0, min(float(raw_llm_conf), 1.0)) clamping before applying the source cap, so values like 1.5 are clipped to 1.0 first. Safe today via the cap, but now defensively correct if the cap is ever relaxed.

  2. Refactoring — kept the coercion inline rather than folding it into _coerce_source_confidence. The two contexts have genuinely different fallback semantics: source facts default to 0.5 (uncertain but plausible, and the system can't improve on that); the LLM's returned confidence defaults to max_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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}"')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@WillemJiang
WillemJiang merged commit 9097642 into bytedance:main Jul 10, 2026
28 of 32 checks passed
jtaynl added a commit to jtaynl/deer-flow that referenced this pull request Jul 10, 2026
…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>
WillemJiang pushed a commit that referenced this pull request Jul 11, 2026
)

* 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
lllyfff added a commit to qin-chenghan/deer-flow that referenced this pull request Jul 11, 2026
…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>
lllyfff added a commit to qin-chenghan/deer-flow that referenced this pull request Jul 11, 2026
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>
wangzhengxi pushed a commit to wangzhengxi/deer-flow that referenced this pull request Jul 14, 2026
…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>
WillemJiang pushed a commit that referenced this pull request Jul 15, 2026
…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>
sontianye added a commit to sontianye/deer-flow that referenced this pull request Jul 16, 2026
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.
sontianye added a commit to sontianye/deer-flow that referenced this pull request Jul 16, 2026
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.
WillemJiang pushed a commit that referenced this pull request Jul 21, 2026
…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).
marvin9551 pushed a commit to marvin9551/deer-flow that referenced this pull request Aug 21, 2026
…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>
marvin9551 pushed a commit to marvin9551/deer-flow that referenced this pull request Aug 21, 2026
…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
marvin9551 pushed a commit to marvin9551/deer-flow that referenced this pull request Aug 21, 2026
…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>
marvin9551 pushed a commit to marvin9551/deer-flow that referenced this pull request Aug 21, 2026
…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).
jihtsan pushed a commit to jihtsan/dnx-deer-flow that referenced this pull request Aug 29, 2026
…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>
jihtsan pushed a commit to jihtsan/dnx-deer-flow that referenced this pull request Aug 29, 2026
…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
jihtsan pushed a commit to jihtsan/dnx-deer-flow that referenced this pull request Aug 29, 2026
…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>
jihtsan pushed a commit to jihtsan/dnx-deer-flow that referenced this pull request Aug 29, 2026
…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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:agents Agents, subagents, graph wiring, prompts, langgraph.json area:backend Gateway / runtime / core backend under backend/ area:docs Documentation and Markdown only needs-validation Touches front/back contract surface; needs real-path validation reviewing A maintainer is reviewing this PR risk:high High risk: backend API, agents, sandbox, auth, deps, CI size/XL PR changes 700+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants