mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-21 02:05:45 +00:00
main
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Tianye SongandGitHub
|
8511fa6aa3 |
fix(memory): consolidated facts inherit expected_valid_days from sources (#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). |
||
|
Tianye SongandGitHub
|
8da7cbf028 |
feat(memory): LLM-assigned per-fact expected_valid_days and staleFactsToExtend (#4143)
Re-ports this feature onto the pluggable-memory backend introduced in #4122 (the original #4143 was force-pushed clean by accident and auto-closed). The #4122 refactor moved the staleness logic into the self-contained DeerMem backend (backends/deermem/deermem/core/) and reverted it to the pre-feature global-threshold version, so the per-fact lifetime work is re-applied here against the new module layout + DI (MemoryUpdater is now (config, storage, llm)-injected; config lives on DeerMemConfig, not host MemoryConfig). **expected_valid_days (creation)** The LLM assigns a per-fact review window when storing each new fact. The prompt exposes five tiers (<=14 d transient -> >365 d very stable). The value is capped at write time by staleness_age_days x staleness_max_lifetime_multiplier (default 20.0 -> 1800 d ~= 5 years; range 1.0-100.0) so the model cannot set an initial lifetime so long the fact is never re-evaluated. The default 20.0 makes the "> 365 d very stable" tier achievable out of the box (3.0 silently clamped it to 270 d). **staleFactsToExtend (review)** During staleness review the LLM can emit extension entries for kept facts whose window seems miscalibrated. new_evd = min(days_since_created + extend_by_days, staleness_max_extension_days). Extensions use an absolute ceiling (default 3650 d ~= 10 years; range 90-36500) rather than the creation multiplier - they are deliberate review decisions that must be able to advance the window beyond the initial cap, but the absolute bound prevents timedelta overflow (a model-supplied extend_by_days of 10**9 previously crashed every later candidate-selection pass with OverflowError) and LLM misfire. **Invariant correctness** - Read-time cap removed from _effective_fact_staleness_age; cap is write-time only so extensions actually advance the review window. - proposed_remove_ids hoisted out of the removals sub-block and used to exclude from extension, so a cap-surviving proposed-removal fact is never extended. - extend_by coerced to int before the > 0 guard (a fractional 0.9 would pass the float check then int() to 0, silently writing a zero-delta extension). - days_since uses total_seconds() // 86400 (not .days truncation). - staleness-section html.escape uses quote=False to match the prompt.py convention; only <, >, & break element-text structure. **Tests** test_memory_staleness_review.py was module-level skipped by #4122 ("full unit-test migration is a follow-up"). This PR performs that migration: DI construction via (DeerMemConfig, _FakeStorage), _build_staleness_section back to the (candidates, config) signature, plus new coverage for per-fact selection, EXTEND with the absolute cap, the overflow next-cycle regression, the proposed-removal-not-extendable case, fractional extend_by skipping, and the creation-time cap. 67 tests, all green. |
||
|
Tianye SongandGitHub
|
54f3c43fe3 |
fix(security): html-escape fact content in memory prompt sections (#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 #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 |
||
|
|
9097642658 |
feat(memory): add memory consolidation to synthesize fragmented facts (#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 #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> |
||
|
|
28f2b07b79 |
feat(memory): add staleness review to prune silently-outdated facts (#3860)
* feat(memory): add staleness review to prune silently-outdated facts
Facts created long ago may become outdated without any future conversation
explicitly contradicting them ("Silent Staleness"). This adds a staleness
review mechanism that surfaces aged facts to the LLM during the normal
memory-update call so it can semantically judge whether each is still valid.
- New MemoryConfig fields: staleness_review_enabled, staleness_age_days,
staleness_min_candidates, staleness_max_removals_per_cycle,
staleness_protected_categories
- New STALENESS_REVIEW_PROMPT section injected into MEMORY_UPDATE_PROMPT
when enough stale candidates exist
- New staleFactsToRemove output field in the LLM response schema
- Safety cap limits max removals per cycle, keeping lowest-confidence
entries when the LLM returns more than the cap
- Correction facts (category=correction) are protected by default
- Observability via structured logging of each removal with reason
- 32 unit tests covering parsing, selection, triggers, formatting,
normalization, safety cap, and integration
* fix(memory): add deterministic guardrail for staleness removals
_apply_updates previously removed any fact id the LLM returned in
staleFactsToRemove without verifying it was in the actual staleness
candidate set. An LLM slip could silently delete protected-category
facts (e.g. correction) or fresh facts, defeating the stated guarantee.
Now intersect stale_ids_to_remove with _select_stale_candidates before
the safety cap, making the protection independent of both model behavior
and the staleness_review_enabled flag.
Add three regression tests:
- test_protected_category_fact_refused_at_apply
- test_non_aged_fact_refused_at_apply
- test_guardrail_runs_when_staleness_review_disabled
* docs(memory): sync AGENTS.md staleness config + simplify datetime parsing
Address reviewer feedback from PR #3860:
- Add staleness workflow step and 5 new config fields to backend/AGENTS.md
- Simplify _parse_fact_datetime: drop manual Z→+00:00 replace, Python 3.12+ fromisoformat handles Z natively
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
|
15454b6fec |
feat(skills): deferred skill discovery via describe_skill tool (#3775)
Replace the full-metadata <available_skills> system-prompt block with a compact <skill_index> (names only) and an on-demand describe_skill tool when skills.deferred_discovery: true (default: false / backward compat). New modules: - skills/catalog.py — SkillCatalog (immutable, searchable; select: has no cap, keyword/prefix search caps at MAX_RESULTS=5) - skills/describe.py — build_describe_skill_tool(catalog) closure; build_skill_search_setup() wires SkillSearchSetup into both the LangGraph agent factory (agent.py) and DeerFlowClient (client.py) Changes: - Skill @dataclass(frozen=True); allowed_tools/required_secrets list→tuple - Skill First prompt line gated on skill_names (deferred vs legacy wording) - get_skills_prompt_section: short-circuit storage on deferred path; merge user_id (upstream) + skill_names (this PR) params - describe_skill tool parameter named "name" (matches prompt wording) - select: branch removes [:MAX_RESULTS] cap (exact request, not ranking) - AGENTS.md: document deferred_discovery config field + new modules Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
b990da785f |
feat(memory): add guaranteed injection for correction facts with graceful fallback (#3592)
* feat(memory): add guaranteed injection for correction facts with graceful fallback When the token budget is tight, high-value facts (e.g. user corrections) can be silently evicted by lower-priority regular facts. This change: - Introduces configurable 'guaranteed_categories' (default: [correction]) whose facts draw from a separate 'guaranteed_token_budget', ensuring they are never dropped due to budget pressure. - Adds a graceful fallback to confidence-only ranking when the guaranteed-category path raises an unexpected exception. - Refactors fact selection into a header-agnostic helper (_select_fact_lines) with explicit token accounting in the caller, eliminating double-counting of separators. - Emits a single 'Facts:' header regardless of whether both guaranteed and regular facts are present. - Extends the final safety truncation limit to account for the additional guaranteed budget so guaranteed facts survive end-to-end. * refactor(memory): address review feedback on guaranteed injection - Restore strict break-on-overflow in `_select_fact_lines` to preserve the caller's confidence-ordered ranking; add a regression test locking in the invariant that a shorter lower-confidence fact never slips ahead of a skipped higher-confidence one. - Account for the inter-group `\n` separator between guaranteed and regular fact blocks in the regular budget (1-token precision fix). - Clarify docstrings on `format_memory_for_injection` and `MemoryConfig.guaranteed_token_budget` to distinguish the common *displacement* case (total stays within `max_tokens`) from the rarer *additive* case (safety-truncation ceiling raised when guaranteed lines alone would overflow). * fix(memory): address P1 safety truncation + P2s from review - Structure-aware safety truncation: Facts block is now a protected suffix so guaranteed-category facts can never be silently discarded by a prefix-cut on overflow. Only the preceding (user/history) sections are eligible for truncation. - Extend the same protected-suffix treatment to the except/fallback path by returning fact lines alongside the formatted section from _fallback_format_facts, avoiding string parsing. - Single inter-section separator: facts section no longer embeds its own leading \n\n; the final "\n\n".join(sections) is the single source of truth for section-to-section spacing. - Bare string for guaranteed_categories now raises TypeError instead of silently iterating single characters. - Category-less / malformed facts no longer default-promote into the guaranteed "context" pool — only facts with an explicit category field qualify. - Lift valid_facts pre-filter outside the try so the fallback path reuses it instead of re-doing validation work. - MemoryConfigResponse + DeerFlowClient.get_memory_config now expose guaranteed_categories / guaranteed_token_budget. - config.example.yaml: document the two new fields and bump config_version from 12 to 13. - Add regression tests for every finding. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |