fix(pr799): address F1/F2/F3 review findings (scope pagination, durable reflection tag)#805
Draft
jlin53882 wants to merge 4 commits into
Draft
fix(pr799): address F1/F2/F3 review findings (scope pagination, durable reflection tag)#805jlin53882 wants to merge 4 commits into
jlin53882 wants to merge 4 commits into
Conversation
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
Collaborator
|
This draft appears to overlap with #799, but it is still conflicting with Before marking ready for review, please also clean up the diff:
Please run before requesting review: npm run test:packaging-and-workflow
npm run test:core-regression |
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
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.
PR799 Review Fixes
Addresses Must-Fix items from review #4289587231 on PR #799.
Must Fix (all 3 resolved)
MAX_LOW_RATIO_PAGES: 3→5,LOW_RATIO_THRESHOLD: 0.1→0.01dreaming-engineadded toMemorySourcetype +normalizeSource()stop()already callstracker.destroy()MR Items (pre-existing in base)
readFileSync_cachedWorkspaceDirMAX_LOW_RATIO_PAGES= 5)Diff
2 files changed, +9/-3 lines — fully targeted patches from base commit
26d313b.Review: #4289587231 | Base:
jlinfork/jlinfork/pr-752-rebuild@26d313b