feat: [ENG-2177] Increase Storage Limit for Curate Log and Query Log

[bao-byterover]

No description provided.

[github-actions]

Claude finished @bao-byterover's task in 2m 4s —— View job

---

PR Review: feat: [ENG-2177] Increase Storage Limit for Curate Log and Query Log

• Gather context and read changed files
• Analyze implementation changes
• Review test coverage
• Post detailed review

---

Summary

A minimal, focused change: bumps the default MAX_ENTRIES constant from 100→1000 in FileCurateLogStore and 200→1000 in FileQueryLogStore, and adds boundary-condition tests to confirm the old limits no longer trigger pruning while the new ones do.

Score: 4 / 5

The implementation is correct and the test strategy (seed files directly on disk, then assert file counts and getById outcomes at/above the boundary) is well-chosen — it avoids the slow overhead of saving 1000 entries through the store. Two minor issues and one stale doc comment prevent a perfect score.

---

Issues Found

Stale JSDoc comment (not in diff — manual fix needed)

src/server/infra/storage/file-query-log-store.ts line 89 still reads:

 * Writes are atomic (tmp → rename). Reads validate with Zod and return undefined
 * for corrupt/missing files. Prunes by age (default 30 days) then by count (default 200).

Should be updated to default 1000. Fix this →

Test numbering out of order

In test/integration/infra/storage/file-query-log-store.test.ts, the new test is labeled // Test 21 but is placed before the existing // Test 20. Renumber or reorder so the sequence is readable. (Inline comment posted.)

Type assertions in curate-log test

test/unit/infra/storage/file-curate-log-store.test.ts uses as Partial<CurateLogEntry> & {id: string} three times (lines 401, 414, 425). Per project TypeScript standards, as Type casts should be avoided. Consider either adjusting makeEntry's signature to accept partial inputs without casting, or constructing the object to satisfy the type directly. (Inline comment posted.)

---

Positive Notes

• Correct pruning logic untouched: The pruneOldest implementations in both stores are files.length - maxEntries inclusive, so the boundary check (<= maxEntries) is correct. The tests exercise exactly the off-by-one scenarios (at-limit: no prune; at-limit+1: one file pruned).
• Age-pruning awareness: The query-log test correctly uses Date.now() - 500_000 (≈8 min ago) as baseTs to stay well within the 30-day age window, avoiding false failures from the age-prune phase.
• pruneInFlight guard in FileQueryLogStore: The deduplication guard means that only one prune pass runs at a time, which is important when 1000-entry volumes could trigger rapid concurrent saves.
• Atomic writes preserved: The implementation change touches only the constant; the safe tmp→rename write path is unchanged.

[github-actions]

nitpick: Test label is out of order — // Test 21 appears before // Test 20 in the file. Either renumber this one to // Test 21 at the end (after Test 20), or move it to its logical position. Minor cosmetic issue, but makes the numbering confusing to future readers.

[github-actions]

nitpick: The as Partial<CurateLogEntry> & {id: string} type assertion is used three times in this test. Per the project's TypeScript standards, as Type assertions should be avoided — prefer a typed helper or constructing the object to match the expected shape directly. The existing makeEntry helper presumably handles this; check if its signature can be relaxed to accept the needed fields without assertion.

[github-actions]

Claude finished @bao-byterover's task in 2m 11s —— View job

---

PR Review: feat: [ENG-2177] Increase Storage Limit for Curate Log and Query Log

• Gather context and read changed files
• Analyze implementation changes
• Review test coverage
• Check if previous review issues were addressed
• Post detailed review

---

Summary

This PR bumps DEFAULT_MAX_ENTRIES from 100→1000 in FileCurateLogStore and 200→1000 in FileQueryLogStore, and adds boundary-condition integration/unit tests validating the old thresholds no longer trigger pruning while the new limit does.

Score: 5 / 5

All three issues flagged in the previous review have been resolved in the follow-up commit (fa39a45). The implementation remains minimal and correct.

---

Previous Review Issues — All Resolved ✅

Issue	Status
Stale JSDoc comment (default 200) in file-query-log-store.ts:85	✅ Fixed — now reads default 1000
Test numbering out of order (// Test 21 before // Test 20)	✅ Fixed — // Test 20 now correctly precedes // Test 21
as Partial<CurateLogEntry> & {id: string} assertions on three call sites	✅ Fixed — replaced with clean makeEntry({id, startedAt}) calls matching the helper's actual signature

---

Implementation Verification

src/server/infra/storage/file-curate-log-store.ts

// line 71
const DEFAULT_MAX_ENTRIES = 1000  // was 100 ✓

src/server/infra/storage/file-query-log-store.ts

// line 67
const DEFAULT_MAX_ENTRIES = 1000  // was 200 ✓

// line 85 (JSDoc)
// Prunes by age (default 30 days) then by count (default 1000). ✓

All pruning logic is untouched — only the constant changed. The pruneOldest implementations in both stores use files.length - maxEntries with a <= maxEntries boundary, so the tests correctly exercise the off-by-one cases.

---

Test Strategy

The tests seed files directly on disk (bypassing the store's save() path) to avoid the overhead of writing 1000 entries through the service layer — smart and fast. Three phases per test:

• Phase 1 — seed at old limit → no pruning (confirms backward safety)
• Phase 2 — seed at new limit (1000) → no pruning at boundary
• Phase 3 — save entry #1001 → exactly one file pruned, oldest entry removed

The curate-log unit test correctly uses a fixed historical timestamp (1_700_000_000_000) since FileCurateLogStore has no age-based pruning. The query-log integration test correctly uses Date.now() - 500_000 (~8 min ago) to stay well within the 30-day age-prune window.

---

Minor Observation (pre-existing, not blocking)

The makeEntry helper at test/unit/infra/storage/file-curate-log-store.test.ts:19 itself retains a as CurateLogEntry assertion — but this is pre-existing code, not introduced by this PR, and is a reasonable convenience for a discriminated union whose full shape is known at the helper level.

---

Positive Notes

• Atomic tmp → rename write path is unchanged
• pruneInFlight deduplication guard remains intact for concurrent high-volume saves
• Separation of maxEntries into a constructor option means tests can verify limits independently of the production default
• No unnecessary refactoring beyond the stated scope