PR #540 (7/10 rebased): feat: run LCM observed extraction in maintenance#28
Closed
100yenadmin wants to merge 4 commits into
Closed
PR #540 (7/10 rebased): feat: run LCM observed extraction in maintenance#28100yenadmin wants to merge 4 commits into
100yenadmin wants to merge 4 commits into
Conversation
…raction in maintenance Inverse-merge of upstream PR Martian-Engineering#540 onto main, layered above 8 hotfix PRs (Martian-Engineering#572-Martian-Engineering#579) plus PR Martian-Engineering#516. Strategy was to start from main and add only pr-540's net new code on top. Net new from pr-540: - src/observed-work-extractor.ts - src/store/event-observation-store.ts - src/store/observed-work-store.ts - src/store/task-bridge-suggestion-store.ts - src/tools/lcm-event-search-tool.ts - src/tools/lcm-work-density-tool.ts - 5 .changeset entries, 2 specs/, 2 new test files - maintain() observed-work + event extraction hook in src/engine.ts (gated on observedWorkMaintenanceEnabled) - new migrations: ensureObservedWorkTables/Indexes/StateCursorColumns, ensureTaskBridgeSuggestionTables/Indexes, ensureEventObservationTables/Indexes (after ensureRollupViews) - new config field observedWorkMaintenanceEnabled (default false) - new contracts.tools entries: lcm_work_density, lcm_event_search - startOfWeekDayString in src/timezone-windows.ts Hotfix code preserved in: - src/engine.ts: kept normalizeSummaryOverlapText/messageContentCoveredBySummary, rollupDailyMaxTokens/WeeklyMaxTokens/MonthlyMaxTokens wiring, computeRollupMaintenanceDaysBack(now: Date) signature + clock.now() call - src/db/config.ts: kept rollup{Daily,Weekly,Monthly}MaxTokens fields/defaults - src/db/migration.ts: kept TableColumnInfo (vs pr-540's old SummaryColumnInfo) - openclaw.plugin.json: kept all main hotfix uiHints/configSchema fields - README.md, docs/configuration.md: kept main's rollup token docs - test/engine.test.ts, test/rollup-store-builder.test.ts: took main - src/tools/lcm-recent-tool.ts (computeAutoDetailLevel): took main - src/rollup-builder.ts, src/store/rollup-store.ts: took main Type fixes: change `args: unknown[]` to `SQLInputValue[]` in observed-work-store.ts and task-bridge-suggestion-store.ts to satisfy node:sqlite types in this main branch. 0 conflict markers. npm run build clean. npm test 997/997 pass.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR adds a comprehensive observed-work feature enabling deterministic extraction of work signals from conversation summaries into SQLite persistence. It introduces new stores (observed items, events, task-bridge suggestions), an extraction pipeline with cursor-based state, read-only tools for density queries and event search, and integrates maintenance hooks into the engine—all gated by a new ChangesObserved-Work Feature Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Key Review Checkpoints
🚥 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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 0/5 reviews remaining, refill in 59 minutes and 49 seconds. Comment |
There was a problem hiding this comment.
Pull request overview
This PR rebases upstream lossless-claw#540 onto main and introduces an “observed work” / “event observation” extraction pipeline that runs during maintain() (opt-in via config), plus new read-only tools to query that extracted evidence.
Changes:
- Add new SQLite tables + stores for observed work items/sources/state, event observations, and task-bridge suggestion scaffolding, wired via migrations.
- Add deterministic extractor (
ObservedWorkExtractor) and run it fromLcmContextEngine.maintain()behindobservedWorkMaintenanceEnabled(defaultfalse). - Register two new tools:
lcm_work_densityandlcm_event_search(with tests/docs/changesets).
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
test/task-bridge-suggestion-store.test.ts |
Adds coverage for the inert task-bridge suggestion store behavior. |
test/observed-work-store.test.ts |
Adds extensive tests for extraction cursoring, density queries, tool behavior, and rollback semantics. |
test/config.test.ts |
Asserts new config defaults and schema exposure for observedWorkMaintenanceEnabled (and rollupDebugEnabled). |
src/tools/lcm-work-density-tool.ts |
Implements lcm_work_density tool (period parsing, scoping, output-budget trimming). |
src/tools/lcm-event-search-tool.ts |
Implements lcm_event_search tool (scoping + deterministic query filters). |
src/timezone-windows.ts |
Adds shared startOfWeekDayString() helper. |
src/store/task-bridge-suggestion-store.ts |
Adds suggestion persistence + review-state transitions (no task writes). |
src/store/observed-work-store.ts |
Adds observed-work write model + density read model + provenance loading. |
src/store/index.ts |
Exports EventObservationStore types/classes from the store barrel. |
src/store/event-observation-store.ts |
Adds event observation persistence + query/search APIs. |
src/plugin/index.ts |
Registers new tools with the plugin runtime. |
src/observed-work-extractor.ts |
Adds deterministic leaf-summary extraction for observed work + events, with per-summary savepoints. |
src/engine.ts |
Wires stores/extractor into LcmContextEngine; runs extraction during maintenance when enabled; adds getters. |
src/db/migration.ts |
Adds migrations for new tables/indexes + cursor column; renames SummaryColumnInfo -> TableColumnInfo. |
src/db/config.ts |
Adds config observedWorkMaintenanceEnabled and env var LCM_OBSERVED_WORK_MAINTENANCE_ENABLED. |
openclaw.plugin.json |
Adds tools to the manifest and exposes observedWorkMaintenanceEnabled in config schema + UI hints. |
docs/configuration.md |
Documents observedWorkMaintenanceEnabled option and env var mapping. |
README.md |
Documents new config key + env var. |
specs/lcm-task-bridge-option-c-experimental.md |
Adds experimental spec for future task bridge; clarifies non-goals/guardrails. |
specs/lcm-observed-work-density-option-b.md |
Adds Option B spec describing observed work density model/tooling. |
.changeset/lcm-task-bridge-suggestions.md |
Changeset for suggestion-store scaffold. |
.changeset/lcm-observed-work-extraction.md |
Changeset for deterministic extraction. |
.changeset/lcm-observed-work-density.md |
Changeset for observed work density + tool. |
.changeset/lcm-extraction-maintenance.md |
Changeset for running extraction during maintenance. |
.changeset/lcm-event-observations.md |
Changeset for event observations + tool. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…anceEnabled flag lcm_work_density and lcm_event_search were registered unconditionally even when observedWorkMaintenanceEnabled=false (the default). With the data path off, the model could call the tools and only ever see empty results — wasted tokens and confusing routing. Gate the registerTool calls behind the flag, mirroring the existing rollupDebugEnabled pattern. Option A: keep both tools listed in openclaw.plugin.json#contracts.tools. OpenClaw 2026.5.2+ rejects registrations not pre-declared in the manifest, so dropping them from contracts.tools (Option B) would break the feature when the flag is enabled. The manifest drift-guard tests (test/manifest.test.ts) continue to pass: contracts.tools.length still matches the registerTool call-site count (the regex matches the calls regardless of being inside an if block) and contracts.tools still matches the canonical name fields in src/tools/lcm-*-tool.ts. No test changes required. MAJOR-2 (top-level transaction in observed-work-extractor.ts) skipped per task instructions — the audit already noted evidence_count is protected by hasSource → sourceAlreadyRecorded, so the missing outer transaction is defensive only.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
added 2 commits
May 4, 2026 17:32
NaN guards on every read-path limit (task-bridge-suggestion-store.list, observed-work-store.getDensity, observed-work-extractor.processConversation, event-observation-store.listObservations, lcm_event_search, lcm_work_density) so callers passing NaN/Infinity fall through to safe defaults instead of binding non-integer values to SQL LIMIT ?. lcm_work_density: validate minConfidence as finite and within 0..1, clamp limit/detailLevel, and capture wall-clock through deps.clock.now() for period resolution so tests can inject a frozen clock. Reorder applyOutputBudget so accounting fields are added BEFORE budget enforcement, ensuring the returned JSON respects maxOutputTokens and the stored estimatedOutputTokens reflects the final payload. event-observation-store: drop redundant lower(coalesce(query_key, '')) on the read path so SQLite can use lcm_event_observations_query_time_idx (query_key is already lowercased on write via normalizeQueryKey). observed-work-extractor: preload existing source keys via a chunked loadExistingSourceKeys() batch query so dense leaf summaries no longer trigger an N+1 hasSource() pattern; keep the in-memory set in sync after addSource() so subsequent entries in the same pass observe the recorded source. lcm-recent-tool: drop the duplicated startOfWeekDayString helper and import the shared one from timezone-windows so date validation is consistent.
…antics; cleanup Adversarial review surfaced that PR #28 carried older versions of the observed-work stack than release/v0.9.4 (which already shipped PR #20-#25 hardening). This commit ports release/v0.9.4 forward and keeps PR #28's actual contribution (engine maintain() wiring + config flag + gated tool registration). Port-forwarded from release/v0.9.4 (PR #20-#25 hardening): - src/store/observed-work-store.ts (+ COALESCE/cursor invariants) - src/store/task-bridge-suggestion-store.ts (withDatabaseTransaction, MAX_SUGGESTION_SOURCE_IDS cap) - src/store/event-observation-store.ts (withDatabaseTransaction, MAX_EVENT_SOURCE_IDS cap) - src/observed-work-extractor.ts (NEGATED_COMPLETION_RE, fingerprintVersion=3, async processConversation) - src/tools/lcm-work-density-tool.ts - src/tools/lcm-event-search-tool.ts - test/observed-work-store.test.ts, test/task-bridge-suggestion-store.test.ts Cascading PR #28 changes: - src/engine.ts maintain() awaits the now-async processConversation. - TypeBox enum idiom in lcm-work-density-tool / lcm-event-search-tool switched from Type.String({enum:[...]}) to Type.Union of Type.Literal, matching lcm-recent-tool's pattern. CRITICAL fix — pendingRebuild semantics: listUnprocessedLeafSummaries reads only the (createdAt,id,rowid) cursor and never consults pending_rebuild, so the maintain() catch block's upsertState({pendingRebuild:true}) was write-only — a crashed batch would be silently skipped on the next pass. Resolution: add ObservedWorkStore.clearProcessingCursor(conversationId) that nulls the cursor fields while leaving pending_rebuild=true intact. engine.ts now calls both upsertState({pendingRebuild:true}) AND clearProcessingCursor() in the catch block, forcing the next pass to rescan from the conversation's first leaf summary. Regression test added to test/observed-work-store.test.ts. Cleanup: - node_modules symlink removed from git index, .gitignore widened to match both `node_modules/` (directory) and `node_modules` (symlink). - test/manifest.test.ts adds a comment explaining the expected asymmetry — lcm_work_density and lcm_event_search are declared in contracts.tools but their registerTool() calls live inside an `if (config.observedWorkMaintenanceEnabled)` block. Validation: - npm run build: clean - npm test: 1002/1002 (was 999 on release/v0.9.4 + 3 new tests) - npx tsc --noEmit: zero net-new errors in changed files - node_modules NOT in git index
100yenadmin
pushed a commit
that referenced
this pull request
May 4, 2026
…enance Conflict resolution: took theirs (rebase/540 has port-forwarded PR #20-#25 hardening + new wiring). New: engine.maintain() invokes ObservedWorkExtractor.processConversation gated by observedWorkMaintenanceEnabled flag. Tool registration (lcm_work_density + lcm_event_search) also gated. pendingRebuild semantics fixed via clearProcessingCursor() helper. Manifest declares tools (asymmetry with gated registration documented in test/manifest.test.ts).
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@node_modules`:
- Line 1: Remove the committed file named "node_modules" (the tracked file that
contains the absolute path "/Volumes/LEXAR/...") from the repository and stop
tracking it, then add "node_modules/" to .gitignore to prevent reintroduction;
ensure you remove the tracked file from the index (so npm can create a real
node_modules directory) and commit the removal and .gitignore change with a
clear message.
In `@specs/lcm-observed-work-density-option-b.md`:
- Around line 124-130: The spec's table definition is out of date: update the
lcm_observed_work_state CREATE TABLE sketch to include the
last_processed_summary_rowid column (matching the migration/runtime usage) and
adjust the narrative around extraction to reflect that
observed-work-extractor.ts and the rowid cursor are implemented; specifically
mention last_processed_summary_rowid in the schema and remove or rewrite the
statements at the previous "extraction out-of-scope/not implemented" locations
to document that an extractor and maintenance integration exist and how they
interact with the pending_rebuild/last_processed_summary_* fields.
In `@src/engine.ts`:
- Around line 6015-6046: The maintain() path currently ignores observed-work
mutations: when this.observedWorkExtractor.processConversation(...) returns
observedResult that upserts summaries/workItems/events, those mutations must be
reflected in the maintain() return value (the local result variable) instead of
returning the original unmodified result; update the code in maintain() after
receiving observedResult to merge/increment the corresponding fields on result
(e.g., summariesScanned, workItemsUpserted, eventsUpserted or a generic
changed/updated flag) so callers see the persisted changes, and ensure the error
path that calls this.observedWorkStore.upsertState(...) still preserves/upserts
state but does not prevent propagating any prior observedResult mutations into
the returned result.
In `@src/observed-work-extractor.ts`:
- Around line 251-257: The state read and batch selection
(observedWorkStore.getState and listUnprocessedLeafSummaries using
conversationId and state) must be protected by a per-conversation critical
section to avoid races; wrap the sequence "getState ->
listUnprocessedLeafSummaries -> cursor update/insert" inside a lock keyed by
conversationId (either a DB advisory lock or a process-level async mutex map) so
only one extractor run for a given conversation executes that block at a time,
await the lock before calling observedWorkStore.getState and release it in a
finally block after advancing the cursor to ensure atomicity from the
extractor’s perspective.
In `@src/store/event-observation-store.ts`:
- Around line 134-171: The upsertObservation function allows empty
eventId/conversationId which can cause unintended ON CONFLICT(event_id)
collisions; before using normalizeSourceIds or running the prepared INSERT/ON
CONFLICT statement, trim and validate input.eventId and input.conversationId
(e.g., const eventId = input.eventId?.trim(); const conversationId =
input.conversationId?.trim()) and throw descriptive Errors if either is
missing/empty; keep using normalizeSourceIds and the existing db.prepare/ON
CONFLICT logic but pass the validated eventId and conversationId variables into
.run to prevent silent overwrites.
In `@src/store/task-bridge-suggestion-store.ts`:
- Around line 279-295: The reviewSuggestion handler currently uses the raw
input.suggestionId which can contain surrounding whitespace and bypass the
validation/normalization used in upsertSuggestion; update reviewSuggestion to
normalize and validate suggestionId the same way as upsertSuggestion (trim the
ID, reject empty/invalid IDs) before running the DB UPDATE so whitespace-padded
IDs are normalized and the query targets the intended row; reference the
reviewSuggestion function and reuse the same normalization/validation logic or
helper used by upsertSuggestion.
- Around line 197-200: The UPDATE's CASE for task_id keeps old values due to
COALESCE(excluded.task_id, lcm_task_bridge_suggestions.task_id); change the
logic so pending upserts assign excluded.task_id directly (allowing NULL to
clear stale task_id when kind changes, e.g., to create_task). In the SQL
fragment in task-bridge-suggestion-store.ts, replace "WHEN ... THEN
COALESCE(excluded.task_id, lcm_task_bridge_suggestions.task_id)" with "WHEN ...
THEN excluded.task_id" and leave the ELSE branch as
lcm_task_bridge_suggestions.task_id so non-pending rows retain their existing
task_id.
In `@src/tools/lcm-work-density-tool.ts`:
- Around line 250-266: The current loop sets budgetTruncated only when trimming
operations occurred, so budgetTruncated can remain false even if the final
payload still exceeds the budget; after the trimming loop in
lcm-work-density-tool.ts (the while using
estimateJsonTokens/trimOneSource/trimOneItem and the guard counter), re-evaluate
the final token count with estimateJsonTokens(next) and set budgetTruncated =
true if that count still exceeds the budget variable (or maxOutputTokens), then
update accounting.itemsReturned, accounting.budgetTruncated and
accounting.truncated and accounting.estimatedOutputTokens accordingly so
accounting accurately reflects whether the payload was truncated due to budget.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 77abf5de-878a-46b7-8cf7-ef9cf7430f21
📒 Files selected for processing (27)
.changeset/lcm-event-observations.md.changeset/lcm-extraction-maintenance.md.changeset/lcm-observed-work-density.md.changeset/lcm-observed-work-extraction.md.changeset/lcm-task-bridge-suggestions.mdREADME.mddocs/configuration.mdnode_modulesopenclaw.plugin.jsonspecs/lcm-observed-work-density-option-b.mdspecs/lcm-task-bridge-option-c-experimental.mdsrc/db/config.tssrc/db/migration.tssrc/engine.tssrc/observed-work-extractor.tssrc/plugin/index.tssrc/store/event-observation-store.tssrc/store/index.tssrc/store/observed-work-store.tssrc/store/task-bridge-suggestion-store.tssrc/timezone-windows.tssrc/tools/lcm-event-search-tool.tssrc/tools/lcm-recent-tool.tssrc/tools/lcm-work-density-tool.tstest/config.test.tstest/observed-work-store.test.tstest/task-bridge-suggestion-store.test.ts
📜 Review details
🧰 Additional context used
🪛 LanguageTool
specs/lcm-observed-work-density-option-b.md
[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...completed? - Which appear unfinished? - Which are ambiguous and need deeper inspectio...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~213-~213: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... extraction from new leaf summaries. 3. Add compact lcm_work_density read tool. 4...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~214-~214: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ompact lcm_work_density read tool. 4. Add confidence/rationale/accounting. 5. Add...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~215-~215: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...Add confidence/rationale/accounting. 5. Add tests for density counts, false complet...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~249-~249: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ose tasks. - Does not sync to Cortex. - Does not mutate state from reads. ### Revie...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~253-~253: Since ownership is already implied, this phrasing may be redundant.
Context: ...his branch - The tables are created in their own idempotent migration steps instead of b...
(PRP_OWN)
specs/lcm-task-bridge-option-c-experimental.md
[grammar] ~9-~9: Use a hyphen to join words.
Context: ... Option C explores a bridge between LCM observed work signals and OpenClaw's tas...
(QB_NEW_EN_HYPHEN)
[style] ~15-~15: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ce help a user review existing tasks? - Can agents link conversation evidence to op...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...creation. 4. No silent task closure. 5. No reminders/wakes/notifications from LCM ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
.changeset/lcm-observed-work-extraction.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.changeset/lcm-task-bridge-suggestions.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.changeset/lcm-observed-work-density.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.changeset/lcm-event-observations.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.changeset/lcm-extraction-maintenance.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (18)
src/timezone-windows.ts (1)
28-34: Looks good: shared week-start helper is correct.The UTC weekday calculation plus the Monday offset preserves the existing Monday-start behavior, and reusing
addDayskeeps the date arithmetic safe and consistent.src/tools/lcm-recent-tool.ts (1)
7-14: Good cleanup: shared helper import is consistent.Moving the week-start logic into
timezone-windows.tsremoves duplication without changing the period-resolution flow here..changeset/lcm-event-observations.md (1)
1-5: Changeset entry is correctly scoped and release-typed.
- Patch bump and summary match the introduced event-observation extraction/tooling surface.
.changeset/lcm-observed-work-extraction.md (1)
1-5: Release note aligns with the extraction behavior introduced in this PR.
- Patch level and wording are consistent with deterministic observed-work extraction scope.
.changeset/lcm-observed-work-density.md (1)
1-5: Semver classification and note text look correct.
- Minor bump appropriately reflects the new
lcm_work_densitytool/read-model addition..changeset/lcm-task-bridge-suggestions.md (1)
1-5: Changeset messaging is accurate for the inert task-bridge scaffold.
- Patch bump and wording match the non-activated persistence-only behavior.
specs/lcm-task-bridge-option-c-experimental.md (1)
23-35: Guardrails and implementation boundaries are clearly documented.
- The spec explicitly prevents silent task mutation and keeps Option C opt-in/inert, which matches the current store-only scaffold and review workflow.
Also applies to: 172-210
.changeset/lcm-extraction-maintenance.md (1)
1-5: Changeset accurately captures maintenance-path extraction behavior.
- Patch bump and description are consistent with the gated maintain() extraction flow.
docs/configuration.md (1)
49-49: Config docs are consistent and operationally clear.
- The new key is documented in both the full example and reference table, with the correct default (
false) and env override mapping.Also applies to: 121-121
src/store/index.ts (1)
46-51: Barrel exports are correctly wired for the new event-observation store surface.
- Re-exporting both the class and public types keeps the store API consistent with existing module export patterns.
README.md (1)
145-146: Docs match the new config surface.The example config, env-var reference, and plugin-config equivalents are consistent with the resolver and manifest changes.
Also applies to: 194-194, 242-243
src/plugin/index.ts (1)
24-25: Conditional tool wiring looks right.The new imports and registrations are cleanly gated behind
observedWorkMaintenanceEnabled, which matches the rest of the feature rollout.Also applies to: 2309-2324
src/db/config.ts (1)
132-133: Config resolution is consistent.The new flag is wired through the type and resolver with the expected env/plugin/default precedence.
Also applies to: 510-513
test/config.test.ts (1)
44-45: Coverage looks solid.These assertions cover defaults, plugin config, env overrides, and manifest schema for the new flag without introducing drift from the resolver logic.
Also applies to: 78-79, 113-114, 144-145, 166-167, 195-196, 652-662
openclaw.plugin.json (1)
14-16: Manifest wiring is consistent.The new tool contracts and
observedWorkMaintenanceEnabledschema/UI hint match the runtime feature gating.Also applies to: 228-231, 451-453
src/engine.ts (2)
53-75: Observed-work wiring looks consistent.The new imports, member declarations, and constructor initialization follow the existing migration-first setup cleanly.
Also applies to: 1874-2009
8058-8064: Public accessors are fine.These getters are a minimal, low-risk way to expose the new stores to the tools that need them.
src/store/observed-work-store.ts (1)
443-556: Density query assembly and category slicing look solid.
- Parameterized filters, bounded limits, and per-status retrieval are consistent and readable.
itemsIncluded/itemsOmittedaccounting is correctly computed from unique IDs.
Comment on lines
+124
to
+130
| CREATE TABLE lcm_observed_work_state ( | ||
| conversation_id INTEGER PRIMARY KEY, | ||
| last_processed_summary_created_at TEXT, | ||
| last_processed_summary_id TEXT, | ||
| pending_rebuild INTEGER NOT NULL DEFAULT 0, | ||
| updated_at TEXT NOT NULL DEFAULT (datetime('now')) | ||
| ); |
There was a problem hiding this comment.
Spec is stale vs current implementation (extractor + rowid cursor now exist)
- Line 124–130 table sketch omits
last_processed_summary_rowid, but the migration/runtime now use it. - Line 230–231 and Line 258–263 state extraction is out-of-scope/not implemented, but this PR adds
src/observed-work-extractor.tsand maintenance integration.
Please update these sections so the spec matches the shipped behavior.
Also applies to: 230-231, 258-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/lcm-observed-work-density-option-b.md` around lines 124 - 130, The
spec's table definition is out of date: update the lcm_observed_work_state
CREATE TABLE sketch to include the last_processed_summary_rowid column (matching
the migration/runtime usage) and adjust the narrative around extraction to
reflect that observed-work-extractor.ts and the rowid cursor are implemented;
specifically mention last_processed_summary_rowid in the schema and remove or
rewrite the statements at the previous "extraction out-of-scope/not implemented"
locations to document that an extractor and maintenance integration exist and
how they interact with the pending_rebuild/last_processed_summary_* fields.
Comment on lines
+6015
to
6046
| if (this.config.observedWorkMaintenanceEnabled) { | ||
| try { | ||
| const observedResult = this.observedWorkExtractor.processConversation( | ||
| conversation.conversationId, | ||
| { limit: 500 }, | ||
| ); | ||
| if ( | ||
| observedResult.summariesScanned > 0 || | ||
| observedResult.workItemsUpserted > 0 || | ||
| observedResult.eventsUpserted > 0 | ||
| ) { | ||
| this.deps.log.info( | ||
| `[lcm] maintain: observed-work extraction conversation=${conversation.conversationId} ${sessionLabel} summariesScanned=${observedResult.summariesScanned} workItemsUpserted=${observedResult.workItemsUpserted} eventsUpserted=${observedResult.eventsUpserted}`, | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| this.deps.log.warn( | ||
| `[lcm] maintain: observed-work extraction failed conversation=${conversation.conversationId} ${sessionLabel}: ${describeLogError(error)}`, | ||
| ); | ||
| try { | ||
| this.observedWorkStore.upsertState({ | ||
| conversationId: conversation.conversationId, | ||
| pendingRebuild: true, | ||
| }); | ||
| } catch (stateError) { | ||
| this.deps.log.warn( | ||
| `[lcm] maintain: failed to preserve observed-work pending state conversation=${conversation.conversationId} ${sessionLabel}: ${describeLogError(stateError)}`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| return result; |
There was a problem hiding this comment.
Propagate observed-work writes into maintain()'s return value.
processConversation() can upsert work items/events even when the existing GC/deferred-compaction result is unchanged, but finish() still returns the original result. That makes maintain() report a no-op after it has actually mutated persistent state, which can hide real maintenance work from callers.
♻️ Suggested fix
if (this.config.observedWorkMaintenanceEnabled) {
try {
const observedResult = this.observedWorkExtractor.processConversation(
conversation.conversationId,
{ limit: 500 },
);
+ const observedWorkChanged =
+ observedResult.summariesScanned > 0 ||
+ observedResult.workItemsUpserted > 0 ||
+ observedResult.eventsUpserted > 0;
if (
observedResult.summariesScanned > 0 ||
observedResult.workItemsUpserted > 0 ||
observedResult.eventsUpserted > 0
) {
@@
);
}
+ if (observedWorkChanged) {
+ result = { ...result, changed: true };
+ }
} catch (error) {📝 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
| if (this.config.observedWorkMaintenanceEnabled) { | |
| try { | |
| const observedResult = this.observedWorkExtractor.processConversation( | |
| conversation.conversationId, | |
| { limit: 500 }, | |
| ); | |
| if ( | |
| observedResult.summariesScanned > 0 || | |
| observedResult.workItemsUpserted > 0 || | |
| observedResult.eventsUpserted > 0 | |
| ) { | |
| this.deps.log.info( | |
| `[lcm] maintain: observed-work extraction conversation=${conversation.conversationId} ${sessionLabel} summariesScanned=${observedResult.summariesScanned} workItemsUpserted=${observedResult.workItemsUpserted} eventsUpserted=${observedResult.eventsUpserted}`, | |
| ); | |
| } | |
| } catch (error) { | |
| this.deps.log.warn( | |
| `[lcm] maintain: observed-work extraction failed conversation=${conversation.conversationId} ${sessionLabel}: ${describeLogError(error)}`, | |
| ); | |
| try { | |
| this.observedWorkStore.upsertState({ | |
| conversationId: conversation.conversationId, | |
| pendingRebuild: true, | |
| }); | |
| } catch (stateError) { | |
| this.deps.log.warn( | |
| `[lcm] maintain: failed to preserve observed-work pending state conversation=${conversation.conversationId} ${sessionLabel}: ${describeLogError(stateError)}`, | |
| ); | |
| } | |
| } | |
| } | |
| return result; | |
| if (this.config.observedWorkMaintenanceEnabled) { | |
| try { | |
| const observedResult = this.observedWorkExtractor.processConversation( | |
| conversation.conversationId, | |
| { limit: 500 }, | |
| ); | |
| const observedWorkChanged = | |
| observedResult.summariesScanned > 0 || | |
| observedResult.workItemsUpserted > 0 || | |
| observedResult.eventsUpserted > 0; | |
| if ( | |
| observedResult.summariesScanned > 0 || | |
| observedResult.workItemsUpserted > 0 || | |
| observedResult.eventsUpserted > 0 | |
| ) { | |
| this.deps.log.info( | |
| `[lcm] maintain: observed-work extraction conversation=${conversation.conversationId} ${sessionLabel} summariesScanned=${observedResult.summariesScanned} workItemsUpserted=${observedResult.workItemsUpserted} eventsUpserted=${observedResult.eventsUpserted}`, | |
| ); | |
| } | |
| if (observedWorkChanged) { | |
| result = { ...result, changed: true }; | |
| } | |
| } catch (error) { | |
| this.deps.log.warn( | |
| `[lcm] maintain: observed-work extraction failed conversation=${conversation.conversationId} ${sessionLabel}: ${describeLogError(error)}`, | |
| ); | |
| try { | |
| this.observedWorkStore.upsertState({ | |
| conversationId: conversation.conversationId, | |
| pendingRebuild: true, | |
| }); | |
| } catch (stateError) { | |
| this.deps.log.warn( | |
| `[lcm] maintain: failed to preserve observed-work pending state conversation=${conversation.conversationId} ${sessionLabel}: ${describeLogError(stateError)}`, | |
| ); | |
| } | |
| } | |
| } | |
| return result; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/engine.ts` around lines 6015 - 6046, The maintain() path currently
ignores observed-work mutations: when
this.observedWorkExtractor.processConversation(...) returns observedResult that
upserts summaries/workItems/events, those mutations must be reflected in the
maintain() return value (the local result variable) instead of returning the
original unmodified result; update the code in maintain() after receiving
observedResult to merge/increment the corresponding fields on result (e.g.,
summariesScanned, workItemsUpserted, eventsUpserted or a generic changed/updated
flag) so callers see the persisted changes, and ensure the error path that calls
this.observedWorkStore.upsertState(...) still preserves/upserts state but does
not prevent propagating any prior observedResult mutations into the returned
result.
Comment on lines
+251
to
+257
| const state = this.observedWorkStore.getState(conversationId); | ||
| const rawLimit = options?.limit; | ||
| const limit = | ||
| typeof rawLimit === "number" && Number.isFinite(rawLimit) | ||
| ? Math.max(1, Math.min(Math.trunc(rawLimit), 1000)) | ||
| : 200; | ||
| const rows = this.listUnprocessedLeafSummaries(conversationId, state, limit); |
There was a problem hiding this comment.
Serialize extraction per conversation to avoid cursor races
- Line 251 reads state, and Line 257 fetches the next batch without any per-conversation lock.
- Two concurrent runs can select the same summaries, causing overlapping writes and conflict/rollback churn on source insertion and cursor advancement.
Please gate this section with a per-conversation critical section (DB-level or process-level) so state read + batch selection + cursor update are atomic from the extractor’s perspective.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/observed-work-extractor.ts` around lines 251 - 257, The state read and
batch selection (observedWorkStore.getState and listUnprocessedLeafSummaries
using conversationId and state) must be protected by a per-conversation critical
section to avoid races; wrap the sequence "getState ->
listUnprocessedLeafSummaries -> cursor update/insert" inside a lock keyed by
conversationId (either a DB advisory lock or a process-level async mutex map) so
only one extractor run for a given conversation executes that block at a time,
await the lock before calling observedWorkStore.getState and release it in a
finally block after advancing the cursor to ensure atomicity from the
extractor’s perspective.
Comment on lines
+134
to
+171
| upsertObservation(input: EventObservationInput): void { | ||
| if (!Number.isFinite(input.confidence ?? 0.5) || (input.confidence ?? 0.5) < 0 || (input.confidence ?? 0.5) > 1) { | ||
| throw new Error("confidence must be between 0 and 1."); | ||
| } | ||
| if (input.title.trim().length === 0) { | ||
| throw new Error("event title is required."); | ||
| } | ||
| if (input.rationale.trim().length === 0) { | ||
| throw new Error("event rationale is required."); | ||
| } | ||
| const sourceId = input.sourceId.trim(); | ||
| if (sourceId.length === 0) { | ||
| throw new Error("event source ID is required."); | ||
| } | ||
| const sourceIds = normalizeSourceIds(input.sourceIds, sourceId); | ||
| this.db.prepare( | ||
| `INSERT INTO lcm_event_observations ( | ||
| event_id, conversation_id, event_kind, title, description, query_key, | ||
| event_time, ingest_time, confidence, rationale, source_type, source_id, | ||
| source_ids, updated_at | ||
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, datetime('now')) | ||
| ON CONFLICT(event_id) DO UPDATE SET | ||
| conversation_id = excluded.conversation_id, | ||
| event_kind = excluded.event_kind, | ||
| title = excluded.title, | ||
| description = excluded.description, | ||
| query_key = excluded.query_key, | ||
| event_time = excluded.event_time, | ||
| ingest_time = excluded.ingest_time, | ||
| confidence = excluded.confidence, | ||
| rationale = excluded.rationale, | ||
| source_type = excluded.source_type, | ||
| source_id = excluded.source_id, | ||
| source_ids = excluded.source_ids, | ||
| updated_at = datetime('now')`, | ||
| ).run( | ||
| input.eventId, | ||
| input.conversationId, |
There was a problem hiding this comment.
Missing eventId/conversationId validation can cause silent upsert collisions
upsertObservationcurrently accepts blankeventId.- With
ON CONFLICT(event_id), empty/invalid IDs can overwrite unrelated observations in the same key bucket.
Suggested patch
upsertObservation(input: EventObservationInput): void {
+ const eventId = input.eventId.trim();
+ if (eventId.length === 0) {
+ throw new Error("eventId is required.");
+ }
+ if (!Number.isInteger(input.conversationId) || input.conversationId <= 0) {
+ throw new Error("conversationId must be a positive integer.");
+ }
if (!Number.isFinite(input.confidence ?? 0.5) || (input.confidence ?? 0.5) < 0 || (input.confidence ?? 0.5) > 1) {
throw new Error("confidence must be between 0 and 1.");
}
@@
- input.eventId,
+ eventId,
input.conversationId,📝 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
| upsertObservation(input: EventObservationInput): void { | |
| if (!Number.isFinite(input.confidence ?? 0.5) || (input.confidence ?? 0.5) < 0 || (input.confidence ?? 0.5) > 1) { | |
| throw new Error("confidence must be between 0 and 1."); | |
| } | |
| if (input.title.trim().length === 0) { | |
| throw new Error("event title is required."); | |
| } | |
| if (input.rationale.trim().length === 0) { | |
| throw new Error("event rationale is required."); | |
| } | |
| const sourceId = input.sourceId.trim(); | |
| if (sourceId.length === 0) { | |
| throw new Error("event source ID is required."); | |
| } | |
| const sourceIds = normalizeSourceIds(input.sourceIds, sourceId); | |
| this.db.prepare( | |
| `INSERT INTO lcm_event_observations ( | |
| event_id, conversation_id, event_kind, title, description, query_key, | |
| event_time, ingest_time, confidence, rationale, source_type, source_id, | |
| source_ids, updated_at | |
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, datetime('now')) | |
| ON CONFLICT(event_id) DO UPDATE SET | |
| conversation_id = excluded.conversation_id, | |
| event_kind = excluded.event_kind, | |
| title = excluded.title, | |
| description = excluded.description, | |
| query_key = excluded.query_key, | |
| event_time = excluded.event_time, | |
| ingest_time = excluded.ingest_time, | |
| confidence = excluded.confidence, | |
| rationale = excluded.rationale, | |
| source_type = excluded.source_type, | |
| source_id = excluded.source_id, | |
| source_ids = excluded.source_ids, | |
| updated_at = datetime('now')`, | |
| ).run( | |
| input.eventId, | |
| input.conversationId, | |
| upsertObservation(input: EventObservationInput): void { | |
| const eventId = input.eventId.trim(); | |
| if (eventId.length === 0) { | |
| throw new Error("eventId is required."); | |
| } | |
| if (!Number.isInteger(input.conversationId) || input.conversationId <= 0) { | |
| throw new Error("conversationId must be a positive integer."); | |
| } | |
| if (!Number.isFinite(input.confidence ?? 0.5) || (input.confidence ?? 0.5) < 0 || (input.confidence ?? 0.5) > 1) { | |
| throw new Error("confidence must be between 0 and 1."); | |
| } | |
| if (input.title.trim().length === 0) { | |
| throw new Error("event title is required."); | |
| } | |
| if (input.rationale.trim().length === 0) { | |
| throw new Error("event rationale is required."); | |
| } | |
| const sourceId = input.sourceId.trim(); | |
| if (sourceId.length === 0) { | |
| throw new Error("event source ID is required."); | |
| } | |
| const sourceIds = normalizeSourceIds(input.sourceIds, sourceId); | |
| this.db.prepare( | |
| `INSERT INTO lcm_event_observations ( | |
| event_id, conversation_id, event_kind, title, description, query_key, | |
| event_time, ingest_time, confidence, rationale, source_type, source_id, | |
| source_ids, updated_at | |
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, datetime('now')) | |
| ON CONFLICT(event_id) DO UPDATE SET | |
| conversation_id = excluded.conversation_id, | |
| event_kind = excluded.event_kind, | |
| title = excluded.title, | |
| description = excluded.description, | |
| query_key = excluded.query_key, | |
| event_time = excluded.event_time, | |
| ingest_time = excluded.ingest_time, | |
| confidence = excluded.confidence, | |
| rationale = excluded.rationale, | |
| source_type = excluded.source_type, | |
| source_id = excluded.source_id, | |
| source_ids = excluded.source_ids, | |
| updated_at = datetime('now')`, | |
| ).run( | |
| eventId, | |
| input.conversationId, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/store/event-observation-store.ts` around lines 134 - 171, The
upsertObservation function allows empty eventId/conversationId which can cause
unintended ON CONFLICT(event_id) collisions; before using normalizeSourceIds or
running the prepared INSERT/ON CONFLICT statement, trim and validate
input.eventId and input.conversationId (e.g., const eventId =
input.eventId?.trim(); const conversationId = input.conversationId?.trim()) and
throw descriptive Errors if either is missing/empty; keep using
normalizeSourceIds and the existing db.prepare/ON CONFLICT logic but pass the
validated eventId and conversationId variables into .run to prevent silent
overwrites.
Comment on lines
+279
to
+295
| reviewSuggestion(input: { | ||
| suggestionId: string; | ||
| status: Exclude<TaskBridgeSuggestionStatus, "pending">; | ||
| reviewedBy?: string; | ||
| }): boolean { | ||
| if (!REVIEW_STATUSES.has(input.status)) { | ||
| throw new Error("review status must be accepted, rejected, dismissed, or expired."); | ||
| } | ||
| const reviewedBy = input.reviewedBy?.trim() || null; | ||
| const result = this.db.prepare( | ||
| `UPDATE lcm_task_bridge_suggestions | ||
| SET status = ?, | ||
| reviewed_by = COALESCE(?, reviewed_by), | ||
| reviewed_at = datetime('now'), | ||
| updated_at = datetime('now') | ||
| WHERE suggestion_id = ? AND status = 'pending'`, | ||
| ).run(input.status, reviewedBy, input.suggestionId); |
There was a problem hiding this comment.
Normalize and validate suggestionId in review path.
upsertSuggestion()trims/validates IDs, butreviewSuggestion()uses rawinput.suggestionId(Line 295).- Whitespace-padded IDs will silently return
falseinstead of updating the intended row.
Suggested fix
reviewSuggestion(input: {
suggestionId: string;
status: Exclude<TaskBridgeSuggestionStatus, "pending">;
reviewedBy?: string;
}): boolean {
+ const suggestionId = input.suggestionId.trim();
+ if (suggestionId.length === 0) {
+ throw new Error("suggestionId is required.");
+ }
if (!REVIEW_STATUSES.has(input.status)) {
throw new Error("review status must be accepted, rejected, dismissed, or expired.");
}
const reviewedBy = input.reviewedBy?.trim() || null;
const result = this.db.prepare(
@@
- ).run(input.status, reviewedBy, input.suggestionId);
+ ).run(input.status, reviewedBy, suggestionId);
return result.changes > 0;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/store/task-bridge-suggestion-store.ts` around lines 279 - 295, The
reviewSuggestion handler currently uses the raw input.suggestionId which can
contain surrounding whitespace and bypass the validation/normalization used in
upsertSuggestion; update reviewSuggestion to normalize and validate suggestionId
the same way as upsertSuggestion (trim the ID, reject empty/invalid IDs) before
running the DB UPDATE so whitespace-padded IDs are normalized and the query
targets the intended row; reference the reviewSuggestion function and reuse the
same normalization/validation logic or helper used by upsertSuggestion.
Comment on lines
+250
to
+266
| let budgetTruncated = false; | ||
| let guard = 0; | ||
| while (estimateJsonTokens(next) > budget && guard < 10_000) { | ||
| guard += 1; | ||
| if (trimOneSource(next) || trimOneItem(next)) { | ||
| budgetTruncated = true; | ||
| continue; | ||
| } | ||
| break; | ||
| } | ||
| const accounting = next.accounting as Record<string, unknown>; | ||
| accounting.itemsReturned = countReturnedItems(next); | ||
| accounting.budgetTruncated = budgetTruncated; | ||
| accounting.truncated = Boolean(accounting.truncated) || budgetTruncated; | ||
| // Final estimate reflects the trimmed payload AND every accounting field. | ||
| accounting.estimatedOutputTokens = estimateJsonTokens(next); | ||
| return next; |
There was a problem hiding this comment.
budgetTruncated can be false even when the final payload still exceeds budget
- On Line 252 onward, trimming may stop because no more removable fields exist.
- On Line 262,
budgetTruncatedonly reflects whether trimming happened, not whether the final payload is still overmaxOutputTokens.
Suggested patch
- const accounting = next.accounting as Record<string, unknown>;
- accounting.itemsReturned = countReturnedItems(next);
- accounting.budgetTruncated = budgetTruncated;
- accounting.truncated = Boolean(accounting.truncated) || budgetTruncated;
- // Final estimate reflects the trimmed payload AND every accounting field.
- accounting.estimatedOutputTokens = estimateJsonTokens(next);
+ const accounting = next.accounting as Record<string, unknown>;
+ const estimatedOutputTokens = estimateJsonTokens(next);
+ const overBudgetAfterTrim = estimatedOutputTokens > budget;
+ accounting.itemsReturned = countReturnedItems(next);
+ accounting.budgetTruncated = budgetTruncated || overBudgetAfterTrim;
+ accounting.truncated =
+ Boolean(accounting.truncated) || Boolean(accounting.budgetTruncated);
+ // Final estimate reflects the trimmed payload AND every accounting field.
+ accounting.estimatedOutputTokens = estimatedOutputTokens;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/lcm-work-density-tool.ts` around lines 250 - 266, The current loop
sets budgetTruncated only when trimming operations occurred, so budgetTruncated
can remain false even if the final payload still exceeds the budget; after the
trimming loop in lcm-work-density-tool.ts (the while using
estimateJsonTokens/trimOneSource/trimOneItem and the guard counter), re-evaluate
the final token count with estimateJsonTokens(next) and set budgetTruncated =
true if that count still exceeds the budget variable (or maxOutputTokens), then
update accounting.itemsReturned, accounting.budgetTruncated and
accounting.truncated and accounting.estimatedOutputTokens accordingly so
accounting accurately reflects whether the payload was truncated due to budget.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/store/event-observation-store.ts (1)
149-162:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
eventIdandconversationIdvalidation.The function validates
title,rationale, andsourceIdbut noteventIdorconversationId. An emptyeventIdwould causeON CONFLICT(event_id)collisions; an invalidconversationId(0, negative, NaN) could corrupt data.Suggested fix
async upsertObservation(input: EventObservationInput): Promise<void> { + const eventId = input.eventId.trim(); + if (eventId.length === 0) { + throw new Error("eventId is required."); + } + if (!Number.isInteger(input.conversationId) || input.conversationId <= 0) { + throw new Error("conversationId must be a positive integer."); + } if (!Number.isFinite(input.confidence ?? 0.5) || (input.confidence ?? 0.5) < 0 || (input.confidence ?? 0.5) > 1) {Then use
eventIdinstead ofinput.eventIdin the.run()call at line 206.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/event-observation-store.ts` around lines 149 - 162, The upsertObservation function is missing validation for input.eventId and input.conversationId; add checks inside upsertObservation to ensure eventId is a non-empty trimmed string (throw "event ID is required.") and conversationId is a finite positive integer (throw "conversation ID must be a positive integer.") before proceeding, use the validated local eventId variable (not input.eventId) when building the DB parameters for the .run() call, and update any variable references to conversationId to use the validated value so invalid/empty IDs can't be inserted or cause ON CONFLICT collisions.src/engine.ts (1)
6015-6054:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate observed-work mutations into
maintain()'s return value.
- This block performs durable writes on both paths:
- success can advance the processing cursor and upsert observed-work/event rows,
- failure recovery upserts
pendingRebuildand clears the processing cursor.finish()still returns the originalresult, so callers can seechanged=falseafter SQLite was mutated.♻️ Suggested fix
if (this.config.observedWorkMaintenanceEnabled) { + let observedWorkChanged = false; try { const observedResult = await this.observedWorkExtractor.processConversation( conversation.conversationId, { limit: 500 }, ); + observedWorkChanged = + observedResult.summariesScanned > 0 || + observedResult.workItemsUpserted > 0 || + observedResult.eventsUpserted > 0; if ( observedResult.summariesScanned > 0 || observedResult.workItemsUpserted > 0 || observedResult.eventsUpserted > 0 ) { @@ this.observedWorkStore.upsertState({ conversationId: conversation.conversationId, pendingRebuild: true, }); this.observedWorkStore.clearProcessingCursor(conversation.conversationId); + observedWorkChanged = true; } catch (stateError) { this.deps.log.warn( `[lcm] maintain: failed to preserve observed-work pending state conversation=${conversation.conversationId} ${sessionLabel}: ${describeLogError(stateError)}`, ); } } + if (observedWorkChanged) { + result = { ...result, changed: true }; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine.ts` around lines 6015 - 6054, The maintain() path mutates observed-work state (via observedWorkExtractor.processConversation and the recovery writes observedWorkStore.upsertState / observedWorkStore.clearProcessingCursor) but returns the original result, so callers don't see that durable changes occurred; update the code around finish()/return result to propagate these mutations by setting/returning a result that reflects the writes (e.g., set result.changed = true when observedResult.summariesScanned/workItemsUpserted/eventsUpserted > 0, and also set result.changed = true in the catch recovery after calling observedWorkStore.upsertState/clearProcessingCursor), ensuring any async store calls are awaited before returning.src/store/task-bridge-suggestion-store.ts (1)
328-345:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize and validate
suggestionIdinreviewSuggestion().This path bypasses the trimming/empty-check used by
upsertSuggestion(). A padded ID like" sug_2 "will silently miss the row and reportfalse.Suggested fix
reviewSuggestion(input: { suggestionId: string; status: Exclude<TaskBridgeSuggestionStatus, "pending">; reviewedBy?: string; }): boolean { + const suggestionId = input.suggestionId.trim(); + if (suggestionId.length === 0) { + throw new Error("suggestionId is required."); + } if (!REVIEW_STATUSES.has(input.status)) { throw new Error("review status must be accepted, rejected, dismissed, or expired."); } const reviewedBy = input.reviewedBy?.trim() || null; @@ - ).run(input.status, reviewedBy, input.suggestionId); + ).run(input.status, reviewedBy, suggestionId); return result.changes > 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/task-bridge-suggestion-store.ts` around lines 328 - 345, In reviewSuggestion(), normalize and validate the incoming suggestionId like upsertSuggestion does: trim the input.suggestionId, reject/throw if the trimmed id is empty, and use the trimmed id variable in the DB update call (the prepared statement and .run arguments) so padded IDs like " sug_2 " match rows; keep the existing reviewedBy trimming logic and return semantics (result.changes > 0).src/tools/lcm-work-density-tool.ts (1)
267-289:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMark the response truncated when it is still over budget after trimming stops.
If the loop exits because nothing else can be removed,
budgetTruncatedstaysfalseeven whenestimatedOutputTokens > budget. That leaves the accounting block lying about truncation.Suggested fix
- accounting.estimatedOutputTokens = estimateJsonTokens(next); - if (budgetTruncated) { + const estimatedOutputTokens = estimateJsonTokens(next); + const overBudgetAfterTrim = estimatedOutputTokens > budget; + accounting.estimatedOutputTokens = estimatedOutputTokens; + if (budgetTruncated || overBudgetAfterTrim) { accounting.budgetTruncated = true; accounting.truncated = true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/lcm-work-density-tool.ts` around lines 267 - 289, The loop may exit because no more trimming is possible while the response still exceeds budget, leaving budgetTruncated false; after the while loop (before assigning accounting.estimatedOutputTokens) check if estimateJsonTokens(next) > budget and if so set budgetTruncated = true and mark accounting.budgetTruncated = true and accounting.truncated = true (and preserve accounting.itemsReturned/itemsOmitted as already updated) so truncation is correctly reported; update the existing final accounting block around accounting.estimatedOutputTokens to reflect this extra check and ensure the flags are set when the output is still over budget.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/observed-work-extractor.ts`:
- Around line 148-155: The toIso function currently returns the raw input when
parsing fails, leading to inconsistent timestamps; update toIso to throw a
descriptive Error instead of returning the original value (e.g., include the
invalid input in the error message) so its behavior matches
canonicalizeIsoTimestamp; locate toIso in observed-work-extractor.ts and replace
the fallback return at the Number.isNaN(parsed.getTime()) branch with a thrown
Error (and update any callers/tests that expect the old behavior).
In `@src/tools/lcm-event-search-tool.ts`:
- Around line 36-45: Replace the local parseTimestamp implementation with the
shared parser: remove parseTimestamp and import/use parseIsoTimestampParam
instead; update all call sites in this file (including uses for lcm_work_density
and lcm_grep) to call parseIsoTimestampParam(value, key) and return/propagate
its result so timestamp validation is consistent with the rest of the codebase.
---
Duplicate comments:
In `@src/engine.ts`:
- Around line 6015-6054: The maintain() path mutates observed-work state (via
observedWorkExtractor.processConversation and the recovery writes
observedWorkStore.upsertState / observedWorkStore.clearProcessingCursor) but
returns the original result, so callers don't see that durable changes occurred;
update the code around finish()/return result to propagate these mutations by
setting/returning a result that reflects the writes (e.g., set result.changed =
true when observedResult.summariesScanned/workItemsUpserted/eventsUpserted > 0,
and also set result.changed = true in the catch recovery after calling
observedWorkStore.upsertState/clearProcessingCursor), ensuring any async store
calls are awaited before returning.
In `@src/store/event-observation-store.ts`:
- Around line 149-162: The upsertObservation function is missing validation for
input.eventId and input.conversationId; add checks inside upsertObservation to
ensure eventId is a non-empty trimmed string (throw "event ID is required.") and
conversationId is a finite positive integer (throw "conversation ID must be a
positive integer.") before proceeding, use the validated local eventId variable
(not input.eventId) when building the DB parameters for the .run() call, and
update any variable references to conversationId to use the validated value so
invalid/empty IDs can't be inserted or cause ON CONFLICT collisions.
In `@src/store/task-bridge-suggestion-store.ts`:
- Around line 328-345: In reviewSuggestion(), normalize and validate the
incoming suggestionId like upsertSuggestion does: trim the input.suggestionId,
reject/throw if the trimmed id is empty, and use the trimmed id variable in the
DB update call (the prepared statement and .run arguments) so padded IDs like "
sug_2 " match rows; keep the existing reviewedBy trimming logic and return
semantics (result.changes > 0).
In `@src/tools/lcm-work-density-tool.ts`:
- Around line 267-289: The loop may exit because no more trimming is possible
while the response still exceeds budget, leaving budgetTruncated false; after
the while loop (before assigning accounting.estimatedOutputTokens) check if
estimateJsonTokens(next) > budget and if so set budgetTruncated = true and mark
accounting.budgetTruncated = true and accounting.truncated = true (and preserve
accounting.itemsReturned/itemsOmitted as already updated) so truncation is
correctly reported; update the existing final accounting block around
accounting.estimatedOutputTokens to reflect this extra check and ensure the
flags are set when the output is still over budget.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c31046bf-9a85-4a55-977d-07d80b1991a0
📒 Files selected for processing (11)
.gitignoresrc/engine.tssrc/observed-work-extractor.tssrc/store/event-observation-store.tssrc/store/observed-work-store.tssrc/store/task-bridge-suggestion-store.tssrc/tools/lcm-event-search-tool.tssrc/tools/lcm-work-density-tool.tstest/manifest.test.tstest/observed-work-store.test.tstest/task-bridge-suggestion-store.test.ts
📜 Review details
🔇 Additional comments (13)
test/manifest.test.ts (1)
84-95: Commentary is accurate and improves test intent clarity.
- The added notes correctly explain conditional runtime registration vs unconditional manifest declaration.
- This reduces future false-positive confusion when the flag is disabled at runtime.
.gitignore (1)
2-6: Good.gitignorehardening for symlinked dependencies.
- This correctly handles both real directories (
node_modules/) and symlink entries (node_modules).- The inline rationale is clear and maintenance-friendly.
src/observed-work-extractor.ts (4)
275-281: Cursor race condition partially mitigated but not eliminated.Lines 279-281 read state and select summaries without a per-conversation lock. Two concurrent
processConversationcalls could:
- Both read the same cursor state.
- Both select overlapping summary batches.
- Both attempt to process the same summaries.
Mitigations in place:
hasSourceFromAnyEvidenceprevents double-counting sources.- Per-row
BEGIN IMMEDIATEtransactions serialize writes.ON CONFLICThandlers make upserts idempotent.Remaining risk: Wasted work and potential lock contention, not data corruption. If this is acceptable given the maintenance context (single-threaded
maintain()loop), consider documenting the assumption.
70-85: LGTM!Regex patterns are well-designed:
NEGATED_COMPLETION_REcomprehensively catches negation tokens preceding completion verbs.hashIduses 24 hex characters (96 bits) providing adequate collision resistance for fingerprinting.
369-444: LGTM!Evidence handling is well-designed:
- Two-tier source checking (loose via
hasSourceFromAnyEvidence, strict viahasSource) correctly handles:
- Same-source replay detection (prevents double-counting).
- Evidence kind promotion (different evidenceKind from same source).
- Confidence boosting only applies to genuinely new evidence.
- In-memory cache (
existingByWorkItemId) is updated after each upsert for subsequent iterations.
502-550: LGTM with minor performance note.Cursor logic is robust:
- Prefers rowid-based cursor (most efficient).
- Falls back to
(created_at, summary_id)composite comparison.- ISO-8601 lexicographic comparison is index-friendly.
The correlated subquery for
source_message_count(lines 540-544) runs per row, but impact is bounded by theLIMITclause.src/store/event-observation-store.ts (2)
63-144: LGTM!Helper functions are well-implemented:
escapeLikePatterncorrectly escapes SQL LIKE metacharacters.canonicalizeIsoTimestampproperly validates and rejects invalid dates.normalizeQueryKeyhas comprehensive PR number canonicalization.parseSourceIdssafely handles malformed JSON.
223-281: LGTM!
listObservationsimplementation:
- Properly clamps
limitto safe bounds.- Uses escaped LIKE patterns with explicit ESCAPE clause.
- Time window filters use
coalesce(event_time, ingest_time)consistently.- Query building handles all optional filters correctly.
src/store/observed-work-store.ts (5)
183-231: LGTM!Type definitions are comprehensive and well-structured. The
rowToItemhelper correctly handles optional fields with conditional spreading.getItemis a clean primary key lookup.
233-331: LGTM!
upsertItemhas robust conflict handling:
- Terminal status regression guard (line 255-259) prevents completed/dismissed items from regressing.
- ISO-8601 timestamp comparisons are index-friendly.
- Confidence clamping (line 314) prevents CHECK constraint violations.
- Counter fields use MAX() to preserve accumulation on partial reprocessing.
Well-documented with inline comments explaining design decisions.
402-462: LGTM!
upsertStateproperly enforces cursor field atomicity (lines 416-425):
- All three cursor fields must be provided together or none at all.
- Prevents mixed cursors that could break incremental resume.
The ON CONFLICT CASE statements correctly preserve existing values when input is NULL, allowing
pendingRebuildto be toggled independently.
509-622: LGTM!
getDensityimplementation:
- EXISTS subquery correctly filters to items with at least one source.
- Per-status limits allow balanced representation across categories.
- Aggregate counts and per-status fetches use consistent WHERE clauses.
- Source fetching is correctly gated by
includeSourcesflag.
632-648: No issue found — thecreated_atcolumn exists in thelcm_observed_work_sourcestable definition (migration.ts line 1281) and is correctly used in the window function's ORDER BY clause for deterministic ranking. The column is not projected in the SELECT result set, which is correct behavior.> Likely an incorrect or invalid review comment.
Comment on lines
+148
to
+155
| function toIso(value: string | null | undefined): string { | ||
| if (!value) { | ||
| return new Date().toISOString(); | ||
| } | ||
| const normalized = value.includes("T") ? value : `${value.replace(" ", "T")}Z`; | ||
| const parsed = new Date(normalized); | ||
| return Number.isNaN(parsed.getTime()) ? value : parsed.toISOString(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
toIso returns unparsed value on parse failure, potentially inconsistent.
Line 154 returns the original value if parsed.getTime() is NaN. This could produce inconsistent timestamp formats downstream (e.g., a malformed string like "invalid-date" would pass through unchanged), while canonicalizeIsoTimestamp in event-observation-store.ts throws on invalid dates.
Consider throwing or returning a fallback ISO timestamp instead:
- return Number.isNaN(parsed.getTime()) ? value : parsed.toISOString();
+ if (Number.isNaN(parsed.getTime())) {
+ return new Date().toISOString(); // or throw
+ }
+ return parsed.toISOString();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/observed-work-extractor.ts` around lines 148 - 155, The toIso function
currently returns the raw input when parsing fails, leading to inconsistent
timestamps; update toIso to throw a descriptive Error instead of returning the
original value (e.g., include the invalid input in the error message) so its
behavior matches canonicalizeIsoTimestamp; locate toIso in
observed-work-extractor.ts and replace the fallback return at the
Number.isNaN(parsed.getTime()) branch with a thrown Error (and update any
callers/tests that expect the old behavior).
Comment on lines
+36
to
+45
| function parseTimestamp(value: unknown, key: string): string | undefined { | ||
| if (typeof value !== "string" || value.trim().length === 0) { | ||
| return undefined; | ||
| } | ||
| const parsed = new Date(value.trim()); | ||
| if (Number.isNaN(parsed.getTime())) { | ||
| throw new Error(`${key} must be a valid ISO timestamp.`); | ||
| } | ||
| return parsed.toISOString(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "lcm-event-search-tool.ts|lcm-conversation-scope.ts" src/Repository: 100yenadmin/lossless-claw-test
Length of output: 145
🏁 Script executed:
cat -n src/tools/lcm-event-search-tool.ts | head -120Repository: 100yenadmin/lossless-claw-test
Length of output: 5984
🏁 Script executed:
cat -n src/tools/lcm-conversation-scope.ts | head -200Repository: 100yenadmin/lossless-claw-test
Length of output: 7780
🏁 Script executed:
rg -i "lcm_work_density\|work.?density" src/Repository: 100yenadmin/lossless-claw-test
Length of output: 56
🏁 Script executed:
rg "parseIsoTimestampParam" src/Repository: 100yenadmin/lossless-claw-test
Length of output: 748
🏁 Script executed:
cat -n src/tools/lcm-work-density-tool.ts | grep -A 10 -B 10 "parseIsoTimestampParam"Repository: 100yenadmin/lossless-claw-test
Length of output: 2082
Reuse parseIsoTimestampParam for consistency with other tools.
parseTimestamp() duplicates validation logic already present in parseIsoTimestampParam, which is used by both lcm_work_density and lcm_grep. Use the shared parser to avoid duplication and maintain a single source of truth for timestamp validation.
Suggested fix
import {
+ parseIsoTimestampParam,
resolveLcmConversationScope,
} from "./lcm-conversation-scope.js";
-function parseTimestamp(value: unknown, key: string): string | undefined {
- if (typeof value !== "string" || value.trim().length === 0) {
- return undefined;
- }
- const parsed = new Date(value.trim());
- if (Number.isNaN(parsed.getTime())) {
- throw new Error(`${key} must be a valid ISO timestamp.`);
- }
- return parsed.toISOString();
-}
-
function arrayParam<T extends string>(value: unknown, allowed: readonly T[], key: string): T[] | undefined {- since = parseTimestamp(p.since, "since");
- before = parseTimestamp(p.before, "before");
+ since = parseIsoTimestampParam(p, "since")?.toISOString();
+ before = parseIsoTimestampParam(p, "before")?.toISOString();Also applies to: 101-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/lcm-event-search-tool.ts` around lines 36 - 45, Replace the local
parseTimestamp implementation with the shared parser: remove parseTimestamp and
import/use parseIsoTimestampParam instead; update all call sites in this file
(including uses for lcm_work_density and lcm_grep) to call
parseIsoTimestampParam(value, key) and return/propagate its result so timestamp
validation is consistent with the rest of the codebase.
100yenadmin
pushed a commit
that referenced
this pull request
May 4, 2026
…ions on top PR #27 (rebase/539) carried older versions of observed-work-store, work-density-tool, and migration.ts that pre-dated PR #20/#21/#22/#25/#28 hardening on release/v0.9.4. This commit takes the release/v0.9.4 versions of those files as the base and layers PR #27's NEW work on top: - KEPT (PR #27 additions, layered on release): - bulkUpsertSuggestions API (uses release's mutex routing + per-row SAVEPOINT) - getSuggestionConversationId helper - taskBridgeToolsEnabled config flag - reviewSuggestion suggestionId trim - lcm_task_suggestions + lcm_task_suggestion_review tools (gated) - TaskBridgeSuggestionStore engine wiring + getter - PORT-FORWARDED from release/v0.9.4: - src/store/observed-work-store.ts (cursor state, getItem, listItems, etc.) - src/store/event-observation-store.ts (NEW from release; wired in engine) - src/observed-work-extractor.ts (NEW from release; NEGATED_COMPLETION_RE, fingerprintVersion, clearProcessingCursor) - src/tools/lcm-work-density-tool.ts (release hardening) - src/tools/lcm-event-search-tool.ts (NEW from release) - src/db/migration.ts (release index set) - src/db/config.ts (observedWorkMaintenanceEnabled flag preserved) - src/plugin/index.ts (release gating + PR #27 task-suggestions blocks) - src/engine.ts (release ObservedWorkExtractor + EventObservationStore wiring, pendingRebuild + clearProcessingCursor on extraction failure) - test/observed-work-store.test.ts (release version aligned with new store API) - MERGED (release base + PR #27 additions): - src/store/task-bridge-suggestion-store.ts: MAX_SUGGESTION_SOURCE_IDS=50 cap, chunked source-id IN(...) lookup, ORDER BY suggestion_id ASC tiebreaker, SQLInputValue typing — all from release; bulkUpsertSuggestions, getSuggestionConversationId, "skipped" outcome, savepoint counter — from PR #27. - src/db/config.ts: both observedWorkMaintenanceEnabled (from release) and taskBridgeToolsEnabled (from PR #27). - src/plugin/index.ts: gates work-density + event-search behind observedWorkMaintenanceEnabled; gates task-suggestions tools behind taskBridgeToolsEnabled. - ADDED to openclaw.plugin.json: - lcm_event_search to contracts.tools (was missing — manifest drift guard caught it) - observedWorkMaintenanceEnabled in uiHints + configSchema Tests: 1006/1006 pass (was 992 before; release hardening added more coverage). Net new tsc errors: 0 (332 vs 342 baseline = 10 fewer).
100yenadmin
pushed a commit
that referenced
this pull request
May 4, 2026
Conflict resolution: took theirs (rebase/539 port-forwarded release + layered PR #27 on top). New tools: lcm_task_suggestions + lcm_task_suggestion_review (gated behind taskBridgeToolsEnabled flag, default off). New store API: bulkUpsertSuggestions (per-row SAVEPOINT, skipped outcome), getSuggestionConversationId (cross-conversation review prevention). Verified all PR #20-#28 hardening preserved.
Member
Author
|
Merged into |
100yenadmin
pushed a commit
that referenced
this pull request
May 4, 2026
… additions Adversarial review found PR #23 carried older versions of files now hardened on main (release/v0.9.4 = PRs #20-#28). Port-forward main as canonical, then layer PR #23-specific additions back on top. Port-forwarded from origin/main: - src/store/{observed-work-store,task-bridge-suggestion-store,event-observation-store}.ts - src/observed-work-extractor.ts - src/tools/{lcm-work-density-tool,lcm-event-search-tool,lcm-task-suggestions-tool}.ts - src/db/{migration,config}.ts - src/engine.ts - src/plugin/index.ts - openclaw.plugin.json (contracts.tools) - test/{observed-work-store,task-bridge-suggestion-store}.test.ts Hardening now present (verified by greps): - NEGATED_COMPLETION_RE in observed-work-extractor - fingerprintVersion 3 - withDatabaseTransaction (31 call sites) for transactional upsert mutex - MAX_EVENT_SOURCE_IDS / MAX_SUGGESTION_SOURCE_IDS bounds - deps.clock.now() threading - clearProcessingCursor on extractor failure (cursor + pendingRebuild) - COALESCE-on-update + status-regression CASE in observed-work upsertItem - ensureMigrated on tool-side store getters - gating: lcm_work_density / lcm_event_search behind observedWorkMaintenanceEnabled; lcm_task_suggestions* behind taskBridgeToolsEnabled PR #23 additions preserved: - ObservedWorkTransitionType + ObservedWorkTransition types - lcm_observed_work_transitions table + 2 indexes (migration step ensureObservedWorkTransitionTables) with FK cascade and CHECK constraints - ObservedWorkStore.addTransition() - ObservedWorkStore.updateItemObservation() with strict status-regression CASE - ObservedWorkStore.findActiveItemsByTopic() - ObservedWorkStore.getDensity() now supports staleAfterDays + includeTransitions - ObservedWorkItemSnapshot extended with conversationId/kind/title/topicKey/rationale - ObservedWorkDensityResult.{staleItems,transitions} optional fields - ObservedWorkDensityQuery.{includeTransitions,staleAfterDays,now} Build clean. Tests: 1008 passed (53 files).
100yenadmin
pushed a commit
that referenced
this pull request
May 4, 2026
…eering#530) PR #23 incremental work on top of main (which already has PR #20-#28 hardening). Adds: - ObservedWorkStore.addTransition + getTransitionsForWorkItems (LIMIT 50 per item via ROW_NUMBER) - ObservedWorkStore.updateItemObservation with stricter status-regression CASE - ObservedWorkStore.findActiveItemsByTopic - ObservedWorkStore.getDensity supports staleAfterDays + includeTransitions + clock-threaded now - lcm_observed_work_transitions table + 2 indexes (item+time, type+time) - ObservedWorkTransitionType union (created/reinforced/completion_observed/ambiguity_increased/dismissed/decision_recorded) - Extended ObservedWorkItemSnapshot with conversationId/kind/title/topicKey/rationale - ObservedWorkDensityResult.staleItems + transitions optional output Verified: all PR #20-#28 hardening preserved (NEGATED_COMPLETION_RE, fingerprintVersion 3, withDatabaseTransaction x 31 sites, MAX_*_SOURCE_IDS 50, deps.clock.now, clearProcessingCursor, COALESCE + status-regression in upsertItem, ensureMigrated on store getters, await on processConversation, tool gating). 1008 tests pass. Build clean.
100yenadmin
pushed a commit
that referenced
this pull request
May 4, 2026
PR #26 incremental work on top of main (which has PR #20-#28 + #23 hardening). Adds: - lcm_event_episodes + lcm_event_episode_observations tables + indexes - EventEpisode type + rebuildEpisode + upsertEpisodeFromObservation methods on EventObservationStore - Episode upsert wired into existing withDatabaseTransaction in upsertObservation - listEpisodes API for query tools - MAX_PERSISTED_EPISODE_SOURCES = 40 cap on episode source_ids JSON - 6 new tests covering grouping, filtering, re-bucketing, and source dedup Verified: all PR #20-#28 + #23 hardening preserved (NEGATED_COMPLETION_RE, fingerprintVersion 3, withDatabaseTransaction routing, MAX_*_SOURCE_IDS, deps.clock.now, clearProcessingCursor, COALESCE upsertItem, status-regression CASE, ensureMigrated on store getters, await processConversation, tool gating, canonicalizeIsoTimestamp).
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.
Summary
Inverse-merge rebase of upstream Martian-Engineering#540 onto our
main(which carries 8 hotfix PRs Martian-Engineering#572-Martian-Engineering#579 plus PR Martian-Engineering#516). Strategy: start frommain, layer pr-540's NEW code on top so no hotfix is lost.What landed from pr-540
observed-work-store.ts,event-observation-store.ts,task-bridge-suggestion-store.tslcm_work_density,lcm_event_search(registered + listed incontracts.tools)observed-work-extractor.tsmaintain()hook inengine.tsrunning observed-work + event extraction, gated on new configobservedWorkMaintenanceEnabled(defaultfalse)ensureRollupViews:ensureObservedWorkTables/Indexes/StateCursorColumns,ensureTaskBridgeSuggestionTables/Indexes,ensureEventObservationTables/IndexesstartOfWeekDayStringhelper intimezone-windows.tsobserved-work-store.test.ts,task-bridge-suggestion-store.test.ts)Main hotfixes preserved
engine.ts:normalizeSummaryOverlapText/messageContentCoveredBySummary,rollup{Daily,Weekly,Monthly}MaxTokenswiring through RollupBuilder,computeRollupMaintenanceDaysBack(now: Date)+this.deps.clock.now()calldb/config.ts: rollup token caps + envs,DEFAULT_CRITICAL_BUDGET_PRESSURE_RATIOexportdb/migration.ts:TableColumnInfo(pr-540 used a staleSummaryColumnInfoname)openclaw.plugin.json: all hotfixuiHints/configSchemafieldsREADME.md,docs/configuration.md: rollup token docstest/engine.test.ts,test/rollup-store-builder.test.ts,src/tools/lcm-recent-tool.ts(carriescomputeAutoDetailLevel),src/rollup-builder.ts,src/store/rollup-store.ts: took mainType fix
src/store/observed-work-store.tsandsrc/store/task-bridge-suggestion-store.tshadargs: unknown[]arrays passed todb.prepare(...).get(...args). node:sqlite typings on this main branch requireSQLInputValue[]. Fixed in both.Validation
grep -rn '<<<<<<\|>>>>>>')npm run buildcleannpm test-> 997 tests passed across 53 filesTest plan
engine.tsunion (especially the maintain() hook + clock.now())observedWorkMaintenanceEnabled=falsedefault keeps behavior on main unchangedSummary by CodeRabbit
Release Notes
New Features
lcm_work_densitytool for work-density analytics and progress summarieslcm_event_searchtool for deterministic event observation searchDocumentation
observedWorkMaintenanceEnabledoptionConfiguration
observedWorkMaintenanceEnabledsetting to enable observed-work extraction during maintenance