Skip to content

fix(pr799): address F1/F2/F3 review findings (scope pagination, durable reflection tag)#805

Draft
jlin53882 wants to merge 4 commits into
CortexReach:masterfrom
jlin53882:fix/PR799-v3
Draft

fix(pr799): address F1/F2/F3 review findings (scope pagination, durable reflection tag)#805
jlin53882 wants to merge 4 commits into
CortexReach:masterfrom
jlin53882:fix/PR799-v3

Conversation

@jlin53882
Copy link
Copy Markdown
Contributor

PR799 Review Fixes

Addresses Must-Fix items from review #4289587231 on PR #799.

Must Fix (all 3 resolved)

Issue Fix
F1 Scope pagination starves target scopes MAX_LOW_RATIO_PAGES: 3→5, LOW_RATIO_THRESHOLD: 0.1→0.01
F2 Reflection loop tag not durable dreaming-engine added to MemorySource type + normalizeSource()
F3 AccessTracker cleanup out-of-scope var Confirmed OK — stop() already calls tracker.destroy()

MR Items (pre-existing in base)

Issue Status
MR1 Hot path readFileSync ✅ Already cached via _cachedWorkspaceDir
MR2 DREAMS.md unbounded N/A — current architecture uses queue-based self-improvement
MR3 Scope discovery no upper bound ✅ Covered by F1 (MAX_LOW_RATIO_PAGES = 5)
MR4 Dynamic import in hot path ✅ Static import already used
MR5 openclaw.plugin.json default ⚠️ Description change only — intentional behavior update

Diff

2 files changed, +9/-3 lines — fully targeted patches from base commit 26d313b.


Review: #4289587231 | Base: jlinfork/jlinfork/pr-752-rebuild @ 26d313b

TurboTheTurtle and others added 4 commits May 15, 2026 14:07
Co-authored-by: Andy Ye <andy@Andys-MacBook-Pro-2.local>
… tests [FIXED openclaw.plugin.json block replace]
Must Fix:
- F1: collectExactScope ratio-based pagination (10% threshold, 3-page stop)
- F2: top-level DREAMING_SOURCE_FIELD for reflection durability
- F3: AccessTracker cleanup via _singletonState?.accessTracker
- F4: remove unused accessCount++ in recordAccessAndMaybeTransition
- F5: register dreaming-engine.test.ts in CI manifest
- F6: fix init-plugin-state-async.test.mjs import (memoryLanceDBProPlugin?.register)

Minor Refactors:
- MR1: cache workspaceDir + existsSync instead of sync readFileSync in hot path
- MR2: MAX_DREAMS_LINES=2000 truncation to prevent unbounded DREAMS.md growth
- MR3: MAX_SCOPE_DISCOVERY_ITERATIONS=20 bound on scope discovery loop
- MR4: remove duplicate dynamic import, use static createDreamingEngine
- MR5: restore default:[] for autoRecallExcludeAgents in plugin schema
Must Fix (all 3):
- F1: MAX_LOW_RATIO_PAGES 3→5, LOW_RATIO_THRESHOLD 0.1→0.01
  (scope pagination was starving target scopes when mixed with null-scope rows)
- F2: Add 'dreaming-engine' to MemorySource type + normalizeSource()
  (durable tag for dream reflections through compact/upgrade cycles)
- F3: AccessTracker cleanup already present in stop() — confirmed OK
  (tracker.destroy() on plugin shutdown)

MR items (already present in base):
- MR1: _cachedWorkspaceDir caching (present in 26d313b)
- MR2: DREAMS.md not applicable (no DREAMS.md in current architecture)
- MR3: Scope pagination via MAX_LOW_RATIO_PAGES (covered by F1 fix)
- MR4: Static import of createDreamingEngine (present in 26d313b)
- MR5: openclaw.plugin.json autoRecallExcludeAgents (description change only)

Base: jlinfork/jlinfork/pr-752-rebuild @ 26d313b
@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented May 17, 2026

This draft appears to overlap with #799, but it is still conflicting with master and is not ready for review. Please clarify whether this PR replaces #799; if it does, close the superseded PR so there is only one active branch to review.

Before marking ready for review, please also clean up the diff:

  • Rebase onto latest origin/master.
  • Preserve all current master entries in scripts/ci-test-manifest.mjs and only add this PR's necessary test entry.
  • Fix the test/dreaming-engine.test.ts registration so CI can actually run it, or convert it to the repo's existing runnable test format.
  • Remove unrelated files/changes from the branch, including .learnings and unrelated retriever/MMR/package-lock changes unless they are explicitly required for this PR.

Please run before requesting review:

npm run test:packaging-and-workflow
npm run test:core-regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants