Use query range to solve hanging video overlay generations

[rafaellehmkuhl]

Generating the telemetry overlay used to scan the whole temporary log store (load every key, then read each in-range point one by one), so it got slower as the store grew — eventually hanging/crashing during video processing.

Now the temporary store uses IndexedDbStore and reads only the recording's time window via a native getAll(IDBKeyRange) range query. Cost scales with recording length instead of total store size. Point-filtering logic and output are unchanged.

Fix #2844.

[github-actions]

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to consider: 1.1, 1.2

This PR replaces the localforage-backed temporary log store with the project's own IndexedDbStore, enabling a native IDBKeyRange range query via getAll() to read only the recording's time window. This eliminates the previous O(total-store-size) scan (load all keys, then getItem each in-range key sequentially) that could hang or crash during video processing when the temporary store was large. The filtering logic and output format are unchanged.

---

1. Correctness & Implementation Bugs

1.1 minor — Lexicographic key ordering assumes fixed-width epochs

The new comment (diff line 77) correctly notes "fixed-width 13-digit epochs", and Date.now() does produce 13-digit values for the foreseeable future (until ~2286). However, if the system clock is wildly misconfigured or if old data somehow stored a shorter epoch, a key like epoch=999999999999 (12 digits) would sort after epoch=1700000000000 lexicographically (because '9' > '1'). This is unlikely in practice but the assumption is undocumented in the key-building helper.

Consider zero-padding the epoch in temporaryLogPointKey (e.g. epoch.toString().padStart(13, '0')) to make the lexicographic contract explicit and bullet-proof, or at minimum add a brief runtime assertion / guard.

1.2 minor — localforage import retained but cockpitTemporaryLogsDB no longer uses it

After this change, cockpitLogsDB is the only remaining consumer of localforage in this file (line 282). This is fine — the import is still needed — but the old cockpitTemporaryLogsDB was also a localforage instance, so if a future PR migrates cockpitLogsDB too, the import can be dropped. No action needed now; just noting for context.

(Retracted — this is informational, not a bug.)

---

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance

No negative findings — the whole point of the PR is a performance win, and it achieves it cleanly. The getAll(IDBKeyRange) call delegates filtering to IndexedDB's B-tree cursor, replacing O(N) key enumeration + O(K) sequential getItem calls with a single native range read. ✅

5. UI / UX — ✅

6. Code Quality & Style — ✅

7. Commit Hygiene — ✅

8. Tests

8.1 minor — No test coverage for generateLog or the range query path

The generateLog method is the critical path changed here. There are no unit tests for it (before or after this PR). While IndexedDB is hard to test without a browser environment, the filtering logic (the forEach callback that validates log points and excludes out-of-range ones) is pure logic that could be extracted and tested. Not a blocker for this PR, but worth noting.

9. Documentation — ✅

10. Nitpicks / Optional

10.1 nit — The temporaryLogPointKey helper and its temporaryLogPointKeyPrefix constant are only used in two places (write in startLogging, read in generateLog). The helper is a nice touch for DRY, but the prefix constant temporaryLogPointKeyPrefix is never used independently of the helper — it could be inlined into the function body to reduce top-level surface area.

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 1

Done

• src/libs/sensors-logging.ts (1.1 — lexicographic ordering assumes fixed-width epochs): temporaryLogPointKey now zero-pads the epoch to 13 digits (padStart(13, '0')), so keys sort chronologically even if the clock is set before 2001. Backward-compatible — a no-op for all real 13-digit epochs, so it still matches existing stored keys.
• src/libs/sensors-logging.ts (10.1 — unused prefix constant): inlined temporaryLogPointKeyPrefix into the helper. Also de-duplicated the lexicographic-ordering rationale so it lives only on the helper, with generateLog pointing at it.

Won't change (with reasoning)

• 1.2 — retained localforage import: retracted in the review; cockpitLogsDB still uses it, so no action.

Deferred

• 8.1 — no test coverage for generateLog/range path: there's no IndexedDB test harness in the repo today and this PR is a focused perf fix. Extracting the pure point-filter into a testable function is reasonable as a follow-up rather than expanding scope here.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Re-review 2 (Claude)

Comparing 5bacb3a8823cf8c08dbc547daac3bdac973bc261 → 19787f7dbd76c9fce60d6c6f5222b168ea370e10

The previous two commits were force-pushed/squashed into a single commit, so the incremental diff covers the full PR change. Re-checked against pr.diff.

Previous findings status

#	Finding	Severity	Status
1.1	Lexicographic key ordering assumes fixed-width epochs — suggested zero-padding	minor	✅ Addressed
1.2	localforage import retained (retracted as informational)	minor	⚪ No longer applicable
8.1	No test coverage for generateLog / range query path	minor	:large_yellow_circle: Partially addressed (acknowledged and deferred)
10.1	temporaryLogPointKeyPrefix constant could be inlined	nit	✅ Addressed

Discussion since last review

@rafaellehmkuhl posted a follow-up comment confirming: 1.1 and 10.1 are done, 1.2 was already retracted, and 8.1 is deferred ("there's no IndexedDB test harness in the repo today and this PR is a focused perf fix"). Agreed — deferring 8.1 is reasonable for this scope.

New findings

0. Summary

Verdict: READY TO MERGE

This PR replaces the localforage-backed temporary log store with the project's own IndexedDbStore, enabling a native IDBKeyRange range query via getAll() to read only the recording's time window instead of scanning the entire store. The previous review's actionable suggestions (zero-padded epoch keys, inlined prefix constant) have been addressed, and the two commits were squashed into a single clean perf: commit. No new issues found.

1. Correctness & Implementation Bugs — ✅

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance — ✅

5. UI / UX — ✅

6. Code Quality & Style — ✅

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Re-review 3 (Claude)

Comparing 19787f7dbd76c9fce60d6c6f5222b168ea370e10 → 6029e8ba0cef0381a4c05a18c233260ac65b6b0a

The previous single squashed commit was split into two logical commits (refactor + perf). The net code change is identical to what was reviewed in seq 2; the incremental diff covers the full PR diff because the old single commit was replaced.

Previous findings status

#	Finding	Severity	Status
1.1	Lexicographic key ordering assumes fixed-width epochs — suggested zero-padding	minor	✅ Addressed
1.2	localforage import retained (retracted as informational)	minor	⚪ No longer applicable
8.1	No test coverage for generateLog / range query path	minor	:large_yellow_circle: Partially addressed (deferred — no IndexedDB test harness in repo)
10.1	temporaryLogPointKeyPrefix constant could be inlined	nit	✅ Addressed

New findings

0. Summary

Verdict: READY TO MERGE

This PR replaces the localforage-backed temporary log store with the project's own IndexedDbStore, enabling a native IDBKeyRange range query to read only the recording's time window instead of scanning the entire store. The only change since the previous review is that the single squashed commit was split into two well-structured commits (refactor: backing-store swap, then perf: range-query implementation), which improves commit hygiene. No new issues found.

1. Correctness & Implementation Bugs — ✅

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance — ✅

5. UI / UX — ✅

6. Code Quality & Style — ✅

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

Generated by Claude. This is advisory; a human reviewer must still approve.