fix(memory): complete recursive memory search follow-up#72748
Conversation
Greptile SummaryThis PR closes remaining gaps from issue #34400 across four areas: nested dated-path support in temporal decay, blocking search-sync to prevent stale first-search results, a QMD watcher refactor that watches collection roots and filters events at the handler stage, and user-facing text alignment to The core logic changes are well-reasoned and the test coverage (107 tests) is thorough. The only P2 note is that Confidence Score: 4/5Safe to merge; only P2 style finding on an unused parameter. No P0 or P1 issues found. One P2 style observation (unused No files require special attention beyond the unused Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/memory-core/src/memory/qmd-manager.ts
Line: 183-192
Comment:
**Unused `stats` parameter**
`shouldIgnoreMemoryWatchPath` accepts `stats` to match chokidar's callback signature, but the implementation never inspects it. The function currently only needs the path to check for ignored directory name segments. If the intent is purely to satisfy the type signature, a leading underscore (`_stats`) would make this intent explicit and suppress lint warnings.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: complete memory recursive search fo..." | Re-trigger Greptile |
| function shouldIgnoreMemoryWatchPath( | ||
| watchPath: string, | ||
| stats: { isFile(): boolean; isDirectory(): boolean } | undefined, | ||
| ): boolean { | ||
| const normalized = path.normalize(watchPath); | ||
| const parts = normalized | ||
| .split(path.sep) | ||
| .map((segment) => normalizeLowercaseStringOrEmpty(segment)); | ||
| return parts.some((segment) => IGNORED_MEMORY_WATCH_DIR_NAMES.has(segment)); | ||
| } |
There was a problem hiding this comment.
shouldIgnoreMemoryWatchPath accepts stats to match chokidar's callback signature, but the implementation never inspects it. The function currently only needs the path to check for ignored directory name segments. If the intent is purely to satisfy the type signature, a leading underscore (_stats) would make this intent explicit and suppress lint warnings.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/memory-core/src/memory/qmd-manager.ts
Line: 183-192
Comment:
**Unused `stats` parameter**
`shouldIgnoreMemoryWatchPath` accepts `stats` to match chokidar's callback signature, but the implementation never inspects it. The function currently only needs the path to check for ignored directory name segments. If the intent is purely to satisfy the type signature, a leading underscore (`_stats`) would make this intent explicit and suppress lint warnings.
How can I resolve this? If you propose a fix, please make it concise.|
Thanks — addressed. I renamed the unused
Result: |
6cceb36 to
3575931
Compare
|
Codex review: needs real behavior proof before merge. Latest ClawSweeper review: 2026-05-20 23:08 UTC / May 20, 2026, 7:08 PM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. source-reproducible. Current main fire-and-forgets dirty search sync, only matches flat dated memory paths for temporal decay, and passes glob-shaped QMD watch targets despite chokidar v5 documentation saying glob support was removed. PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review findings
Review detailsBest possible solution: Land a rebased memory-core follow-up that keeps the runtime fixes, mirrors recursive wording across lazy descriptors and public docs, preserves or explicitly defers timestamp-suffix decay coverage, and includes redacted real behavior proof. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible. Current main fire-and-forgets dirty search sync, only matches flat dated memory paths for temporal decay, and passes glob-shaped QMD watch targets despite chokidar v5 documentation saying glob support was removed. Is this the best way to solve the issue? No, not yet. Awaiting dirty sync and moving QMD filtering to the handler are maintainable, but the PR still needs descriptor/docs alignment, rebase, real proof, and a decision on the temporal-decay overlap. Label changes:
Label justifications:
Full review comments:
Overall correctness: patch is incorrect What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against b58572e283bc. |
3575931 to
77f9f16
Compare
77f9f16 to
2f0d481
Compare
90dd9f3 to
4df486a
Compare
|
Heads up — the 4 remaining red checks on this PR look unrelated to the change:
After rebasing onto current |
4df486a to
446352d
Compare
|
Rebased onto latest |
446352d to
b871c4a
Compare
|
Rebased onto latest Conflicts resolved
Local verification
Pre-existing lint note (not introduced by this PR) |
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
Closing in favor of #85899. The runtime now walks |
PR Draft — memory recursive search follow-up
Suggested title
fix(memory): complete recursive memory search follow-up
Suggested base/head
upstream/mainfix/34400-memory-recursive-followup-mainSummary
This follow-up closes the remaining gaps around issue #34400 for recursive memory search support.
It adds support for nested dated memory paths in temporal decay, ensures dirty search state is synchronized before querying, fixes QMD watcher handling to avoid root-ignore regressions while still filtering non-matching events, and aligns user-facing text with the actual recursive
memory/**/*.mdbehavior.What changed
memory/YYYY-MM/YYYY-MM-DD.mdand similar nested paths are now recognized as dated memory filesmemory/*.mdtomemory/**/*.mdValidation
Unit / targeted tests
4 files / 107 tests passedBuild
Live smoke
Real chokidar smoke
.txtevents are ignored at handling stage.mdand nested.mdevents are handled as intendedLive QMD backend end-to-end smoke
provider: "qmd",backend: "qmd",effectiveMode: "search").mdfile was found on the first search.txtfile in the same nested path was not surfaced in searchLimitations / notes
Related issue