Fix log slowdown, add scroll to bottom button, upload build artifact for testing

[h2zero]

Summary by CodeRabbit

• New Features

  • Scroll-to-bottom button in the serial monitor
  • Clickable file/line links and richer crash output formatting

• Changed / Improved

  • Batched serial data updates and reduced DOM work for smoother rendering
  • Faster serial decoding and smoother autoscroll using RAF-based coordination

• Bug Fixes

  • Better cleanup on disconnect and guarded autoscroll to avoid race conditions
  • Fixed high-baud performance, line-buffer trimming, and post-disconnect message issues

• Chores

  • VSIX artifact uploaded as a CI build artifact; publish workflow restored and corrected

[coderabbitai]

Warning

Rate limit exceeded

@Jason2866 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 6 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 499d5e98-31d0-4e00-9ac4-48bec38d3203

📥 Commits

Reviewing files that changed from the base of the PR and between 044f142 and 7e09964.

📒 Files selected for processing (1)

• src/webviewPanel.ts

📝 Walkthrough

Walkthrough

Batches serial data in the extension and sends merged payloads to the webview; adds a scroll-to-bottom UI and RAF-based autoscroll guards; cancels pending serial flushes on disconnect/dispose; updates GitHub Actions to upload the compiled VSIX and make publish depend on build; adds CHANGELOG for v0.6.0.

Changes

Cohort / File(s)	Summary
CI Workflow ​.github/workflows/build.yml	Add upload of *.vsix artifact in build; make publish depend on build, download the artifact in publish, and use npx @vscode/vsce publish --packagePath *.vsix -p ${{ secrets.VSCE_TOKEN }}.
Changelog / Docs CHANGELOG.md	Add v0.6.0 (2026-03-09) entry documenting scroll-to-bottom UI, CI artifact upload, restored publish flow, performance improvements, and multiple fixes.
Serial Monitor & Webview src/webviewPanel.ts	Introduce buffered serial handling: pendingSerialData, serialFlushTimer, SERIAL_FLUSH_INTERVAL_MS, flushSerialData() and cancelSerialFlush(); replace per-chunk posts with batched posts; on disconnect cancel and discard pending buffer; switch line trimming from shift to slice; add scroll-to-bottom button, programmaticScroll and scrollRAFPending guards and RAF-based scrolling; update DOM trimming, linkify/annotation formatting, and ensure cleanup on dispose.

Sequence Diagram(s)

mermaid
sequenceDiagram
rect rgba(100,149,237,0.5)
participant SerialDevice as Serial Device
end
rect rgba(60,179,113,0.5)
participant Extension as Extension Host
end
rect rgba(255,165,0,0.5)
participant Webview as Webview UI
end

SerialDevice->>Extension: chunked serial data (onData)
Extension->>Extension: append to pendingSerialData; schedule flush timer
Note right of Extension: multiple chunks accumulate
Extension->>Webview: flushSerialData() — single `serialData` IPC with batched payload
Webview->>Webview: append to DOM, trim lines via slice, linkify/annotate
Webview->>Webview: if autoscroll enabled -> requestAnimationFrame scroll (set programmaticScroll)
Webview-->>Extension: user scroll or disconnect -> toggle autoscroll / trigger cancelSerialFlush

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Refactor #1 — Touches src/webviewPanel.ts and overlaps on disposal/cleanup and decodeCrash-related runtime flows.

Poem

🐰
I nibble bytes and batch the flow,
So render storms move soft and slow,
A bottom button beckons bright,
RAF guards keep the scroll aright,
The decoder hums till morning's glow.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the three main changes: fixing log slowdown, adding a scroll-to-bottom button, and uploading build artifacts—all reflected in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

.github/workflows/build.yml (1)

32-36: Use the uploaded VSIX as the publish input.

publish now waits for build, but it still packages a fresh extension in its own job. That means the Marketplace upload can diverge from the esp-decoder-vsix artifact reviewers download from CI, especially with the current build step deleting package-lock.json before install. Reusing the uploaded artifact here would keep test and release output aligned.

Also applies to: 39-43

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build.yml around lines 32 - 36, The publish job is
re-building the VSIX instead of reusing the uploaded artifact, causing possible
divergence; modify the workflow so the job that currently recreates the package
uses the previously uploaded artifact named "esp-decoder-vsix" (uploaded via the
"Upload vsix artifact" step using actions/upload-artifact@v4) as its input;
update the "publish" job to add a download-artifact step
(actions/download-artifact) to fetch "esp-decoder-vsix" and use that VSIX for
Marketplace upload instead of running the packaging steps that delete
package-lock.json and repack, ensuring the artifact reviewers download matches
what is published.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/webviewPanel.ts`:
- Around line 271-281: The cancelSerialFlush method currently clears the timer
and resets pendingSerialData but doesn't reset the lineBuffer, so buffered bytes
can still be flushed later; modify cancelSerialFlush to also zero/reset
lineBuffer (the same way the existing clear logic does) or call the shared reset
logic used by clear so both pendingSerialData and lineBuffer are cleared and
serialFlushTimer is nulled in cancelSerialFlush.
- Around line 1244-1251: The current DOM trimming uses childNodes.length which
no longer reflects rendered log size; update the trimming logic in
webviewPanel.ts (the block manipulating serialOutput.replaceChildren(...keep))
to enforce a line/character budget instead: compute the total lines/characters
represented by serialOutput.childNodes (or, better, consult the already-trimmed
buffer like serialMonitor.maxLines / the serial monitor's line window if
available), then drop oldest nodes until the budget is satisfied and call
replaceChildren with the remaining nodes; ensure you measure per-node line
counts (or character counts) so a single large node can't bypass the limit and
keep references to symbols serialOutput and serialMonitor.maxLines (or the
monitor's trimmed line buffer) when locating and replacing the logic.
- Around line 123-128: Introduce a connection-generation token (e.g.,
this.serialGen) that is incremented whenever onConnectionChange(false) runs;
capture the current generation at the start of handleSerialData() and
flushSerialData() and ignore any data/timers whose captured generation does not
match the current this.serialGen so late onData callbacks are dropped. In
handleSerialData() store the generation with any buffered work and only
schedule/execute flushSerialData() if the generation matches; in
flushSerialData() verify the generation before sending to the webview. Also
update the "clear" message handler to reset pendingSerialData, clear
serialFlushTimer, and bump the generation so cleared sessions cannot be
repopulated by delayed callbacks. Ensure all references use the existing
method/field names (handleSerialData, flushSerialData, pendingSerialData,
serialFlushTimer, onConnectionChange/clear handler) so the guards are applied
consistently.

---

Nitpick comments:
In @.github/workflows/build.yml:
- Around line 32-36: The publish job is re-building the VSIX instead of reusing
the uploaded artifact, causing possible divergence; modify the workflow so the
job that currently recreates the package uses the previously uploaded artifact
named "esp-decoder-vsix" (uploaded via the "Upload vsix artifact" step using
actions/upload-artifact@v4) as its input; update the "publish" job to add a
download-artifact step (actions/download-artifact) to fetch "esp-decoder-vsix"
and use that VSIX for Marketplace upload instead of running the packaging steps
that delete package-lock.json and repack, ensuring the artifact reviewers
download matches what is published.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 54efc318-0f62-4cc0-a48c-4fe0fbdfaaf5

📥 Commits

Reviewing files that changed from the base of the PR and between 1153eb2 and 3ed9a93.

📒 Files selected for processing (3)

• .github/workflows/build.yml
• CHANGELOG.md
• src/webviewPanel.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/build.yml:
- Around line 32-36: The publish job is rebuilding the VSIX instead of using the
CI-produced artifact; update the publish job to download the previously uploaded
artifact (matching the name used in the build job: esp-decoder-vsix) via
actions/download-artifact@v4 and call vsce publish with the --packagePath
pointing to the downloaded esp-decoder-*.vsix (so the publish step consumes the
exact artifact uploaded by the "Upload vsix artifact" step rather than
re-running the package/build steps).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 88778554-21df-4740-ac40-279a7d234936

📥 Commits

Reviewing files that changed from the base of the PR and between 3ed9a93 and 8fc796c.

📒 Files selected for processing (1)

• .github/workflows/build.yml

[Jason2866]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[Jason2866]

@coderabbitai What do you think about:

DOM trimming (line 1244): The 10,000-node cap is a reasonable DOM safety limit; the actual line budget is enforced server-side via serialLines/maxLines. No change needed.
Connection-generation token (line 123): No real race condition exists — cancelSerialFlush synchronously clears the timer and buffers on disconnect, and Node.js event loop guarantees no interleaving.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me inspect the relevant code sections before forming an opinion.

---

🏁 Script executed:

#!/bin/bash
# Read lines around 1244 and 123 in src/webviewPanel.ts
echo "=== Lines 115-135 (connection-generation token area) ==="
sed -n '115,135p' src/webviewPanel.ts

echo ""
echo "=== Lines 1235-1260 (DOM trimming area) ==="
sed -n '1235,1260p' src/webviewPanel.ts

echo ""
echo "=== cancelSerialFlush and flushSerialData implementations ==="
grep -n "cancelSerialFlush\|flushSerialData\|serialFlushTimer\|pendingSerialData\|handleSerialData\|serialLines\|maxLines" src/webviewPanel.ts | head -60

Length of output: 3241

---

@Jason2866 Both arguments hold up well after inspecting the code.

DOM trimming (line 1248):
The two-tier design is sound. The server-side serialLines budget (default 5,000 via maxLines) is the primary enforcer — it slices the array and only ships the trimmed batch to the webview. The 10,000-node DOM cap is a pure safety backstop at 2× that default, catching edge cases such as a user raising maxLines above 5,000 or rapid bursts before a trim cycle runs. Using a single replaceChildren() call also keeps it to one layout invalidation rather than one per removed node, so the cap is both safe and efficient. No change needed.

Connection-generation token (line 123):
The synchronous invariant is solid. cancelSerialFlush() (lines 276–281) runs clearTimeout and zeroes pendingSerialData atomically within the same event-loop callback that handles the disconnect. Node.js is single-threaded — the setTimeout callback for flushSerialData cannot fire in the middle of another running callback. By the time the event loop could ever dispatch a pending flush, cancelSerialFlush has already killed both the timer handle and the buffer. A generation/token guard would add bookkeeping complexity without closing any real race window.

Both assessments are well-reasoned — agreed on both counts.

---

✏️ Learnings added

Learnt from: Jason2866
URL: https://github.com/Jason2866/ESP-Decoder/pull/2

Timestamp: 2026-03-10T00:30:08.997Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the DOM trimming 10,000-node cap is intentional as a safety backstop (2× the default server-side maxLines of 5,000). The primary line budget is enforced server-side via serialLines/maxLines. No DOM-side cap change is needed.

---

Learnt from: Jason2866
URL: https://github.com/Jason2866/ESP-Decoder/pull/2

Timestamp: 2026-03-10T00:30:08.997Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), there is no real race condition around the serial flush timer on disconnect. cancelSerialFlush() synchronously clears the timer (clearTimeout) and wipes pendingSerialData in the same event-loop tick as the disconnect callback. Node.js single-threaded guarantees prevent any interleaving, so a connection-generation token is unnecessary.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/webviewPanel.ts`:
- Around line 1489-1498: The linkifyPaths function currently escapes the whole
text before running the file-path regex, which causes captured file paths (used
for data-file) to contain HTML entities and break file opening; update
linkifyPaths to run the regex against the raw input first to capture the real
file path (from the original text) and only escape values when inserting into
the DOM attributes and visible text (use escapeAttr for data-file/data-line and
escapeHtml for displayed shortFile:line), or perform a two-pass approach:
replace path matches in the raw text with placeholders wrapped in properly
escaped HTML while escaping only the non-matched portions. Ensure this change is
made in the linkifyPaths function and continue to use escapeAttr/escapeHtml for
their respective outputs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fed360b1-4f23-4a50-b8a2-8ee53c54364d

📥 Commits

Reviewing files that changed from the base of the PR and between 8fc796c and 044f142.

📒 Files selected for processing (2)

• .github/workflows/build.yml
• src/webviewPanel.ts