feat(console)!: restructure web console + retention-score forgetting model (0.2.0) - #43
Merged
Merged
Conversation
…model Reorganize the web console around the four memory-lifecycle stages — Manage / Activate / Consolidate / Forget — plus a System page for infrastructure config, with a unified design language, deep-linkable in-page tabs, and a lifecycle teardown registry. Migrate forgetting from the dynamic-TTL crossover to a retention-score (Ebbinghaus) model: retention(idle) = exp(-idle / eff_half_life), eff_half_life = half_life_days * (1 + k_importance*(importance/10) + k_access*(access_count/10)), forgotten when retention < threshold. Monotonic in importance and access, per-region defaults (30-180d half-lives), long-term timescales. The Forget page is reworked (retention decay curve + threshold line, importance x access matrix on a 0-100 axis with WCAG-contrast cell text, real-memory impact preview), hippocampus is hidden from the picker, and a new forgetting run tracker records each sweep. BREAKING CHANGE: the global base_ttl_hours / decay_factor settings and the per-partition override fields of the same name are removed, replaced by half_life_days / k_importance / k_access / forget_threshold / forget_min_retention_days (global) and half_life_days / k_importance / k_access / threshold (per-partition). Legacy hebb.json keys are ignored; per-partition overrides fall back to the region/global defaults. Docs (EN+ZH), CHANGELOG, and tests updated; JS<->Python forgetting mirror verified exact; 0.1.8 -> 0.2.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughVersion 0.2.0 replaces the TTL-based forgetting model with an Ebbinghaus retention-score model ( ChangesRetention-Score Forgetting Model & Per-Partition API
Web Console Restructure
Documentation, Design Reports & Version Bump
Sequence DiagramsequenceDiagram
participant Browser
participant app.js
participant forgetting.js
participant forgetting-math.js
participant ForgettingRouter
participant forgetting_job.py
participant ForgettingTracker
Browser->>app.js: navigate `#forget`
app.js->>app.js: runCleanups()
app.js->>forgetting.js: renderForget(root)
forgetting.js->>ForgettingRouter: GET /forgetting (config)
ForgettingRouter->>forgetting_job.py: resolve_forgetting_params per partition
ForgettingRouter-->>forgetting.js: ForgettingConfigResponse
forgetting.js->>forgetting-math.js: buildCurve(params)
forgetting.js->>forgetting-math.js: buildMatrix(params)
forgetting-math.js-->>forgetting.js: SVG curve + matrix data
Browser->>forgetting.js: adjust slider (debounced)
forgetting.js->>ForgettingRouter: POST /forgetting/{id}/preview
ForgettingRouter-->>forgetting.js: would_forget / would_keep
Browser->>forgetting.js: click "Save override"
forgetting.js->>ForgettingRouter: PUT /forgetting/{id}
ForgettingRouter-->>forgetting.js: PartitionForgettingEntry updated
Browser->>forgetting.js: click "Clean up now"
forgetting.js->>ForgettingRouter: POST /admin/forget
ForgettingRouter->>forgetting_job.py: per-partition sweep
ForgettingRouter->>ForgettingTracker: record_run(trigger=manual)
ForgettingRouter-->>forgetting.js: deleted count
forgetting.js->>ForgettingRouter: GET /forgetting/runs
ForgettingRouter->>ForgettingTracker: list_runs()
ForgettingRouter-->>forgetting.js: ForgettingRunsResponse
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the version to 0.2.0 and introduces a major overhaul of the forgetting mechanism, replacing the legacy dynamic-TTL model with an Ebbinghaus-inspired retention-score model. It adds per-partition forgetting overrides stored in the configuration, introduces a lightweight forgetting run tracker, and restructures the web console around the four memory lifecycle stages (Manage, Activate, Consolidate, and Forget). Additionally, it adds comprehensive unit and integration tests, including a command-drift guard for documentation. The review feedback correctly identifies two critical platform-specific issues where files are opened or written without specifying UTF-8 encoding, which could cause decoding failures on Windows when handling non-ASCII characters.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| raise FileNotFoundError(f"Config file not found: {path}") | ||
|
|
||
| with _ConfigLock(path): | ||
| with open(path) as f: |
There was a problem hiding this comment.
On Windows and other platforms where the default system encoding is not UTF-8, opening the configuration file without specifying an encoding can lead to UnicodeDecodeError if the file contains non-ASCII characters (such as Chinese descriptions or partition names). Please specify encoding="utf-8" when opening the file.
Suggested change
| with open(path) as f: | |
| with open(path, encoding="utf-8") as f: |
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| fd, tmp_name = tempfile.mkstemp(dir=str(path.parent), prefix=".manifest.", suffix=".tmp") | ||
| try: | ||
| with os.fdopen(fd, "w") as f: |
There was a problem hiding this comment.
When writing the forgetting run manifest, os.fdopen is used without specifying an encoding. On systems where the default encoding is not UTF-8 (e.g., Windows), this can write the file in a different encoding (like CP1252). Since _load_manifest reads the file using encoding="utf-8", this mismatch will cause a UnicodeDecodeError and corrupt the run history if any error messages contain non-ASCII characters. Please specify encoding="utf-8" in os.fdopen.
Suggested change
| with os.fdopen(fd, "w") as f: | |
| with os.fdopen(fd, "w", encoding="utf-8") as f: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hebb/static/js/components/graph.js (1)
163-169: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear auto-pause timers on unmount to avoid cross-instance interval kills.
setTimeoutcallbacks are not tracked/cleared, but they callclearInterval(fa2Interval)on a module-scoped variable. After remount, an old timeout can stop the new layout loop unexpectedly.🔧 Proposed fix
const FA2 = globalThis.ForceAtlas2; let layoutRunning = true; + let autoPauseTimer = null; + + function scheduleAutoPause() { + if (autoPauseTimer) clearTimeout(autoPauseTimer); + autoPauseTimer = setTimeout(() => { + if (layoutRunning) { + layoutRunning = false; + clearInterval(fa2Interval); fa2Interval = null; + layoutBtn.textContent = t('graph.resume_layout'); + } + }, 10000); + } for (let i = 0; i < 50; i++) runFA2Step(); renderer.getCamera().animatedReset(); fa2Interval = setInterval(runFA2Step, 50); - setTimeout(() => { - if (layoutRunning) { - layoutRunning = false; - clearInterval(fa2Interval); fa2Interval = null; - layoutBtn.textContent = t('graph.resume_layout'); - } - }, 10000); + scheduleAutoPause(); layoutBtn.addEventListener('click', () => { if (layoutRunning) { layoutRunning = false; clearInterval(fa2Interval); fa2Interval = null; layoutBtn.textContent = t('graph.resume_layout'); } else { layoutRunning = true; fa2Interval = setInterval(runFA2Step, 50); layoutBtn.textContent = t('graph.pause_layout'); - setTimeout(() => { - if (layoutRunning) { - layoutRunning = false; - clearInterval(fa2Interval); fa2Interval = null; - layoutBtn.textContent = t('graph.resume_layout'); - } - }, 10000); + scheduleAutoPause(); } }); onCleanup(() => { + if (autoPauseTimer) { clearTimeout(autoPauseTimer); autoPauseTimer = null; } observer.disconnect(); if (renderer) { renderer.kill(); renderer = null; } if (fa2Interval) { clearInterval(fa2Interval); fa2Interval = null; } });Also applies to: 180-187, 390-394
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/graph.js` around lines 163 - 169, The setTimeout callbacks are not being tracked and cleared, but they reference the module-scoped variable fa2Interval and call clearInterval on it. When the component remounts, old timeouts from previous instances can still fire and unexpectedly stop the new layout loop. Store the setTimeout references in tracked variables (similar to how fa2Interval is tracked), then ensure these timeout IDs are cleared during cleanup or unmount to prevent stale timeouts from interfering with new layout instances. Apply this fix to all setTimeout calls that interact with fa2Interval throughout the component.src/hebb/static/js/components/system.js (1)
279-286: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not persist masked LLM key placeholders on save.
The LLM save path writes any non-empty
llm_api_key, but the embedding save path already guards against masked placeholders (****). Mirror that guard here to avoid replacing a real secret with masked text.🔧 Proposed fix
saveBtn.addEventListener('click', async () => { try { const model = modelInput.value.trim(); const base_url = urlInput.value.trim(); const api_key = keyInput.value.trim(); if (model) await api.updateConfig('llm_model', model); await api.updateConfig('llm_base_url', base_url || 'null'); - if (api_key) await api.updateConfig('llm_api_key', api_key); + if (api_key && !api_key.includes('****')) { + await api.updateConfig('llm_api_key', api_key); + } success(t('system.toast.llm_saved'));Also applies to: 804-807
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/system.js` around lines 279 - 286, The llm_api_key configuration is being saved without checking if it contains a masked placeholder value like the embedding save path already does. In the updateConfig call for llm_api_key (around line 285), add a guard condition to check that api_key is not equal to a masked placeholder (such as '****') before persisting it, similar to how the embedding save path handles this. This same guard should also be applied at lines 804-807 where similar configuration updates occur.
🟡 Minor comments (12)
reports/design/retention-forgetting-model-2026-06-22.md-110-110 (1)
110-110: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the malformed table row.
mem_hippocampusonly provides 2 cells here, so the table won’t render cleanly. Add the missing placeholder columns or move that note outside the table.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reports/design/retention-forgetting-model-2026-06-22.md` at line 110, The table row for mem_hippocampus has fewer columns than the other rows in the table, causing rendering issues. Either add placeholder cells (using | —) to match the column count of the surrounding table rows, or remove the mem_hippocampus row entirely and add the note about it never being swept and being drained by consolidation as separate text outside the table.Source: Linters/SAST tools
repo_pages/concepts/forgetting.md-148-152 (1)
148-152: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTag the example formula blocks too.
Both scenario snippets are missing a fence language, so markdownlint will keep warning until these openings are labeled.
Suggested fix
-``` +```text eff_half_life = 90 * (1 + 3*(8/10) + 1.5*(10/10)) = 90 * 4.9 = 441 days retention(2) = exp(-2 / 441) = 0.995 (≫ 0.3 threshold) forget at = 441 * ln(1/0.3) ≈ 531 days</details> Also applies to: 160-164 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@repo_pages/concepts/forgetting.mdaround lines 148 - 152, The markdown code
fence blocks for the formula calculation examples are missing language tags,
which causes markdownlint warnings. Add the language identifier "text" to the
opening fence markers (changetotext) for both code blocks containing
the formula calculations with eff_half_life, retention(2), and forget at values.
This needs to be applied to two separate example blocks in the file.</details> <!-- cr-comment:v1:edf07a42818b57c6ca62c4d1 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>repo_pages/concepts/memory-lifecycle.md-103-109 (1)</summary><blockquote> `103-109`: _📐 Maintainability & Code Quality_ | _🟡 Minor_ | _⚡ Quick win_ **Tag this formula fence too.** Markdownlint flags this block for the same reason: no language tag on the opening fence. `text` is enough if you don't want highlighting. <details> <summary>Suggested fix</summary> ```diff -``` +```text eff_half_life = half_life_days * (1 + k_importance*(importance/10) + k_access*(access_count/10)) retention(idle) = exp(-idle_days / eff_half_life) forget when retention < threshold ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@repo_pages/concepts/memory-lifecycle.md` around lines 103 - 109, The formula fence block displaying the effective half-life calculation (eff_half_life, retention, and forget condition) is missing a language tag on the opening triple backticks, which causes markdownlint to flag it. Add the language identifier `text` immediately after the opening triple backticks (before the line break) to properly tag this code fence, just like the suggested fix shows, so the block complies with markdown linting standards. ``` </details> <!-- cr-comment:v1:0b374a329a2a9b935782b04b --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>repo_pages/zh/guide/web-console.md-66-68 (1)</summary><blockquote> `66-68`: _🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_ **Update this to the new retention terminology.** `基础 TTL、衰减` is stale in the v0.2.0 docs. This should match the half-life / threshold model used elsewhere so readers don't carry over the old config shape. <details> <summary>Suggested fix</summary> ```diff -下方是**遗忘运行记录**、**全局遗忘默认值**（基础 TTL、衰减、扫描间隔），以及一个**按分区遗忘调参器** +下方是**遗忘运行记录**、**全局遗忘默认值**（half_life_days、k_importance、k_access、forget_threshold、forget_min_retention_days、扫描间隔），以及一个**按分区遗忘调参器** ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@repo_pages/zh/guide/web-console.md` around lines 66 - 68, Update the retention configuration terminology in the "记忆遗忘（`#forget`）" section to align with v0.2.0 documentation standards. Replace the outdated "基础 TTL、衰减" language with the new "half-life / threshold" model terminology to ensure consistency with the rest of the documentation and prevent readers from applying deprecated configuration patterns. Review other relevant sections in the documentation to understand the exact terminology used for the new model and apply it consistently throughout this section. ``` </details> <!-- cr-comment:v1:febd05ada3ecf2fea3791bc7 --> </blockquote></details> <details> <summary>repo_pages/concepts/forgetting.md-11-15 (1)</summary><blockquote> `11-15`: _📐 Maintainability & Code Quality_ | _🟡 Minor_ | _⚡ Quick win_ **Add a language tag to this formula fence.** Markdownlint is flagging the untyped fence here. `text` is enough if you don't want syntax highlighting. <details> <summary>Suggested fix</summary> ```diff -``` +```text eff_half_life = half_life_days * (1 + k_importance * (importance / 10) + k_access * (access_count / 10)) retention(idle_days) = exp(-idle_days / eff_half_life) forget when retention < threshold ⇔ idle_days > eff_half_life * ln(1 / threshold) ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@repo_pages/concepts/forgetting.mdaround lines 11 - 15, The markdown code
fence containing the formulas for eff_half_life, retention, and forget threshold
is missing a language tag, which is causing markdownlint to flag it. Add the
language tagtextto the opening backticks of this code fence (change ``` toand plain text is sufficient.Source: Linters/SAST tools
repo_pages/zh/concepts/forgetting.md-11-15 (1)
11-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language tags to the fenced examples.
These bare fences trigger MD040 in markdownlint. Please label them consistently as
textso the docs stay lint-clean.Suggested fix
-``` +```textAlso applies to: 148-152, 160-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@repo_pages/zh/concepts/forgetting.md` around lines 11 - 15, Add language tags to all bare fenced code blocks in the file to comply with markdownlint MD040 rules. For each code fence that currently has no language identifier after the triple backticks (at lines 11-15, 148-152, and 160-164), append the word `text` immediately after the opening triple backticks. This will label all mathematical formulas and similar content blocks consistently as plain text examples and eliminate the linting violations.Source: Linters/SAST tools
repo_pages/guide/web-console.md-66-69 (1)
66-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReplace the legacy TTL wording here.
This paragraph still describes the Forget page defaults in
base TTL/decayterms, which no longer matches the retention-score model exposed by the new console and config docs.Suggested fix
- the **global forgetting defaults** (base TTL, decay, sweep interval), + the **global forgetting defaults** (half-life, importance/access weights, threshold, minimum retained lifetime, sweep interval),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@repo_pages/guide/web-console.md` around lines 66 - 69, The Forget (`#forget`) section still uses legacy terminology with references to "base TTL" and "decay" which does not align with the new retention-score model used in the current console and config documentation. Replace the outdated TTL and decay terminology in this paragraph with appropriate retention-score model terminology to accurately describe how the forgetting defaults and per-partition tuning work in the new console implementation.src/hebb/static/js/components/memories.js-226-229 (1)
226-229: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle create-modal partition fetch failures.
Lines 226-229 can reject without user feedback if
api.listPartitions()fails. Wrap this handler intry/catchand toast the error.🔧 Proposed fix
root.querySelector('`#btn-create`').onclick = async () => { - const partitions = await api.listPartitions(); - showCreateModal(root, partitions); + try { + const partitions = await api.listPartitions(); + showCreateModal(root, partitions); + } catch (e) { + error(e.message); + } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/memories.js` around lines 226 - 229, The async onclick handler for root.querySelector('`#btn-create`') lacks error handling for the api.listPartitions() call, which means if the API request fails, the user receives no feedback. Wrap the api.listPartitions() await call in a try/catch block, keeping the showCreateModal() call in the try block and adding error handling in the catch block that toasts the error message to inform the user of the failure.src/hebb/static/js/app.js-103-107 (1)
103-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winResync
currentSubfromlocation.hashbefore language rerender.Line [107] rerenders with cached
currentSub, but sub-tabs are updated viahistory.replaceState(...)in page modules (nohashchange), socurrentSubcan be stale and restore the wrong tab on language switch.Proposed fix
function applyLang(lang) { setLang(lang); langLabel.textContent = lang.toUpperCase(); updateNavLabels(); // Re-render current page (and its active sub-tab) to apply translations if (currentPage && pages[currentPage]) { + const raw = location.hash.replace(/^`#/`, ''); + const [hashPage, hashSub] = raw.split('/'); + if (pages[hashPage]) { + currentPage = hashPage; + currentSub = hashSub || null; + } runCleanups(); content.innerHTML = ''; pages[currentPage](content, currentSub); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/app.js` around lines 103 - 107, The rerender logic at the pages[currentPage] call is using a potentially stale currentSub variable because sub-tabs are updated via history.replaceState() which does not trigger hashchange events, leaving currentSub out of sync with location.hash. Before calling pages[currentPage](content, currentSub), extract the current sub-tab identifier from location.hash to ensure currentSub is resynchronized with the actual current state before rerendering the page content.src/hebb/static/js/components/forgetting.js-437-439 (1)
437-439: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the access-count table header.
The hardcoded
accstring bypasses i18n and leaks English text in non-English UI.Suggested patch
- <th>acc</th><th>${t('forgetting.col_age')}</th><th></th></tr></thead><tbody>`; + <th>${t('forgetting.access_count')}</th><th>${t('forgetting.col_age')}</th><th></th></tr></thead><tbody>`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/forgetting.js` around lines 437 - 439, The table header in the forgetting impact table contains a hardcoded acc string that is not localized, while all other column headers use the i18n translation function t(). Replace the hardcoded acc string in the table header row with a properly localized translation call using t() function with an appropriate key (such as t('forgetting.col_access_count') or similar) to match the pattern of the other headers like col_content, importance, and col_age.src/hebb/static/css/style.css-340-343 (1)
340-343: 📐 Maintainability & Code Quality | 🟡 MinorReplace deprecated
word-break: break-wordwith standards-compliantoverflow-wrap: anywhere.The
word-break: break-wordproperty is deprecated per CSS standards (MDN, W3C). Useoverflow-wrap: anywhereinstead for better standards compliance and consistent behavior across browsers.Suggested patch
.mem-card-body { font-size: 13.5px; line-height: 1.65; color: var(--text-primary); - white-space: pre-wrap; word-break: break-word; + white-space: pre-wrap; + overflow-wrap: anywhere; + word-break: normal; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/css/style.css` around lines 340 - 343, In the .mem-card-body CSS class, replace the deprecated property `word-break: break-word` with the standards-compliant `overflow-wrap: anywhere`. This ensures better compatibility and aligns with current CSS standards. Remove the word-break line and add overflow-wrap with the value anywhere to maintain the same text wrapping behavior while using a non-deprecated approach.Source: Linters/SAST tools
src/hebb/retrieval/searcher.py-31-36 (1)
31-36: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the half-life math note in the recency comment.
Line 31 currently states that
score = decay ** hourswith0.693is “≈ a 1-day half-life”, but that base yields ~1.9-hour half-life. Please align either the formula text or the half-life statement to avoid mis-tuning later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/retrieval/searcher.py` around lines 31 - 36, The comment for the recency decay base constant contains a mathematical inconsistency: it states that decay=0.693 yields approximately a 1-day half-life in the formula score=decay**hours, but this base actually produces approximately a 1.9-hour half-life. Correct this discrepancy by either updating the decay value to achieve the intended 1-day half-life (approximately 0.967 for 24-hour half-life) or correcting the stated half-life duration in the comment to reflect what 0.693 actually produces. Ensure the comment accurately documents the recency weighting behavior so future developers tune it correctly.
🧹 Nitpick comments (1)
src/hebb/server/routers/admin.py (1)
176-187: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse batched expiry persistence in manual sweep for parity with scheduler performance.
This endpoint still writes expiry one row at a time. On large partitions that recreates the O(N) write/commit pressure the scheduler path already avoided.
Suggested refactor
+ expiry_updates: list[tuple[str, str]] = [] ... for memory in memories: expires_at = compute_expires_at(...) - await memory_store.update_expiry(memory.id, expires_at.isoformat()) if expires_at < now: - await purge_memory(memory_store, kg, memory.id, save=False) - deleted += 1 + to_delete.append(memory.id) + else: + expiry_updates.append((memory.id, expires_at.isoformat())) + + if expiry_updates: + batch = getattr(memory_store, "update_expiry_batch", None) + if callable(batch): + await batch(expiry_updates) + else: + for mid, iso in expiry_updates: + await memory_store.update_expiry(mid, iso)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/server/routers/admin.py` around lines 176 - 187, The current implementation in the admin router calls update_expiry one memory at a time within the for loop over memories, creating O(N) individual database writes. Instead of calling await memory_store.update_expiry for each memory individually, collect all the expiry updates (pairing each memory.id with its computed expires_at) into a single data structure and make a single batched update call to memory_store, similar to how the scheduler path handles this, to avoid the write/commit pressure of individual row updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@reports/design/per-partition-forgetting-ui-2026-06-22.md`:
- Around line 23-25: The design document describes per-partition parameters
using the old TTL model (base_ttl_hours and decay_factor), but the actual live
API contract at /api/v1/admin/forgetting exposes different retention model
parameters (half_life_days, k_importance, k_access, threshold, and enabled).
Update all references to the old parameter names throughout the document
(including sections at lines 23-25, 32-33, 53-63, and 78-88) to use the current
retention model field names so the UI specification matches the actual contract
the frontend will consume.
In `@reports/design/web-console-restructure-2026-06-22.md`:
- Line 25: Update the Forget feature description to replace the outdated
retention field names with the new retention model fields. In the table row
describing the forget functionality, replace the references to `base TTL`,
`decay`, and `sweep interval` with the correct field names: `half_life_days`,
`k_importance`, `k_access`, `threshold`, and per-partition overrides. This
applies to all mentions of these fields throughout the document (lines 25,
30-32) to ensure consistency with the current admin UI contract for the
retention model.
In `@src/hebb/scheduler/forgetting_job.py`:
- Around line 117-119: The `compute_expires_at` function can raise an
`OverflowError` when `idle` from `forget_idle_days` is too large to fit in a
timedelta. Instead of using fractional seconds for capping, implement a
try/except block around the final return statement that catches `OverflowError`
and returns `datetime.max` as a safe fallback, or alternatively cap the `idle`
value to an integer-day limit (such as timedelta's maximum days of 999999999)
before creating the timedelta from `memory.last_accessed_at +
timedelta(days=idle)` to prevent the overflow from occurring in the first place.
In `@src/hebb/server/routers/admin.py`:
- Around line 189-193: The manual purge operation in the `/forget` endpoint
mutates and saves the knowledge graph without acquiring the shared graph lock,
creating a race condition with concurrent scheduler writes. Wrap the critical
section containing the purge_memory calls and the subsequent kg.save() operation
with the knowledge_graph.lock context manager (using async with) to ensure
mutual exclusion. This mirrors the locking pattern used in the scheduler path
and prevents interleaved graph writes from persisting inconsistent state.
In `@src/hebb/server/routers/forgetting.py`:
- Around line 234-236: The code at lines 234-236 and 252-253 reconstructs the
full forgetting_overrides map from an in-memory snapshot of
settings.forgetting_overrides and then persists it entirely via
_persist_and_sync, creating a lost-update vulnerability where concurrent
requests overwriting different partitions can lose changes. Replace this
full-map rebuild approach with an atomic per-partition mutation pattern: read
the current on-disk map while holding a lock, apply only the single partition
key's add or remove operation (for the set path at line 234-236 and the clear
path at lines 252-253), validate the change, and write back only the updated
map. This ensures that each concurrent modification updates from the current
disk state rather than a potentially stale in-memory snapshot.
In `@src/hebb/static/js/app.js`:
- Around line 46-57: The navigate function invokes the page renderer via
pages[page](content, currentSub) without any mechanism to cancel or guard
against stale async operations from the previous page. To fix this, introduce a
navigation epoch or cancellation token (such as a simple counter variable that
increments on each navigate call), pass this token to each page renderer, and
ensure that any async operations within those renderers (like the
api.getConfig() call mentioned) check whether the navigation token is still
current before mutating the content element. This prevents stale renders from
previous pages from modifying the DOM after navigation has already cleared and
reset the view.
In `@src/hebb/static/js/components/consolidate.js`:
- Around line 300-301: The catch block at line 301 is directly injecting the
error message into innerHTML using string interpolation, which creates an XSS
vulnerability since error messages can contain untrusted content. Replace the
innerHTML assignment with a safer approach: either create a div element using
createElement, set the error message using textContent property (which treats
content as plain text rather than HTML), and append it to configRoot, or
alternatively use textContent to set the div content directly without HTML
parsing.
In `@src/hebb/static/js/components/forget.js`:
- Around line 192-193: The catch block at the end of the file is inserting
e.message directly into globalRoot.innerHTML without escaping, creating an XSS
vulnerability if the error contains malicious HTML. Instead of using e.message
raw in the template literal, escape or sanitize the error message before
injecting it into the innerHTML string. You can use a utility function like
textContent assignment or an HTML escaping function to safely convert the error
message to plain text before embedding it in the HTML template.
In `@src/hebb/static/js/components/forgetting.js`:
- Around line 463-469: The refreshImpact function makes async preview API calls
that can overlap when rapid changes occur, allowing older responses to overwrite
newer ones. Guard the renderImpact call by implementing a request tracking
mechanism such as a counter or timestamp that increments before each
api.previewForgetting call, then verify that the completed response corresponds
to the most recent request before calling renderImpact to ensure only the latest
preview response gets rendered.
In `@src/hebb/static/js/components/memories.js`:
- Around line 31-35: The loadList function caches DOM references at the start
(list, info, pag elements) but these can become detached if the user switches
tabs while the api.listMemories await is pending on line 39. Add a mount check
before the await to verify the root element is still in the document, and add
another mount check immediately after the await completes before performing any
DOM operations (innerHTML updates, event listeners, etc.). This ensures that
stale DOM references are not manipulated after tab unmounting. Apply the same
guard pattern to the other async operations mentioned at lines 72-74.
In `@src/hebb/static/js/components/partitions.js`:
- Around line 34-40: The loadList function has a race condition where
api.listPartitions() can resolve after the DOM has changed (tab replaced),
causing stale updates to the container element. After awaiting the
api.listPartitions() call, add a check to verify that the container element is
still present in the root element before proceeding with innerHTML updates and
handler binding. This check should validate that the container reference is
still valid in the DOM to prevent mutations on detached or replaced elements.
In `@src/hebb/static/js/components/system.js`:
- Around line 89-91: The error message from the catch block is being directly
injected into innerHTML without proper escaping, which creates a potential XSS
vulnerability. In the catch block where
root.querySelector('`#system-panel`').innerHTML is set with e.message, escape the
error message text before inserting it into the DOM. You can create a helper
function or use a method like creating a temporary text element and extracting
its textContent, or use a library utility for HTML escaping, to ensure that any
HTML special characters in the error message are properly encoded before being
included in the innerHTML assignment.
In `@tests/integration/server/test_forgetting_router.py`:
- Around line 44-49: The patch target on line 45 is incorrect for the
`create_app` function's usage. Instead of patching
`hebb.embedding.factory.create_embedder`, patch the symbol where it is imported
and used within the `hebb.server.app` module. Change the patch target to
`hebb.server.app.create_embedder` to ensure the patch intercepts the actual
reference used by the `create_app` function, preventing issues with cached
module-level imports from earlier test runs.
In `@tests/unit/test_audit_distribution.py`:
- Around line 195-211: The _doc_files() function uses check=True in the
subprocess.run() call for the git ls-files command, which causes it to raise an
exception and hard-fail when Git is unavailable (such as in sdist/exported trees
without .git or environments without git installed). Remove the check=True
parameter from the subprocess.run() call and instead check the returncode after
execution. If the git command fails (returncode is non-zero), return an empty
list instead of raising an exception, allowing the test to continue with its
actual drift assertions.
---
Outside diff comments:
In `@src/hebb/static/js/components/graph.js`:
- Around line 163-169: The setTimeout callbacks are not being tracked and
cleared, but they reference the module-scoped variable fa2Interval and call
clearInterval on it. When the component remounts, old timeouts from previous
instances can still fire and unexpectedly stop the new layout loop. Store the
setTimeout references in tracked variables (similar to how fa2Interval is
tracked), then ensure these timeout IDs are cleared during cleanup or unmount to
prevent stale timeouts from interfering with new layout instances. Apply this
fix to all setTimeout calls that interact with fa2Interval throughout the
component.
In `@src/hebb/static/js/components/system.js`:
- Around line 279-286: The llm_api_key configuration is being saved without
checking if it contains a masked placeholder value like the embedding save path
already does. In the updateConfig call for llm_api_key (around line 285), add a
guard condition to check that api_key is not equal to a masked placeholder (such
as '****') before persisting it, similar to how the embedding save path handles
this. This same guard should also be applied at lines 804-807 where similar
configuration updates occur.
---
Minor comments:
In `@repo_pages/concepts/forgetting.md`:
- Around line 148-152: The markdown code fence blocks for the formula
calculation examples are missing language tags, which causes markdownlint
warnings. Add the language identifier "text" to the opening fence markers
(change ``` to ```text) for both code blocks containing the formula calculations
with eff_half_life, retention(2), and forget at values. This needs to be applied
to two separate example blocks in the file.
- Around line 11-15: The markdown code fence containing the formulas for
eff_half_life, retention, and forget threshold is missing a language tag, which
is causing markdownlint to flag it. Add the language tag `text` to the opening
backticks of this code fence (change ``` to ```text) since these mathematical
formulas do not require syntax highlighting and plain text is sufficient.
In `@repo_pages/concepts/memory-lifecycle.md`:
- Around line 103-109: The formula fence block displaying the effective
half-life calculation (eff_half_life, retention, and forget condition) is
missing a language tag on the opening triple backticks, which causes
markdownlint to flag it. Add the language identifier `text` immediately after
the opening triple backticks (before the line break) to properly tag this code
fence, just like the suggested fix shows, so the block complies with markdown
linting standards.
In `@repo_pages/guide/web-console.md`:
- Around line 66-69: The Forget (`#forget`) section still uses legacy
terminology with references to "base TTL" and "decay" which does not align with
the new retention-score model used in the current console and config
documentation. Replace the outdated TTL and decay terminology in this paragraph
with appropriate retention-score model terminology to accurately describe how
the forgetting defaults and per-partition tuning work in the new console
implementation.
In `@repo_pages/zh/concepts/forgetting.md`:
- Around line 11-15: Add language tags to all bare fenced code blocks in the
file to comply with markdownlint MD040 rules. For each code fence that currently
has no language identifier after the triple backticks (at lines 11-15, 148-152,
and 160-164), append the word `text` immediately after the opening triple
backticks. This will label all mathematical formulas and similar content blocks
consistently as plain text examples and eliminate the linting violations.
In `@repo_pages/zh/guide/web-console.md`:
- Around line 66-68: Update the retention configuration terminology in the
"记忆遗忘（`#forget`）" section to align with v0.2.0 documentation standards. Replace
the outdated "基础 TTL、衰减" language with the new "half-life / threshold" model
terminology to ensure consistency with the rest of the documentation and prevent
readers from applying deprecated configuration patterns. Review other relevant
sections in the documentation to understand the exact terminology used for the
new model and apply it consistently throughout this section.
In `@reports/design/retention-forgetting-model-2026-06-22.md`:
- Line 110: The table row for mem_hippocampus has fewer columns than the other
rows in the table, causing rendering issues. Either add placeholder cells (using
| —) to match the column count of the surrounding table rows, or remove the
mem_hippocampus row entirely and add the note about it never being swept and
being drained by consolidation as separate text outside the table.
In `@src/hebb/retrieval/searcher.py`:
- Around line 31-36: The comment for the recency decay base constant contains a
mathematical inconsistency: it states that decay=0.693 yields approximately a
1-day half-life in the formula score=decay**hours, but this base actually
produces approximately a 1.9-hour half-life. Correct this discrepancy by either
updating the decay value to achieve the intended 1-day half-life (approximately
0.967 for 24-hour half-life) or correcting the stated half-life duration in the
comment to reflect what 0.693 actually produces. Ensure the comment accurately
documents the recency weighting behavior so future developers tune it correctly.
In `@src/hebb/static/css/style.css`:
- Around line 340-343: In the .mem-card-body CSS class, replace the deprecated
property `word-break: break-word` with the standards-compliant `overflow-wrap:
anywhere`. This ensures better compatibility and aligns with current CSS
standards. Remove the word-break line and add overflow-wrap with the value
anywhere to maintain the same text wrapping behavior while using a
non-deprecated approach.
In `@src/hebb/static/js/app.js`:
- Around line 103-107: The rerender logic at the pages[currentPage] call is
using a potentially stale currentSub variable because sub-tabs are updated via
history.replaceState() which does not trigger hashchange events, leaving
currentSub out of sync with location.hash. Before calling
pages[currentPage](content, currentSub), extract the current sub-tab identifier
from location.hash to ensure currentSub is resynchronized with the actual
current state before rerendering the page content.
In `@src/hebb/static/js/components/forgetting.js`:
- Around line 437-439: The table header in the forgetting impact table contains
a hardcoded acc string that is not localized, while all other column headers use
the i18n translation function t(). Replace the hardcoded acc string in the table
header row with a properly localized translation call using t() function with an
appropriate key (such as t('forgetting.col_access_count') or similar) to match
the pattern of the other headers like col_content, importance, and col_age.
In `@src/hebb/static/js/components/memories.js`:
- Around line 226-229: The async onclick handler for
root.querySelector('`#btn-create`') lacks error handling for the
api.listPartitions() call, which means if the API request fails, the user
receives no feedback. Wrap the api.listPartitions() await call in a try/catch
block, keeping the showCreateModal() call in the try block and adding error
handling in the catch block that toasts the error message to inform the user of
the failure.
---
Nitpick comments:
In `@src/hebb/server/routers/admin.py`:
- Around line 176-187: The current implementation in the admin router calls
update_expiry one memory at a time within the for loop over memories, creating
O(N) individual database writes. Instead of calling await
memory_store.update_expiry for each memory individually, collect all the expiry
updates (pairing each memory.id with its computed expires_at) into a single data
structure and make a single batched update call to memory_store, similar to how
the scheduler path handles this, to avoid the write/commit pressure of
individual row updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ea0e1f00-bfd6-4527-bfea-437f148b7dcb
📒 Files selected for processing (59)
.claude-plugin/plugin.json.release-please-manifest.jsonCHANGELOG.mdpyproject.tomlrepo_pages/api/config.mdrepo_pages/concepts/forgetting.mdrepo_pages/concepts/memory-lifecycle.mdrepo_pages/guide/configuration.mdrepo_pages/guide/switch-embedding-model.mdrepo_pages/guide/web-console.mdrepo_pages/troubleshooting.mdrepo_pages/zh/api/config.mdrepo_pages/zh/concepts/forgetting.mdrepo_pages/zh/concepts/memory-lifecycle.mdrepo_pages/zh/guide/configuration.mdrepo_pages/zh/guide/switch-embedding-model.mdrepo_pages/zh/guide/web-console.mdrepo_pages/zh/troubleshooting.mdreports/design/on-demand-ml-deps-2026-06-22.mdreports/design/per-partition-forgetting-ui-2026-06-22.mdreports/design/retention-forgetting-model-2026-06-22.mdreports/design/web-console-restructure-2026-06-22.mdreports/research/brain-recall-integration-2026-06-17.mdsrc/hebb/__init__.pysrc/hebb/config/loader.pysrc/hebb/config/settings.pysrc/hebb/retrieval/searcher.pysrc/hebb/scheduler/forgetting_job.pysrc/hebb/scheduler/manager.pysrc/hebb/server/app.pysrc/hebb/server/forgetting_tracker.pysrc/hebb/server/routers/admin.pysrc/hebb/server/routers/config.pysrc/hebb/server/routers/forgetting.pysrc/hebb/static/css/style.csssrc/hebb/static/index.htmlsrc/hebb/static/js/api.jssrc/hebb/static/js/app.jssrc/hebb/static/js/components/activate.jssrc/hebb/static/js/components/config-section.jssrc/hebb/static/js/components/consolidate.jssrc/hebb/static/js/components/forget.jssrc/hebb/static/js/components/forgetting.jssrc/hebb/static/js/components/graph.jssrc/hebb/static/js/components/manage.jssrc/hebb/static/js/components/memories.jssrc/hebb/static/js/components/partitions.jssrc/hebb/static/js/components/system.jssrc/hebb/static/js/i18n.jssrc/hebb/static/js/lib/forgetting-math.jssrc/hebb/static/js/lifecycle.jstests/integration/server/test_forgetting_router.pytests/unit/config/test_forgetting_config.pytests/unit/scheduler/test_forgetting_formula.pytests/unit/scheduler/test_forgetting_job.pytests/unit/scheduler/test_forgetting_overrides.pytests/unit/server/test_forgetting_tracker.pytests/unit/test_audit_consolidation.pytests/unit/test_audit_distribution.py
Comment on lines
+23
to
+25
| 1. **Per-partition params = `base_ttl_hours`, `decay_factor`, `enabled`.** | ||
| The two formula knobs plus an on/off switch (off = never forget that | ||
| partition). `min_ttl`/grace stay global module constants (out of scope). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align this spec with the retention-model contract.
These sections still describe the removed TTL model (base_ttl_hours / decay_factor), but the live /api/v1/admin/forgetting contract now exposes half_life_days, k_importance, k_access, threshold, and enabled. As per PR objectives, this UI needs to follow the retention fields so frontend work doesn’t target a stale schema.
Also applies to: 32-33, 53-63, 78-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@reports/design/per-partition-forgetting-ui-2026-06-22.md` around lines 23 -
25, The design document describes per-partition parameters using the old TTL
model (base_ttl_hours and decay_factor), but the actual live API contract at
/api/v1/admin/forgetting exposes different retention model parameters
(half_life_days, k_importance, k_access, threshold, and enabled). Update all
references to the old parameter names throughout the document (including
sections at lines 23-25, 32-33, 53-63, and 78-88) to use the current retention
model field names so the UI specification matches the actual contract the
frontend will consume.
| | `manage` | Manage / 记忆管理 | **Overview stat band** (total memories · partitions · graph nodes/edges) on top, then in-page **tabs**: 记忆 (list + add) · 分区 (distribution chart + list + add + edit) · 图谱 (knowledge graph) | | ||
| | `activate` | Activate / 记忆激活 | **Recall test** (search w/ weight sliders + results) on top, then **Recall parameters** (recall pipeline toggles · rerank · scoring weights) | | ||
| | `consolidate` | Consolidate / 记忆巩固 | **Trigger** (Organize now + pending/auto-next meta + interrupted note) · **Run records** (history w/ live log) · **Consolidation config** (time, concurrency, max tokens, drain-empty) | | ||
| | `forget` | Forget / 记忆遗忘 | **Trigger** (Clean up now) · **Forget records** (NEW tracker) · **Global config** (base TTL, decay, sweep interval) · **Per-partition tuning** (curve + matrix + impact + override) | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the Forget page copy to the new retention fields.
base TTL / decay / sweep interval no longer matches the admin UI contract. The Forget page should describe half_life_days, k_importance, k_access, threshold, and per-partition overrides instead. As per PR objectives, the new Forget workflow is wired to the retention model.
Also applies to: 30-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@reports/design/web-console-restructure-2026-06-22.md` at line 25, Update the
Forget feature description to replace the outdated retention field names with
the new retention model fields. In the table row describing the forget
functionality, replace the references to `base TTL`, `decay`, and `sweep
interval` with the correct field names: `half_life_days`, `k_importance`,
`k_access`, `threshold`, and per-partition overrides. This applies to all
mentions of these fields throughout the document (lines 25, 30-32) to ensure
consistency with the current admin UI contract for the retention model.
Comment on lines
+117
to
+119
| eff = eff_half_life_days(half_life_days, k_importance, k_access, memory.importance_score, memory.access_count) | ||
| idle = forget_idle_days(eff, threshold, min_retention_days) | ||
| return memory.last_accessed_at + timedelta(days=idle) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
half_life_days = 3650.0
k_access = 10.0
threshold = 0.3
access_count = 300000 # large but plausible over long-lived hot memories
eff = half_life_days * (1 + k_access * (access_count / 10.0))
idle = eff * math.log(1.0 / threshold)
print("idle_days =", idle)
now = datetime.now(timezone.utc)
try:
_ = now + timedelta(days=idle)
print("No overflow")
except OverflowError as e:
print("OverflowError:", e)
PYRepository: afx-team/hebb-mind
Length of output: 252
🏁 Script executed:
cd /tmp && find . -name "forgetting_job.py" -type f 2>/dev/null | head -5Repository: afx-team/hebb-mind
Length of output: 156
🏁 Script executed:
git ls-files | grep -E "(forgetting|scheduler)" | head -20Repository: afx-team/hebb-mind
Length of output: 976
🏁 Script executed:
cat -n src/hebb/scheduler/forgetting_job.pyRepository: afx-team/hebb-mind
Length of output: 9332
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
# Test the proposed fix logic
half_life_days = 3650.0
k_access = 10.0
threshold = 0.3
access_count = 300000
eff = half_life_days * (1 + k_access * (access_count / 10.0))
idle = eff * math.log(1.0 / threshold)
print(f"idle_days (uncapped) = {idle}")
# Simulate the proposed fix
now = datetime.now(timezone.utc)
print(f"current datetime: {now}")
print(f"datetime.max: {datetime.max}")
max_dt = datetime.max.replace(tzinfo=now.tzinfo)
print(f"max_dt with same tzinfo: {max_dt}")
max_idle_days = max((max_dt - now).total_seconds() / 86400.0, 0.0)
print(f"max_idle_days = {max_idle_days}")
idle_capped = min(idle, max_idle_days)
print(f"idle_capped = {idle_capped}")
# Try adding the capped idle
try:
result = now + timedelta(days=idle_capped)
print(f"Success: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
PYRepository: afx-team/hebb-mind
Length of output: 456
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
# Edge case: test with a datetime far in the past
old_date = datetime(2000, 1, 1, tzinfo=timezone.utc)
print(f"old_date: {old_date}")
half_life_days = 3650.0
k_access = 10.0
threshold = 0.3
access_count = 300000
eff = half_life_days * (1 + k_access * (access_count / 10.0))
idle = eff * math.log(1.0 / threshold)
print(f"idle_days (uncapped) = {idle}")
# Proposed fix with old date
max_dt = datetime.max.replace(tzinfo=old_date.tzinfo)
max_idle_days = max((max_dt - old_date).total_seconds() / 86400.0, 0.0)
print(f"max_idle_days for old date = {max_idle_days}")
idle_capped = min(idle, max_idle_days)
print(f"idle_capped = {idle_capped}")
# Try adding the capped idle
try:
result = old_date + timedelta(days=idle_capped)
print(f"Success: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
PYRepository: afx-team/hebb-mind
Length of output: 336
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
# The issue with the proposed fix: it doesn't account for old dates properly
old_date = datetime(2000, 1, 1, tzinfo=timezone.utc)
half_life_days = 3650.0
k_access = 10.0
threshold = 0.3
access_count = 300000
eff = half_life_days * (1 + k_access * (access_count / 10.0))
idle = eff * math.log(1.0 / threshold)
print(f"idle_days (uncapped) = {idle}")
# The CORRECT fix: cap the result to datetime.max, not the idle days
max_dt = datetime.max.replace(tzinfo=old_date.tzinfo)
max_idle_days = max((max_dt - old_date).total_seconds() / 86400.0, 0.0)
print(f"max_idle_days (from old_date to max): {max_idle_days}")
idle_capped = min(idle, max_idle_days)
print(f"idle_capped = {idle_capped}")
result_before = old_date + timedelta(days=idle_capped)
print(f"result before capping: {result_before}")
# Cap the result itself to datetime.max
result = min(result_before, max_dt)
print(f"result after capping to datetime.max: {result}")
# Verify no overflow
try:
_ = old_date + timedelta(days=idle_capped)
except OverflowError as e:
print(f"OverflowError still occurs: {e}")
PYRepository: afx-team/hebb-mind
Length of output: 383
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
# The correct approach: cap the RESULT, not the days
old_date = datetime(2000, 1, 1, tzinfo=timezone.utc)
half_life_days = 3650.0
k_access = 10.0
threshold = 0.3
access_count = 300000
eff = half_life_days * (1 + k_access * (access_count / 10.0))
idle = eff * math.log(1.0 / threshold)
print(f"idle_days (uncapped) = {idle}")
max_dt = datetime.max.replace(tzinfo=old_date.tzinfo)
# Better approach: use a reasonable max timedelta instead
max_timedelta_days = 999999999.0
idle_capped = min(idle, max_timedelta_days)
print(f"idle_capped = {idle_capped}")
try:
result = old_date + timedelta(days=idle_capped)
print(f"Success with max_timedelta: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
# Fallback: cap to datetime.max
result = min(old_date + timedelta(days=idle_capped), max_dt)
print(f"After fallback: {result}")
PYRepository: afx-team/hebb-mind
Length of output: 559
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
# Test what timedelta max actually is
try:
td = timedelta(days=999999999)
print(f"timedelta(days=999999999) works: {td}")
except OverflowError as e:
print(f"OverflowError: {e}")
try:
td = timedelta(days=999999999.0)
print(f"timedelta(days=999999999.0) works: {td}")
except OverflowError as e:
print(f"OverflowError: {e}")
try:
td = timedelta(days=1000000000)
print(f"timedelta(days=1000000000) works: {td}")
except OverflowError as e:
print(f"OverflowError: {e}")
# Test the actual computation with a reasonable bound
old_date = datetime(2000, 1, 1, tzinfo=timezone.utc)
max_dt = datetime.max.replace(tzinfo=old_date.tzinfo)
idle = 1318354615.2376359
idle_capped = min(idle, 999999999)
print(f"\nidle_capped to 999999999: {idle_capped}")
try:
result = old_date + timedelta(days=idle_capped)
print(f"Result: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
PYRepository: afx-team/hebb-mind
Length of output: 413
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
# The KEY insight: the max_idle_days calculation depends on the starting date
# For a date in 2000, there are fewer days left to 9999
# For a date in 2026, there are fewer days left to 9999
old_date = datetime(2000, 1, 1, tzinfo=timezone.utc)
recent_date = datetime(2026, 6, 23, tzinfo=timezone.utc)
max_dt = datetime.max.replace(tzinfo=timezone.utc)
print(f"From 2000-01-01 to 9999-12-31: {(max_dt - old_date).days} days")
print(f"From 2026-06-23 to 9999-12-31: {(max_dt - recent_date).days} days")
idle = 1318354615.2376359
# The proposed fix tries to cap idle to max_idle_days, but it doesn't work
# for dates in the past because you can't add that many days
max_idle_old = (max_dt - old_date).total_seconds() / 86400.0
idle_capped = min(idle, max_idle_old)
print(f"\nidle_capped for old_date: {idle_capped}")
try:
result = old_date + timedelta(days=idle_capped)
print(f"Success: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
# For a recent date, the calculation works
max_idle_recent = (max_dt - recent_date).total_seconds() / 86400.0
idle_capped_recent = min(idle, max_idle_recent)
print(f"\nidle_capped for recent_date: {idle_capped_recent}")
try:
result = recent_date + timedelta(days=idle_capped_recent)
print(f"Success: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
PYRepository: afx-team/hebb-mind
Length of output: 399
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
old_date = datetime(2000, 1, 1, tzinfo=timezone.utc)
max_dt = datetime.max.replace(tzinfo=timezone.utc)
print(f"max_dt: {max_dt}")
print(f"(max_dt - old_date).days: {(max_dt - old_date).days}")
print(f"(max_dt - old_date).total_seconds() / 86400.0: {(max_dt - old_date).total_seconds() / 86400.0}")
# The issue: max_dt is 9999-12-31 23:59:59.999999
# When we compute total_seconds() / 86400, we get fractional days
# But when we add a timedelta with integer+fractional days, Python tries to
# construct a datetime that goes beyond max
# Let's try capping at slightly less than max
slightly_less = (max_dt - old_date).total_seconds() / 86400.0 - 1.0
try:
result = old_date + timedelta(days=slightly_less)
print(f"Success with slightly_less: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
# Or try a different approach: directly return datetime.max if we would exceed it
idle = 1318354615.2376359
proposed_result = old_date + timedelta(days=idle) # This will fail
# Instead: compute and cap the result
try:
# Clamp to the valid range
result = old_date + timedelta(days=min(idle, 999999999))
except OverflowError:
result = max_dt
print(f"\nDirect capping to max_dt: {result}")
PYRepository: afx-team/hebb-mind
Length of output: 481
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import math
from datetime import datetime, timezone, timedelta
# Test alternative approaches to handle the overflow
old_date = datetime(2000, 1, 1, tzinfo=timezone.utc)
idle = 1318354615.2376359
# Approach 1: Try/except to catch overflow
print("Approach 1: Try/except")
try:
result = old_date + timedelta(days=idle)
except OverflowError:
max_dt = datetime.max.replace(tzinfo=old_date.tzinfo)
result = max_dt
print(f"Result: {result}")
# Approach 2: Compute max_idle_days with safety margin
print("\nApproach 2: Compute max_idle_days with margin")
max_dt = datetime.max.replace(tzinfo=old_date.tzinfo)
max_idle_days = (max_dt - old_date).days - 1 # Use .days instead of total_seconds
idle_capped = min(idle, float(max_idle_days))
try:
result = old_date + timedelta(days=idle_capped)
print(f"Result: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
# Approach 3: Use days property instead of total_seconds
print("\nApproach 3: Use .days property")
max_dt = datetime.max.replace(tzinfo=old_date.tzinfo)
# The difference between datetimes gives a timedelta
# We want the maximum days we can safely add
delta = max_dt - old_date
max_safe_days = delta.days # This is an integer
idle_capped = min(idle, float(max_safe_days))
try:
result = old_date + timedelta(days=idle_capped)
print(f"Result: {result}")
except OverflowError as e:
print(f"OverflowError: {e}")
PYRepository: afx-team/hebb-mind
Length of output: 367
🏁 Script executed:
git ls-files | grep -E "test.*forgetting" | head -10Repository: afx-team/hebb-mind
Length of output: 438
🏁 Script executed:
cat -n tests/unit/scheduler/test_forgetting_job.py | head -100Repository: afx-team/hebb-mind
Length of output: 3709
Guard compute_expires_at against timedelta overflow from high access_count.
With uncapped access_count, idle can exceed timedelta limits and raise OverflowError. This is confirmed to occur with realistic parameters (e.g., access_count=300000 produces idle≈1.3 billion days vs. timedelta's 999999999-day max).
However, the proposed fix has a critical flaw: it uses total_seconds() / 86400.0 for the maximum idle calculation, which can still produce overflow when last_accessed_at is an older date. A more robust solution uses either:
- A try/except pattern to catch overflow and cap to
datetime.max, or - Conservative integer-day capping via
.daysproperty instead of fractional seconds
🛠️ Recommended fix
def compute_expires_at(
@@
) -> datetime:
@@
eff = eff_half_life_days(half_life_days, k_importance, k_access, memory.importance_score, memory.access_count)
idle = forget_idle_days(eff, threshold, min_retention_days)
- return memory.last_accessed_at + timedelta(days=idle)
+ try:
+ return memory.last_accessed_at + timedelta(days=idle)
+ except OverflowError:
+ return datetime.max.replace(tzinfo=memory.last_accessed_at.tzinfo)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/scheduler/forgetting_job.py` around lines 117 - 119, The
`compute_expires_at` function can raise an `OverflowError` when `idle` from
`forget_idle_days` is too large to fit in a timedelta. Instead of using
fractional seconds for capping, implement a try/except block around the final
return statement that catches `OverflowError` and returns `datetime.max` as a
safe fallback, or alternatively cap the `idle` value to an integer-day limit
(such as timedelta's maximum days of 999999999) before creating the timedelta
from `memory.last_accessed_at + timedelta(days=idle)` to prevent the overflow
from occurring in the first place.
Comment on lines
+189
to
+193
| await purge_memory(memory_store, kg, memory.id, save=False) | ||
| deleted += 1 | ||
| if deleted > 0: | ||
| kg.save() | ||
| except Exception as exc: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Wrap manual purge/save in knowledge_graph.lock to avoid graph-write races.
The manual /forget path mutates and saves the graph without the shared graph lock, while the scheduler path does lock this critical section. This can interleave concurrent graph writes and persist inconsistent state.
Suggested fix
+ to_delete: list[str] = []
try:
partitions = await partition_store.list()
for partition in partitions:
...
for memory in memories:
...
if expires_at < now:
- await purge_memory(memory_store, kg, memory.id, save=False)
- deleted += 1
- if deleted > 0:
- kg.save()
+ to_delete.append(memory.id)
+ if to_delete:
+ async with kg.lock:
+ for mid in to_delete:
+ await purge_memory(memory_store, kg, mid, save=False)
+ deleted += 1
+ kg.save()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await purge_memory(memory_store, kg, memory.id, save=False) | |
| deleted += 1 | |
| if deleted > 0: | |
| kg.save() | |
| except Exception as exc: | |
| to_delete.append(memory.id) | |
| if to_delete: | |
| async with kg.lock: | |
| for mid in to_delete: | |
| await purge_memory(memory_store, kg, mid, save=False) | |
| deleted += 1 | |
| kg.save() | |
| except Exception as exc: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/server/routers/admin.py` around lines 189 - 193, The manual purge
operation in the `/forget` endpoint mutates and saves the knowledge graph
without acquiring the shared graph lock, creating a race condition with
concurrent scheduler writes. Wrap the critical section containing the
purge_memory calls and the subsequent kg.save() operation with the
knowledge_graph.lock context manager (using async with) to ensure mutual
exclusion. This mirrors the locking pattern used in the scheduler path and
prevents interleaved graph writes from persisting inconsistent state.
Comment on lines
+234
to
+236
| new_map = {pid: o.model_dump() for pid, o in settings.forgetting_overrides.items()} | ||
| new_map[partition_id] = override.model_dump() | ||
| _persist_and_sync(settings, new_map) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Avoid full-map stale-snapshot writes for overrides (lost-update risk).
Both set/clear paths rebuild new_map from in-memory settings.forgetting_overrides and then replace the entire persisted map. Concurrent requests (or multi-process writers) can overwrite each other’s partition changes.
This needs an atomic per-partition mutate-in-loader flow (read current on-disk map under lock, apply one key add/remove, validate, write), rather than replacing from a potentially stale snapshot.
Also applies to: 252-253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/server/routers/forgetting.py` around lines 234 - 236, The code at
lines 234-236 and 252-253 reconstructs the full forgetting_overrides map from an
in-memory snapshot of settings.forgetting_overrides and then persists it
entirely via _persist_and_sync, creating a lost-update vulnerability where
concurrent requests overwriting different partitions can lose changes. Replace
this full-map rebuild approach with an atomic per-partition mutation pattern:
read the current on-disk map while holding a lock, apply only the single
partition key's add or remove operation (for the set path at line 234-236 and
the clear path at lines 252-253), validate the change, and write back only the
updated map. This ensures that each concurrent modification updates from the
current disk state rather than a potentially stale in-memory snapshot.
Comment on lines
+31
to
+35
| async function loadList(root) { | ||
| const list = root.querySelector('#mem-list'); | ||
| const info = root.querySelector('#mem-info'); | ||
| tbody.innerHTML = '<tr><td colspan="6" class="text-muted" style="text-align:center">Loading...</td></tr>'; | ||
| const pag = root.querySelector('#mem-pagination'); | ||
| list.innerHTML = `<div class="text-muted" style="padding:28px;text-align:center">${t('common.loading')}</div>`; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard loadList against tab unmount during async fetch.
After Line 39 awaits api.listMemories, the Manage tab can be switched, leaving cached DOM refs detached; subsequent DOM writes/event wiring can fail on stale nodes. Add mount checks before and after the await.
🔧 Proposed fix
async function loadList(root) {
const list = root.querySelector('`#mem-list`');
const info = root.querySelector('`#mem-info`');
const pag = root.querySelector('`#mem-pagination`');
+ if (!list || !info || !pag) return;
list.innerHTML = `<div class="text-muted" style="padding:28px;text-align:center">${t('common.loading')}</div>`;
try {
const params = { offset, limit: PAGE_SIZE };
if (currentPartition) params.partition_id = currentPartition;
const data = await api.listMemories(params);
+ if (!list.isConnected || !info.isConnected || !pag.isConnected) return;
info.textContent = t('memories.count', { n: data.total });Also applies to: 39-44, 72-74
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 34-34: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: list.innerHTML = <div class="text-muted" style="padding:28px;text-align:center">${t('common.loading')}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/static/js/components/memories.js` around lines 31 - 35, The loadList
function caches DOM references at the start (list, info, pag elements) but these
can become detached if the user switches tabs while the api.listMemories await
is pending on line 39. Add a mount check before the await to verify the root
element is still in the document, and add another mount check immediately after
the await completes before performing any DOM operations (innerHTML updates,
event listeners, etc.). This ensures that stale DOM references are not
manipulated after tab unmounting. Apply the same guard pattern to the other
async operations mentioned at lines 72-74.
Comment on lines
34
to
40
| async function loadList(root) { | ||
| const container = root.querySelector('#part-list'); | ||
| container.innerHTML = '<div class="text-muted" style="padding:20px;text-align:center">Loading...</div>'; | ||
| container.innerHTML = `<div class="text-muted" style="padding:20px;text-align:center">${t('common.loading')}</div>`; | ||
| try { | ||
| const list = await api.listPartitions(); | ||
| renderDistribution(root, list); | ||
| container.innerHTML = list.map(p => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Prevent stale async DOM updates in loadList.
At Line 38, api.listPartitions() can resolve after the tab has been replaced, but Line 40+ still mutates the old container and binds handlers. Add presence/connection checks around the await.
🔧 Proposed fix
async function loadList(root) {
const container = root.querySelector('`#part-list`');
+ if (!container) return;
container.innerHTML = `<div class="text-muted" style="padding:20px;text-align:center">${t('common.loading')}</div>`;
try {
const list = await api.listPartitions();
+ if (!container.isConnected) return;
renderDistribution(root, list);
container.innerHTML = list.map(p => {Also applies to: 74-83
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 35-35: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: container.innerHTML = <div class="text-muted" style="padding:20px;text-align:center">${t('common.loading')}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/static/js/components/partitions.js` around lines 34 - 40, The
loadList function has a race condition where api.listPartitions() can resolve
after the DOM has changed (tab replaced), causing stale updates to the container
element. After awaiting the api.listPartitions() call, add a check to verify
that the container element is still present in the root element before
proceeding with innerHTML updates and handler binding. This check should
validate that the container reference is still valid in the DOM to prevent
mutations on detached or replaced elements.
Comment on lines
89
to
91
| } catch (e) { | ||
| root.querySelector('#settings-panel').innerHTML = `<div class="empty-state">${e.message}</div>`; | ||
| root.querySelector('#system-panel').innerHTML = `<div class="empty-state">${e.message}</div>`; | ||
| return; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Escape config-load error text before injecting into HTML.
Line 90 injects e.message directly into innerHTML. Rendered error strings should be escaped to prevent script/markup injection.
🔧 Proposed fix
} catch (e) {
- root.querySelector('`#system-panel`').innerHTML = `<div class="empty-state">${e.message}</div>`;
+ root.querySelector('`#system-panel`').innerHTML = `<div class="empty-state">${esc(e.message)}</div>`;
return;
}🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 89-89: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: root.querySelector('#system-panel').innerHTML = <div class="empty-state">${e.message}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/static/js/components/system.js` around lines 89 - 91, The error
message from the catch block is being directly injected into innerHTML without
proper escaping, which creates a potential XSS vulnerability. In the catch block
where root.querySelector('`#system-panel`').innerHTML is set with e.message,
escape the error message text before inserting it into the DOM. You can create a
helper function or use a method like creating a temporary text element and
extracting its textContent, or use a library utility for HTML escaping, to
ensure that any HTML special characters in the error message are properly
encoded before being included in the innerHTML assignment.
Source: Linters/SAST tools
Comment on lines
+44
to
+49
| patch("hebb.config.loader.find_config_file", return_value=config_path), | ||
| patch("hebb.embedding.factory.create_embedder", side_effect=_mock_create_embedder), | ||
| ): | ||
| from hebb.server.app import create_app | ||
|
|
||
| app = create_app() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify possible import-order hazard and patch target alignment.
rg -n -C2 'from\s+hebb\.server\.app\s+import\s+create_app|import\s+hebb\.server\.app' --type=py tests src
rg -n -C2 'patch\("hebb\.embedding\.factory\.create_embedder"|patch\("hebb\.server\.app\.create_embedder"' --type=py tests
rg -n -C2 'from hebb\.embedding\.factory import create_embedder' src/hebb/server/app.pyRepository: afx-team/hebb-mind
Length of output: 7861
🏁 Script executed:
cat src/hebb/server/app.py | head -100Repository: afx-team/hebb-mind
Length of output: 4264
🏁 Script executed:
cat src/hebb/server/app.py | wc -lRepository: afx-team/hebb-mind
Length of output: 160
🏁 Script executed:
cat tests/integration/server/test_forgetting_router.pyRepository: afx-team/hebb-mind
Length of output: 7587
Patch the symbol used by create_app to avoid order-dependent test behavior.
Line 45 patches hebb.embedding.factory.create_embedder, but create_app calls the symbol imported into hebb.server.app. If hebb.server.app was imported earlier in the session (e.g., by another test file), the module-level import in that module will have already executed with the real embedder, and the later patch will not affect the cached reference.
Suggested fix
with (
patch("hebb.config.loader.find_config_file", return_value=config_path),
- patch("hebb.embedding.factory.create_embedder", side_effect=_mock_create_embedder),
+ patch("hebb.server.app.create_embedder", side_effect=_mock_create_embedder),
):
from hebb.server.app import create_app📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| patch("hebb.config.loader.find_config_file", return_value=config_path), | |
| patch("hebb.embedding.factory.create_embedder", side_effect=_mock_create_embedder), | |
| ): | |
| from hebb.server.app import create_app | |
| app = create_app() | |
| patch("hebb.config.loader.find_config_file", return_value=config_path), | |
| patch("hebb.server.app.create_embedder", side_effect=_mock_create_embedder), | |
| ): | |
| from hebb.server.app import create_app | |
| app = create_app() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/server/test_forgetting_router.py` around lines 44 - 49, The
patch target on line 45 is incorrect for the `create_app` function's usage.
Instead of patching `hebb.embedding.factory.create_embedder`, patch the symbol
where it is imported and used within the `hebb.server.app` module. Change the
patch target to `hebb.server.app.create_embedder` to ensure the patch intercepts
the actual reference used by the `create_app` function, preventing issues with
cached module-level imports from earlier test runs.
Comment on lines
+195
to
+211
| result = subprocess.run( | ||
| ["git", "ls-files", "-z", "--", *_DOC_ROOTS], | ||
| cwd=REPO_ROOT, | ||
| capture_output=True, | ||
| text=True, | ||
| check=True, | ||
| ) | ||
| files: list[Path] = [] | ||
| for rel in result.stdout.split("\0"): | ||
| if not rel: | ||
| continue | ||
| path = REPO_ROOT / rel | ||
| if ".vitepress" in path.parts: | ||
| continue # build output / cache / theme, not authored docs | ||
| if path.suffix == ".md" or (path.suffix == ".py" and "examples" in path.parts): | ||
| files.append(path) | ||
| return files |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make doc-file discovery resilient when Git metadata is unavailable.
_doc_files() hard-fails on git ls-files (check=True). In sdist/exported trees (no .git) or environments without git, this test fails before running the actual drift assertions.
Suggested patch
def _doc_files() -> list[Path]:
@@
- result = subprocess.run(
- ["git", "ls-files", "-z", "--", *_DOC_ROOTS],
- cwd=REPO_ROOT,
- capture_output=True,
- text=True,
- check=True,
- )
+ try:
+ result = subprocess.run(
+ ["git", "ls-files", "-z", "--", *_DOC_ROOTS],
+ cwd=REPO_ROOT,
+ capture_output=True,
+ text=True,
+ check=True,
+ )
+ rel_paths = [p for p in result.stdout.split("\0") if p]
+ except (FileNotFoundError, subprocess.CalledProcessError):
+ rel_paths: list[str] = []
+ for root in _DOC_ROOTS:
+ abs_root = REPO_ROOT / root
+ if abs_root.is_file():
+ rel_paths.append(root)
+ elif abs_root.is_dir():
+ rel_paths.extend(str(p.relative_to(REPO_ROOT)) for p in abs_root.rglob("*"))
@@
- for rel in result.stdout.split("\0"):
- if not rel:
- continue
+ for rel in rel_paths:
path = REPO_ROOT / rel📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| result = subprocess.run( | |
| ["git", "ls-files", "-z", "--", *_DOC_ROOTS], | |
| cwd=REPO_ROOT, | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| ) | |
| files: list[Path] = [] | |
| for rel in result.stdout.split("\0"): | |
| if not rel: | |
| continue | |
| path = REPO_ROOT / rel | |
| if ".vitepress" in path.parts: | |
| continue # build output / cache / theme, not authored docs | |
| if path.suffix == ".md" or (path.suffix == ".py" and "examples" in path.parts): | |
| files.append(path) | |
| return files | |
| try: | |
| result = subprocess.run( | |
| ["git", "ls-files", "-z", "--", *_DOC_ROOTS], | |
| cwd=REPO_ROOT, | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| ) | |
| rel_paths = [p for p in result.stdout.split("\0") if p] | |
| except (FileNotFoundError, subprocess.CalledProcessError): | |
| rel_paths: list[str] = [] | |
| for root in _DOC_ROOTS: | |
| abs_root = REPO_ROOT / root | |
| if abs_root.is_file(): | |
| rel_paths.append(root) | |
| elif abs_root.is_dir(): | |
| rel_paths.extend(str(p.relative_to(REPO_ROOT)) for p in abs_root.rglob("*")) | |
| files: list[Path] = [] | |
| for rel in rel_paths: | |
| path = REPO_ROOT / rel | |
| if ".vitepress" in path.parts: | |
| continue # build output / cache / theme, not authored docs | |
| if path.suffix == ".md" or (path.suffix == ".py" and "examples" in path.parts): | |
| files.append(path) | |
| return files |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/test_audit_distribution.py` around lines 195 - 211, The
_doc_files() function uses check=True in the subprocess.run() call for the git
ls-files command, which causes it to raise an exception and hard-fail when Git
is unavailable (such as in sdist/exported trees without .git or environments
without git installed). Remove the check=True parameter from the
subprocess.run() call and instead check the returncode after execution. If the
git command fails (returncode is non-zero), return an empty list instead of
raising an exception, allowing the test to continue with its actual drift
assertions.
…get) numpy 2.5.0 ships PEP 695 `type` statements in its stubs, which mypy (run on Python 3.12 in CI) rejects under our deliberate `python_version = "3.10"` target — a stub-syntax/target mismatch, not our code. Add a numpy mypy override with follow_imports=skip + follow_imports_for_stubs, rather than dropping the 3.10 target or pinning numpy down. numpy is shallowly typed here anyway (disallow_any_generics is off), so no meaningful checking is lost. Verified against numpy 2.5.0 locally: mypy clean, 719 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two coordinated changes that overhaul memory lifecycle management, plus a 0.1.8 → 0.2.0 bump (breaking config change).
1. Web console restructure
Reorganized around the four lifecycle stages, with a unified design language and deep-linkable in-page tabs:
lifecycle.jsteardown registry (tears down timers/observers on navigation); static servedno-cacheto survive ES-module upgrades.2. Retention-score forgetting model (replaces dynamic-TTL crossover)
Before → after (default config): a max-importance, 100×-accessed memory went from forgotten in 4.2 days → ~3.6 years.
Global
base_ttl_hours/decay_factorand the per-partition override fields of the same name are removed, replaced byhalf_life_days/k_importance/k_access/forget_threshold/forget_min_retention_days(global) andhalf_life_days/k_importance/k_access/threshold(per-partition override). Legacyhebb.jsonkeys are ignored; per-partition overrides fall back to region/global defaults — re-tune in the console's Forgetting page if you had custom retention policies.Verification
--strictclean (121 files); ruff clean.Design docs in
reports/design/; concept/config/API docs updated EN+ZH; CHANGELOG[0.2.0]added.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes - v0.2.0
New Features
Breaking Changes
base_ttl_hours/decay_factor, replaced withhalf_life_days,k_*,forget_threshold,forget_min_retention_days).Chores