Phase 3 — Efficiency: close P3-3/P3-4/P3-5/P3-6 #1

Merged
joeyr merged 3 commits from fix/phase3-efficiency into main 2026-07-11 20:22:54 -05:00
Owner

Closes the remaining Phase 3 — Efficiency findings (P3-3, P3-4, P3-5, P3-6). With this, all 6 Phase 3 findings are resolved (P3-1/P3-2 already merged to main).

Findings closed

Finding Sev Resolution
P3-4 Low time/date context queries now carry LIMIT $N (maxContextMessages, default 100) instead of fetching every row in the window and truncating in JS. The (channel_id, timestamp DESC) index returns newest-first, so the cap keeps exactly the rows ContextManager would have kept.
P3-3 Low New setCacheBreakpoint helper places one moving cache_control: {type:"ephemeral"} breakpoint on the last message before each tool-loop request, so rounds 2+ read the prior round's prefix (tools + system + prior turns) at ~0.1× input price. Added only when tools are enabled; stale breakpoints are stripped first so there is never more than one (the API's 4-breakpoint cap is never at risk).
P3-6 Low (a) purge timestamp index left as-is by decision — a standalone index would tax the hot INSERT path to speed a once-a-day purge on a small table; (b) purge uses the driver rowCount via a new database.execute() instead of RETURNING id just to count; (c) RETENTION_DAYS is now wired to config.storage.retentionDays instead of a hardcoded 30; (d) dead getStorageStats() + StorageStats type + test mocks deleted (also closes P4-4b).
P3-5 Info Resolved to leave as-is — d2emu's 2-min poll is third-party-API courtesy, not cost, and is already env-overridable via D2EMU_POLL_INTERVAL_MS. No code change.

Commits

  • P3-4/P3-6: LIMIT context queries; wire RETENTION_DAYS; drop dead stats
  • P3-3: prompt-cache breakpoint for the admin tool loop
  • Log Phase 3 fixes in FINDINGS.md — Phase 3 closed

Verification

  • Typecheck: clean
  • Tests: 641/641 passing (47 suites), incl. new ClaudeClient.cache.test.ts pinning the cache-breakpoint invariants
  • Coverage: 50.3 / 47.88 / 48.93 / 50.44 — clears the CI floor (43/42/42/43)
  • Lint: 0 errors

Notes / deviations (documented in FINDINGS.md)

  • P3-3 done standalone rather than bundled with P4-2's ClaudeClient decomposition — the helper is self-contained and didn't need the split, so P4-2 is now pure structural cleanup.
  • P3-6d discharged P4-4b early (the StorageStats type went out with getStorageStats()).

🤖 Generated with Claude Code

Closes the remaining **Phase 3 — Efficiency** findings (P3-3, P3-4, P3-5, P3-6). With this, all 6 Phase 3 findings are resolved (P3-1/P3-2 already merged to `main`). ## Findings closed | Finding | Sev | Resolution | |---|---|---| | **P3-4** | Low | `time`/`date` context queries now carry `LIMIT $N` (`maxContextMessages`, default 100) instead of fetching every row in the window and truncating in JS. The `(channel_id, timestamp DESC)` index returns newest-first, so the cap keeps exactly the rows `ContextManager` would have kept. | | **P3-3** | Low | New `setCacheBreakpoint` helper places one moving `cache_control: {type:"ephemeral"}` breakpoint on the last message before each tool-loop request, so rounds 2+ read the prior round's prefix (tools + system + prior turns) at ~0.1× input price. Added only when tools are enabled; stale breakpoints are stripped first so there is never more than one (the API's 4-breakpoint cap is never at risk). | | **P3-6** | Low | (a) purge `timestamp` index left as-is by decision — a standalone index would tax the hot INSERT path to speed a once-a-day purge on a small table; (b) purge uses the driver `rowCount` via a new `database.execute()` instead of `RETURNING id` just to count; (c) `RETENTION_DAYS` is now wired to `config.storage.retentionDays` instead of a hardcoded 30; (d) dead `getStorageStats()` + `StorageStats` type + test mocks deleted (also closes **P4-4b**). | | **P3-5** | Info | Resolved to leave as-is — d2emu's 2-min poll is third-party-API courtesy, not cost, and is already env-overridable via `D2EMU_POLL_INTERVAL_MS`. No code change. | ## Commits - `P3-4/P3-6: LIMIT context queries; wire RETENTION_DAYS; drop dead stats` - `P3-3: prompt-cache breakpoint for the admin tool loop` - `Log Phase 3 fixes in FINDINGS.md — Phase 3 closed` ## Verification - Typecheck: clean - Tests: **641/641** passing (47 suites), incl. new `ClaudeClient.cache.test.ts` pinning the cache-breakpoint invariants - Coverage: 50.3 / 47.88 / 48.93 / 50.44 — clears the CI floor (43/42/42/43) - Lint: 0 errors ## Notes / deviations (documented in FINDINGS.md) - **P3-3 done standalone** rather than bundled with P4-2's ClaudeClient decomposition — the helper is self-contained and didn't need the split, so P4-2 is now pure structural cleanup. - **P3-6d discharged P4-4b early** (the `StorageStats` type went out with `getStorageStats()`). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
P3-4: time/date context queries fetched every row in the window and let
ContextManager truncate in JS. Both now carry a LIMIT (maxContextMessages,
default 100); the (channel_id, timestamp DESC) index returns newest-first, so
the cap keeps the messages ContextManager would have kept anyway.

P3-6:
- (b) purge no longer uses `RETURNING id` just to count. New
  database.execute() returns the driver rowCount; purgeOldMessages and
  clearTestData use it.
- (c) RETENTION_DAYS is now wired: DatabaseMessageStore reads
  config.storage.retentionDays instead of hardcoding 30.
- (d) dead getStorageStats() deleted, along with its MessageStore interface
  method, the StorageStats type (closes P4-4b), and all its test mocks.
- (a) purge timestamp index left as-is by decision: a standalone index would
  tax the hot INSERT path to speed a once-a-day purge on a small table.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The tool loop re-sends the entire growing conversation each round at full
input price; worst-case admin commands run to ~100-150K uncached input
tokens. A new setCacheBreakpoint helper places a single moving
cache_control: {type: "ephemeral"} breakpoint on the last content block of
the last message before each request, so rounds 2+ read the prefix (tools +
system + prior turns) the previous round wrote at ~0.1x input price.

The breakpoint is added only when tools are enabled — plain chat requests
never reuse the prefix, so caching would just pay the write premium. The
helper strips any stale breakpoint before setting the new one, so there is
never more than one and the API's 4-breakpoint cap is never at risk.

New ClaudeClient.cache.test.ts pins both invariants: exactly one breakpoint
per request always on the last block, and none on non-admin requests.

Done standalone rather than bundled with P4-2's ClaudeClient decomposition —
the helper is self-contained and did not need the split.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Log Phase 3 fixes in FINDINGS.md — Phase 3 closed
Some checks failed
CI / build-and-test (pull_request) Failing after 16s
CI / docker-build (pull_request) Failing after 1s
20c2da9c37
P3-3/P3-4/P3-6 fixed (see prior two commits); P3-5 resolved to leave-as-is
(d2emu poll cadence is third-party courtesy, not cost, and already
env-overridable). All 6 Phase 3 findings now closed. Updated the status
table, What's-left ordering (only Phase 4 remains), and the P4-2/P4-4
cross-references that P3-3/P3-6 partially discharged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
joeyr merged commit 5c9b6ac85c into main 2026-07-11 20:22:54 -05:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
joeyr/aufhocker!1
No description provided.