Severity: Trivial (bundled housekeeping — none of these changes behavior for users)
Category: Code quality / micro-performance / documentation
Affected files: src/client/main.ts, src/server/logTailer.ts, README.md. Line numbers refer to main @ 3ac64da.
This bundles the remaining minor observations from the 2026-07 security & performance review (REVIEW-FINDINGS.md on branch claude/code-security-performance-review-kwgq0v) so they don't linger unfixed. Each item is independent; implement all three in one PR.
Item 1 — Dead displayedLines variable in the SSE log-line handler
src/client/main.ts, line 479:
let displayedLines: number | null = null;
The variable is declared in the log-line event handler, never assigned, and then passed into the perf-timing details at line 486 (completeSseTiming({ …, displayedLines, … })). It is always null — and necessarily so, because the actual render is deferred to a requestAnimationFrame batch (scheduleLiveRender), so the displayed count cannot be known at this point.
Fix: delete the declaration and remove the displayedLines key from that one completeSseTiming call. Do not try to compute a real value here — the render-side metric (render:log-lines timing, which already records displayedLines) is the correct place and already exists. No other call sites are affected; PerformanceMetricDetails values are optional.
Item 2 — Newline counting iterates the Buffer byte-by-byte in JS
src/server/logTailer.ts, lines 373–377 (inside readLastLines):
for (const byte of chunk) {
if (byte === 0x0a) {
newlineCount += 1;
}
}
A for…of over a Buffer runs one JS iteration per byte. readLastLines scans up to MAX_TAIL_SCAN_BYTES (8 MB) on bootstrap, reattach, and skip-ahead — that's up to ~8 million JS iterations per call on the worst case (a tail with few newlines). Buffer.prototype.indexOf does the same scan in native code.
Fix: replace the loop with:
let searchIndex = chunk.indexOf(0x0a);
while (searchIndex !== -1) {
newlineCount += 1;
searchIndex = chunk.indexOf(0x0a, searchIndex + 1);
}
Semantics are identical (counts \n bytes in the raw chunk; 0x0a never appears as a UTF-8 continuation byte, as the existing comment above the loop explains — keep that comment). Impact is limited to bootstrap/reattach/skip-ahead paths, hence "trivial", but it is free to take.
Item 3 — Document that JS/CSS source maps are intentionally served
scripts/build.mjs builds both bundles with sourcemap: true, and express.static serves dist/client/main.js.map (the server-side dist/server/index.cjs.map is not exposed, as express.static only serves dist/client). For an MIT-licensed open-source project this is intentional and useful for debugging, but it deserves one sentence of documentation so it isn't mistaken for an oversight in future reviews.
Fix: add a short note to README.md (e.g. in the build/deployment section): client source maps are built and served on purpose to ease in-browser debugging; the source is public anyway. No code change.
Not included here (already tracked elsewhere)
Acceptance criteria
displayedLines no longer exists in the log-line handler; client tests pass.
readLastLines produces identical results for: file ending with/without trailing newline, CRLF input, a file with zero newlines larger than one 64 KB chunk (existing tests in logTailer.test.ts should already cover most of this — run them).
- README contains the source-map note.
npm test and npm run typecheck pass.
Severity: Trivial (bundled housekeeping — none of these changes behavior for users)
Category: Code quality / micro-performance / documentation
Affected files:
src/client/main.ts,src/server/logTailer.ts,README.md. Line numbers refer tomain@3ac64da.This bundles the remaining minor observations from the 2026-07 security & performance review (
REVIEW-FINDINGS.mdon branchclaude/code-security-performance-review-kwgq0v) so they don't linger unfixed. Each item is independent; implement all three in one PR.Item 1 — Dead
displayedLinesvariable in the SSElog-linehandlersrc/client/main.ts, line 479:The variable is declared in the
log-lineevent handler, never assigned, and then passed into the perf-timing details at line 486 (completeSseTiming({ …, displayedLines, … })). It is alwaysnull— and necessarily so, because the actual render is deferred to arequestAnimationFramebatch (scheduleLiveRender), so the displayed count cannot be known at this point.Fix: delete the declaration and remove the
displayedLineskey from that onecompleteSseTimingcall. Do not try to compute a real value here — the render-side metric (render:log-linestiming, which already recordsdisplayedLines) is the correct place and already exists. No other call sites are affected;PerformanceMetricDetailsvalues are optional.Item 2 — Newline counting iterates the Buffer byte-by-byte in JS
src/server/logTailer.ts, lines 373–377 (insidereadLastLines):A
for…ofover aBufferruns one JS iteration per byte.readLastLinesscans up toMAX_TAIL_SCAN_BYTES(8 MB) on bootstrap, reattach, and skip-ahead — that's up to ~8 million JS iterations per call on the worst case (a tail with few newlines).Buffer.prototype.indexOfdoes the same scan in native code.Fix: replace the loop with:
Semantics are identical (counts
\nbytes in the raw chunk;0x0anever appears as a UTF-8 continuation byte, as the existing comment above the loop explains — keep that comment). Impact is limited to bootstrap/reattach/skip-ahead paths, hence "trivial", but it is free to take.Item 3 — Document that JS/CSS source maps are intentionally served
scripts/build.mjsbuilds both bundles withsourcemap: true, andexpress.staticservesdist/client/main.js.map(the server-sidedist/server/index.cjs.mapis not exposed, asexpress.staticonly servesdist/client). For an MIT-licensed open-source project this is intentional and useful for debugging, but it deserves one sentence of documentation so it isn't mistaken for an oversight in future reviews.Fix: add a short note to
README.md(e.g. in the build/deployment section): client source maps are built and served on purpose to ease in-browser debugging; the source is public anyway. No code change.Not included here (already tracked elsewhere)
snapshotLinescomputation in/api/resync(src/server/routes.ts:82) is covered by /api/resync materializes the entire buffer before applying the limit (O(maxBufferedLines) per request) #134 and must not be duplicated in this issue.Acceptance criteria
displayedLinesno longer exists in thelog-linehandler; client tests pass.readLastLinesproduces identical results for: file ending with/without trailing newline, CRLF input, a file with zero newlines larger than one 64 KB chunk (existing tests inlogTailer.test.tsshould already cover most of this — run them).npm testandnpm run typecheckpass.