fix: share one exact content-analysis cutoff (#99) - #121

🤖 Automated first-pass review

TL;DR

The change is coherent and well-motivated: sharing one stepped cutoff between ClaudeDataLoader and updateClaudeUsageIndex keeps the two producers aligned, and the cost analysis (per-step re-read instead of per-event) is sound. Two merge-readiness gaps stand out: the full loader's comparison test in the new case uses a real Date.now() while the incremental path uses the stubbed clock, and there is no CHANGELOG.md entry for a user-visible behavior change. Neither is fatal, but both should be resolved before merge.

Analysis

1. Test clock asymmetry in the new test (src/test/claudeIncrementalIndex.test.ts). updateClaudeUsageIndex is called after Date.now = () => now, but ClaudeDataLoader.loadUsageRecords is called in the same block. If the loader reads Date.now() internally (which the diff shows it does, via analysisWindowCutoffMs(Date.now(), windowDays)), it will use the stubbed value only because the stub is global — worth confirming explicitly that the assert.deepEqual(warm.contentAnalysis, full.contentAnalysis) comparison actually exercises the stepped cutoff and not two real-clock readings. If the stub is restored or Date.now is read before the assignment in any code path, this comparison silently becomes a no-op. Prefer a deterministic assertion (or freeze the clock for the whole body) so the equality check cannot pass trivially.

2. Contract change needs a CHANGELOG.md entry. AGENTS.md says user-facing changes require a CHANGELOG.md entry and matching documentation. The PR description explicitly states the full loader's contract changes from a continuous to an hourly cutoff, and the analysis window widens by up to one hour. That is user-visible (an event up to ~1 h older than windowDays can now be included) and should be called out. No CHANGELOG.md or README change appears in the diff.

3. Independence from perf: a file already outside the window keeps its empty contribution (#99) #119. The PR body is candid that the fast-path numbers depend on perf: a file already outside the window keeps its empty contribution (#99) #119 also being applied, and that without it the two changes are not sufficient. This is fine as a stacked PR but should be stated clearly in the merge checklist so a reviewer does not apply fix: share one exact content-analysis cutoff (#99) #121 alone and conclude the fast path is broken. Confirm the intended merge order (likely perf: a file already outside the window keeps its empty contribution (#99) #119 first, or both together) and that CI on this branch reflects that combination.

4. Four existing tests were adjusted (12:00 → 13:01, 16:00:30 → 17:00:30). The tweaks preserve what each test checks, but they shift the boundary by one step; if any of those tests was specifically verifying "an event exactly at the cutoff boundary", the new time may no longer exercise the exact boundary. Spot-check that the boundary semantics (inclusive vs. exclusive at the floored step) are still covered somewhere, ideally by a focused assertion.

5. analysisWindowCutoffMs is exported and used in two modules. That is the right shape. Consider a brief note in ARCHITECTURE.md near the claudeIncrementalIndex.ts row, since the module map says that module owns "content-analysis contributions" and the cutoff step is now a shared contract between two modules. Small, optional.

6. Local Windows note. The PR body flags two test names failing on main on NTFS due to file-identity reuse and passing in CI. That is a pre-existing condition, not introduced here, but worth confirming the new test does not join that set on Windows — the new test writes two fixed filenames and asserts bodyReads === 0, which is exactly the pattern the noted failures involve. Verify on Windows or note the expected behavior.

7. Step constant. ANALYSIS_CUTOFF_STEP_MS = 60 * 60 * 1000 is exported as "the one knob". Good. Make sure nothing else in the codebase computes an analysis window independently (a grep for windowDays * 24 * 60 * 60 * 1000 or similar) so the single-knob claim holds.

Suggested next step(s)

• Add a CHANGELOG.md entry describing the stepped analysis cutoff and the up-to-one-hour window widening.
• Harden the new test's clock handling so ClaudeDataLoader is deterministically evaluated under the same stubbed time, or explicitly assert the cutoff value on both sides.
• Confirm and document merge order with perf: a file already outside the window keeps its empty contribution (#99) #119 (stacked or combined).
• Grep for other window computations to validate the single-knob claim, and spot-check that the floor boundary semantics remain covered after the four test time shifts.
• Confirm the new test passes on Windows, or document that it is expected to pass only in CI alongside the two known NTFS-sensitive tests.

🤖 Generated by DeepSeek as an automated first pass — not a maintainer decision.

Thanks for the first pass. Addressed:

• CHANGELOG — added under Changed (d33b107): the window may now include up to one extra hour, and expired events leave in hourly batches.
• Clock in the new test — the stub is global and set before both calls, so ClaudeDataLoader.loadUsageRecords reads the same frozen clock as updateClaudeUsageIndex; nothing in the body restores it before the finally. The equality check also can't pass on two real-clock readings: the test first asserts the cutoff itself (2026-08-11T12:00:00Z for the whole step, 13:00:00Z after it), which only the stubbed clock produces.
• Merge order — perf: a file already outside the window keeps its empty contribution (#99) #119 first, then this one (or both together). Each is correct on its own and CI is green on both; the fast-path numbers in the description need both, because without perf: a file already outside the window keeps its empty contribution (#99) #119 the files already outside the window still turn the fast path off.

🤖 Generated with Claude Code

Merged after the amendment on this original branch. Thank you @rsyuzyov for profiling the boundary-file costs and extracting the shared cutoff. We retained that useful part, aligned full-loader analysis and calibration to one captured cutoff, and preserved exact millisecond expiry. The original boundary clocks and new one-millisecond/frontier regressions pass; the latest Node, UI and package CI checks are green. This is merged into main, not yet a published extension update.

🤖 Generated with OpenAI Codex