feat: Preact + TypeScript diagnostics dashboard

[strawgate]

Summary

• New dashboard/ frontend project: Preact, TypeScript, Vite
• Builds to a single 33KB inlined HTML file via vite-plugin-singlefile
• Replaces the 138-line prototype dashboard.html with a proper component architecture
• Integrated into justfile, CI, and release workflows

Dashboard architecture

dashboard/src/
  app.tsx           — top-level state + polling
  api.ts            — typed fetch client
  types.ts          — TypeScript interfaces for /api/* responses
  style.css         — dark theme, WCAG AA contrast
  components/
    StatusBar       — health pill + version
    SystemRow       — RSS, heap, batch count
    MetricBadges    — system-level metrics
    ChartGrid       — 6 Canvas time-series charts
    Chart           — single chart with Y-axis, time markers
    PipelineView    — flow diagram + click-to-expand inspector
    ConfigView      — YAML syntax highlighting
  lib/
    format.ts       — fmt, fmtBytes, fmtDuration
    ring.ts         — ring buffer for time series
    rates.ts        — EMA-smoothed rate tracker

Build integration

• Committed HTML: crates/logfwd-io/src/dashboard.html is committed so cargo build works without Node.js
• just dashboard: rebuilds from source (requires Node.js)
• just dashboard-check: CI verifies committed HTML matches source
• Release workflow: builds dashboard before cargo in each matrix job
• Dev mode: cd dashboard && npm run dev — hot reload, proxies API to :9090

Test plan

• cd dashboard && npm run build produces crates/logfwd-io/src/dashboard.html
• cargo build -p logfwd compiles with the new dashboard embedded
• just dashboard-check passes (HTML matches source)
• CI "Dashboard freshness" job passes
• Dev server: npm run dev + running logfwd → dashboard renders at localhost:5173

🤖 Generated with Claude Code

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

A new Preact-based dashboard frontend is introduced in dashboard/, featuring components for visualizing pipelines, metrics time-series, logs, and configuration. The built dashboard is embedded as crates/logfwd-io/src/dashboard.html. Supporting changes add two new diagnostics API endpoints (/api/config, /api/logs), a stderr capture module for log streaming, and platform-aware process metrics collection. CI workflows are updated to build and validate the dashboard. The main application now passes config path and YAML content to the diagnostics server.

Possibly related PRs

• feat: logfwd-core is now #![no_std] + #![forbid(unsafe_code)] #375: Directly extends the diagnostics/dashboard surface by modifying the same logfwd-io diagnostics code to support new API endpoints and dashboard integration.
• Benchmark dashboard v2: matrix CI, runtime sampling, three-section site #301: Refactors process-metrics collection in diagnostics, which is enhanced in this PR with platform-aware implementations using libc getrusage.
• feat: Async pipeline — select! loop, input threads, bounded channels (#204) #221: Modifies cmd_config and run_pipelines signatures in main.rs, overlapping with this PR's changes to pass config data to the diagnostics server.

---

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

[github-actions]

Requesting changes: dashboard/src/app.tsx has a logic bug in the version expression at line 109.

const version = pipes?.system?.version ?? stats?.uptime_sec != null ? "?" : "";

Due to operator precedence, this evaluates as a ternary over the coalesced value, so a present version string becomes truthy and still resolves to "?" instead of the actual version. Please parenthesize/split the expression so version is the actual string when present.

---

What is this? | From workflow: AI: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[coderabbitai]

Actionable comments posted: 24

🤖 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/release.yml:
- Around line 46-48: Create a new dedicated GitHub Actions job named
dashboard-build that runs the dashboard steps (cd dashboard && npm ci && npm run
build), saves the produced file crates/logfwd-io/src/dashboard.html as an
uploaded artifact, and mark it to run before the matrix build job(s); then in
the existing matrix job(s) replace the repeated build step with an artifact
download step (actions/download-artifact) to retrieve
crates/logfwd-io/src/dashboard.html so each matrix target reuses the single
built dashboard artifact instead of rebuilding it.

In `@crates/logfwd-io/src/dashboard.html`:
- Line 8: The pipeline cards are rendered as clickable divs in the Fe component
(the nodes with class names like "pn inp", "pn xfm", "pn out") which breaks
keyboard accessibility; update the source component so those elements are real
interactive elements (replace the clickable divs used in Fe for inputs,
transform and outputs with <button type="button"> preserving the same class
names, onClick handlers and children) or, if you cannot change the tag, add full
button semantics (role="button", tabIndex=0, keyDown handlers for Enter/Space,
and focus-visible styling) to the same symbols (Fe, the pn elements) and then
rebuild/regenerate the bundled dashboard.html artifact.

In `@dashboard/index.html`:
- Line 5: The diagnostics server currently only serves "/", "/health", "/ready",
and "/api/*" but index.html links to "/favicon.svg", causing a 404; fix by
either adding a GET route for "/favicon.svg" in the diagnostics route setup (so
the diagnostics server returns the favicon asset) or change the HTML to
reference an asset the server already serves (e.g., remove the favicon link or
point it to an existing served path). Locate the diagnostics route registration
code that defines the "/", "/health", "/ready", and "/api/*" handlers and add a
handler for "/favicon.svg" that returns the favicon content (or update
index.html to remove/replace the /favicon.svg href).

In `@dashboard/package.json`:
- Line 8: The package.json "build" npm script uses the Unix-only cp command
which breaks on Windows; update the build script (the "build" entry) to use a
cross-platform copier (e.g., install a devDependency like shx or cpy-cli and
change the command to use shx cp or cpy-cli), or replace the copy step with a
small Node script invoked from the same "build" script; ensure you add the
chosen tool to devDependencies and update the "build" value to use that tool
instead of plain cp.

In `@dashboard/src/api.ts`:
- Around line 3-10: The get<T> function can hang during polling; modify get<T>
to use an AbortController with a short timeout (e.g., configurable default like
5s) so stalled fetches are aborted: create an AbortController, start a
setTimeout that calls controller.abort() after the timeout, pass
controller.signal into fetch(url, { signal }), clear the timeout when fetch
completes successfully, and in the catch return null for both network errors and
AbortError; update only the get<T> function to implement this behavior.

In `@dashboard/src/app.tsx`:
- Around line 28-36: The fds metric ("fds" in createSeries()) is never populated
(poll() never writes to series[5]) and /api/stats doesn't expose an fd field in
diagnostics.rs, so remove or backfill it: either delete the "fds" entry from
createSeries() (and any UI references to the "File Descriptors" card) or
implement end-to-end support by adding an fd field to the stats response in
crates/logfwd-io/src/diagnostics.rs and update poll() to read that value and
write it into series with id "fds". Ensure the chosen fix keeps createSeries(),
poll(), and diagnostics.rs consistent so the card no longer stays in “waiting…”.
- Line 133: ConfigView is rendering despite the backend not exposing
/api/config; either add the missing server endpoint or gate the component
client-side. Fix options: (A) Server-side — add a route handler in the
diagnostics route table (crates/logfwd-io/src/diagnostics.rs) to respond to GET
/api/config with the expected config payload so ConfigView can fetch it; or (B)
Client-side — replace the unconditional <ConfigView /> render in app.tsx with a
guarded render that checks API availability (e.g., call GET /api/config in a
useEffect or a feature-check hook, detect 200 vs 404, and only render ConfigView
when the endpoint exists, otherwise show a placeholder or omit the component).
Ensure the change references ConfigView and the /api/config endpoint so the
component only mounts when the backend supports it.
- Line 26: RateTracker is being created/used inside the render path (compRate /
PipelineView) and compRate calls rates.rate() during render, which mutates
baselines; move instantiation and sampling into the poll() routine so render
only receives plain numeric snapshots. Specifically: stop constructing
RateTracker in app render (remove new RateTracker() from where compRate is
used), create/hold a single RateTracker instance as a module-level or component
instance used only by poll(), call rates.rate() inside poll() to compute
per-pipeline numeric rate snapshots, store those numbers in state (e.g.,
pipelineRateMap), and update PipelineView/compRate to accept plain numbers
(replace the rates prop with rate values) so no mutation occurs during render.
Ensure any code paths still passing the RateTracker instance (the prop mentioned
around PipelineView) are updated to pass the computed numeric snapshot instead.
- Line 109: The expression assigned to the variable version uses ?? and ?:
without grouping, so the ternary binds before the nullish coalescing and yields
incorrect results; fix by keeping pipes?.system?.version as the left side of the
nullish coalescing and wrapping the entire fallback ternary (stats?.uptime_sec
!= null ? "?" : "") in parentheses so the code reads: pipes?.system?.version ??
(stats?.uptime_sec != null ? "?" : ""); update the assignment to version
accordingly.
- Around line 47-48: The polling is currently started with setInterval which
allows overlapping calls to poll() (the function defined as const poll =
useCallback(...)) and causes rates.tick() and state updates to race; replace the
setInterval-based start with a self-scheduling loop that awaits poll() to finish
before waiting POLL_MS and scheduling the next iteration, and ensure the loop
can be stopped on unmount (use a mounted/aborted ref or AbortController inside
the effect that starts the loop). Specifically, locate the effect that starts
the interval (the code that references POLL_MS and calls setInterval to invoke
poll) and change it to an async loop: while (mounted) { await poll(); await
sleep(POLL_MS); } (or equivalent) so only one poll runs at a time and
rates.tick() cannot be called out of sequence.

In `@dashboard/src/components/Chart.tsx`:
- Around line 14-16: The useEffect call currently has no dependency array
causing it to run after every render (intentional here); add an inline comment
above the useEffect explaining that this "render-every-frame" behavior is
deliberate because the parent forces re-renders when data updates, referencing
the draw(canvasRef.current, series) invocation and the canvasRef/series
variables so future readers know this is not an accidental omission of
dependencies.
- Around line 21-23: The draw function uses a non-null assertion on
canvas.getContext("2d") which can be null; update the draw(canvas:
HTMLCanvasElement | null, series: MetricSeries) implementation to safely handle
a null context by assigning const ctx = canvas.getContext("2d") and checking if
(!ctx) return (or log/throw) before using it, removing the "!" and guarding all
subsequent canvas drawing logic to avoid runtime errors.

In `@dashboard/src/components/ChartGrid.tsx`:
- Line 1: ChartGrid is importing MetricSeries from app.tsx which couples the
component to the app entry; extract the MetricSeries type into a new shared
types module (e.g., create a types file exporting MetricSeries) and update
ChartGrid to import MetricSeries from that module instead of app.tsx; also
update app.tsx (and any other files) to import MetricSeries from the new shared
types module and remove the type export from app.tsx so the component is
decoupled.

In `@dashboard/src/components/ConfigView.tsx`:
- Around line 5-18: Add a short, security-focused comment inside the
highlightYaml function explaining why we set el.textContent = yaml and then use
el.innerHTML for regex-based span injection (i.e., textContent safely
HTML-escapes user input before we deliberately inject span tags), note the
importance of doing XSS-safe escaping before replacements and why comments are
processed first; reference the function name highlightYaml and the textContent →
innerHTML pattern so future maintainers understand this is intentional and safe.
- Around line 24-31: The useEffect uses a redundant loaded flag to run once;
replace it with a mount-only effect by changing the dependency array to [] and
removing the loaded state management. In the effect that calls api.config()
(referencing useEffect, setLoaded, loaded, setConfig, and api.config), delete
setLoaded and the loaded checks/variable, and simply call api.config().then(data
=> { if (data) setConfig(data); }) inside a useEffect(() => { ... }, []). Also
remove the related loaded state declaration and any setLoaded usages.

In `@dashboard/src/components/MetricBadges.tsx`:
- Around line 19-24: The truthy checks on stats.mem_allocated and
stats.mem_resident suppress valid zero values; update the condition in
MetricBadges.tsx to use null/undefined-safe checks (e.g., stats.mem_allocated !=
null) before pushing into badges and use stats.mem_resident != null when
deciding the limit so that 0 is treated as a valid value; keep the badge
creation logic (label "Heap", value fmtBytes(stats.mem_allocated), limit using
fmtBytes(stats.mem_resident)) the same otherwise.

In `@dashboard/src/components/PipelineView.tsx`:
- Around line 34-48: The map over p.inputs (the JSX fragment inside p.inputs.map
in PipelineView, which renders Arrow and the div with class "pn inp" and uses
sel and toggle) is missing a key on the outer Fragment; update the iteration to
provide a stable key for each item (e.g., use the loop index or preferably
inp.id) by replacing the shorthand Fragment with a keyed element or adding key
to a long-form Fragment, and do the same for the other mapped block around lines
66-83 so every top-level element returned by .map has a unique key.
- Around line 25-28: The render-time call to rates.rate() in PipelineView.tsx
(inside compRate) mutates RateTracker and breaks the poll/tick contract; fix by
adding a read-only accessor on RateTracker (e.g., getRate(key) or lastRate(key))
that returns the last-computed EMA/delta without updating internal state, and
replace render calls to rates.rate(...) in PipelineView.tsx with this new
getter; ensure the actual rates continue to be computed only in the poll loop in
app.tsx (where rate(...) and tick() are invoked) so RateTracker.rate(...)
remains used only during polling and the read-only method is used during render.

In `@dashboard/src/components/StatusBar.tsx`:
- Around line 14-16: The pillText logic currently always uses the plural
"errors"; update the ternary that builds pillText so it special-cases a single
error: when connected and totalErrors > 0 return "1 error" for totalErrors === 1
and otherwise return `${totalErrors} errors`, keeping the existing "healthy" and
"disconnected" branches; look for the pillText constant and adjust its nested
conditional accordingly.

In `@dashboard/src/lib/format.ts`:
- Around line 16-22: The function fmtBytes currently treats 0 as missing by
returning "-" — update fmtBytes to only treat null/undefined as missing (remove
the b === 0 check) so that 0 bytes format normally; i.e., ensure fmtBytes(b:
number | null | undefined) returns "0 B" (or the appropriate unit) for b === 0
by letting the value fall through to the existing numeric formatting logic in
fmtBytes.
- Around line 31-33: The fmtDuration function returns an empty string for
null/undefined which is inconsistent with other formatters; update fmtDuration
to return the missing-value token "-" when s is null or undefined (i.e., replace
the early return of "" with "-") and keep the rest of the logic (the branch for
>= 86400 and other formatting) unchanged so the function name fmtDuration and
its signature remain the same.

In `@dashboard/src/style.css`:
- Line 19: The CSS custom property --sans currently lists unquoted font family
names (e.g., BlinkMacSystemFont, Roboto) which fails stylelint; update the
--sans declaration in the stylesheet to wrap each multi-word or non-keyword
family name in quotes (e.g., "BlinkMacSystemFont", "Roboto") so the font stack
is preserved and linting for value-keyword-case passes.

In `@dashboard/src/types.ts`:
- Around line 64-86: Remove the unused TypeScript interface SystemResponse from
types.ts: delete the entire export interface SystemResponse declaration (the
symbol SystemResponse) since it is not referenced by any modules (api.ts exports
only pipelines(), stats(), and config()) and will be added later when actually
needed; ensure no other files import or reference SystemResponse before removing
and run type-check to confirm no breakages.

In `@justfile`:
- Around line 49-50: The dashboard recipe in the justfile uses `npm install
--prefer-offline`; update the `dashboard` target to use lockfile-strict installs
by replacing `npm install --prefer-offline` with `npm ci` while keeping the `cd
dashboard &&` and `npm run build` steps intact so the `dashboard` recipe
performs deterministic installs in CI.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b5e85413-88bc-4409-aacd-10715302a929

📥 Commits

Reviewing files that changed from the base of the PR and between 2dc75dc and 34c1860.

⛔ Files ignored due to path filters (3)

• dashboard/package-lock.json is excluded by !**/package-lock.json
• dashboard/public/favicon.svg is excluded by !**/*.svg
• dashboard/public/icons.svg is excluded by !**/*.svg

📒 Files selected for processing (27)

• .github/workflows/ci.yml
• .github/workflows/release.yml
• .gitignore
• crates/logfwd-io/src/dashboard.html
• dashboard/.gitignore
• dashboard/index.html
• dashboard/package.json
• dashboard/src/api.ts
• dashboard/src/app.tsx
• dashboard/src/components/Chart.tsx
• dashboard/src/components/ChartGrid.tsx
• dashboard/src/components/ConfigView.tsx
• dashboard/src/components/MetricBadges.tsx
• dashboard/src/components/PipelineView.tsx
• dashboard/src/components/StatusBar.tsx
• dashboard/src/components/SystemRow.tsx
• dashboard/src/lib/format.ts
• dashboard/src/lib/rates.ts
• dashboard/src/lib/ring.ts
• dashboard/src/main.tsx
• dashboard/src/style.css
• dashboard/src/types.ts
• dashboard/tsconfig.app.json
• dashboard/tsconfig.json
• dashboard/tsconfig.node.json
• dashboard/vite.config.ts
• justfile

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Build the dashboard once and fan out as an artifact.

On Line 46-48, the same dashboard build is repeated for every matrix target. Consider a dedicated dashboard-build job that produces crates/logfwd-io/src/dashboard.html once, uploads it, and has matrix builds download it. This reduces runtime and external npm dependency risk during releases.

🤖 Prompt for AI Agents

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

In @.github/workflows/release.yml around lines 46 - 48, Create a new dedicated
GitHub Actions job named dashboard-build that runs the dashboard steps (cd
dashboard && npm ci && npm run build), saves the produced file
crates/logfwd-io/src/dashboard.html as an uploaded artifact, and mark it to run
before the matrix build job(s); then in the existing matrix job(s) replace the
repeated build step with an artifact download step (actions/download-artifact)
to retrieve crates/logfwd-io/src/dashboard.html so each matrix target reuses the
single built dashboard artifact instead of rebuilding it.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use real buttons for the pipeline cards.

The generated bundle renders each .pn node as a clickable div with onClick only, so keyboard users cannot open the inspectors. Please fix the source component to use <button type="button"> (preferred) or add button semantics, Enter/Space handling, and :focus-visible styling, then regenerate this artifact.

🤖 Prompt for AI Agents

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

In `@crates/logfwd-io/src/dashboard.html` at line 8, The pipeline cards are
rendered as clickable divs in the Fe component (the nodes with class names like
"pn inp", "pn xfm", "pn out") which breaks keyboard accessibility; update the
source component so those elements are real interactive elements (replace the
clickable divs used in Fe for inputs, transform and outputs with <button
type="button"> preserving the same class names, onClick handlers and children)
or, if you cannot change the tag, add full button semantics (role="button",
tabIndex=0, keyDown handlers for Enter/Space, and focus-visible styling) to the
same symbols (Fe, the pn elements) and then rebuild/regenerate the bundled
dashboard.html artifact.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

/favicon.svg will 404 on the diagnostics server.

On Line 5, the server route set only serves /, /health, /ready, and /api/*; it does not serve /favicon.svg (crates/logfwd-io/src/diagnostics.rs, Line 295-405). This creates a persistent broken asset request.

Suggested fix

-    <link rel="icon" type="image/svg+xml" href="https://github.com/favicon.svg" />

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

<link rel="icon" type="image/svg+xml" href="https://github.com/favicon.svg" />

🤖 Prompt for AI Agents

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

In `@dashboard/index.html` at line 5, The diagnostics server currently only serves
"/", "/health", "/ready", and "/api/*" but index.html links to "/favicon.svg",
causing a 404; fix by either adding a GET route for "/favicon.svg" in the
diagnostics route setup (so the diagnostics server returns the favicon asset) or
change the HTML to reference an asset the server already serves (e.g., remove
the favicon link or point it to an existing served path). Locate the diagnostics
route registration code that defines the "/", "/health", "/ready", and "/api/*"
handlers and add a handler for "/favicon.svg" that returns the favicon content
(or update index.html to remove/replace the /favicon.svg href).

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Build script uses Unix-only cp command.

This will fail on Windows. Consider using a cross-platform approach like shx or cpy-cli, or document that the build requires a Unix-like environment.

♻️ Cross-platform alternative using shx

-    "build": "tsc -b && vite build && cp ../crates/logfwd-io/src/dashboard-dist/index.html ../crates/logfwd-io/src/dashboard.html",
+    "build": "tsc -b && vite build && shx cp ../crates/logfwd-io/src/dashboard-dist/index.html ../crates/logfwd-io/src/dashboard.html",

Add shx to devDependencies:

     "vite": "^8.0.1",
-    "vite-plugin-singlefile": "^2.3.2"
+    "vite-plugin-singlefile": "^2.3.2",
+    "shx": "^0.3.4"

🤖 Prompt for AI Agents

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

In `@dashboard/package.json` at line 8, The package.json "build" npm script uses
the Unix-only cp command which breaks on Windows; update the build script (the
"build" entry) to use a cross-platform copier (e.g., install a devDependency
like shx or cpy-cli and change the command to use shx cp or cpy-cli), or replace
the copy step with a small Node script invoked from the same "build" script;
ensure you add the chosen tool to devDependencies and update the "build" value
to use that tool instead of plain cp.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -path "*/dashboard/src/api.ts" -o -path "*/api.ts" | head -20

Repository: strawgate/memagent

Length of output: 84

---

🏁 Script executed:

git ls-files | grep -E "(api|dashboard)" | head -30

Repository: strawgate/memagent

Length of output: 941

---

🏁 Script executed:

fd "api.ts" --type f

Repository: strawgate/memagent

Length of output: 82

---

🏁 Script executed:

cat -n dashboard/src/api.ts

Repository: strawgate/memagent

Length of output: 648

---

🏁 Script executed:

rg "get<" dashboard/src --type ts -A 3 -B 1

Repository: strawgate/memagent

Length of output: 589

---

🏁 Script executed:

rg "polling\|interval\|subscribe\|refresh" dashboard/src --type ts -B 2 -A 2

Repository: strawgate/memagent

Length of output: 44

---

🏁 Script executed:

rg "api\.(pipelines|stats|config)" dashboard/src --type ts -B 2 -A 2

Repository: strawgate/memagent

Length of output: 645

---

🏁 Script executed:

rg "setInterval|useEffect|setImmediate" dashboard/src --type ts -B 1 -A 3

Repository: strawgate/memagent

Length of output: 1757

---

🏁 Script executed:

cat -n dashboard/src/app.tsx | head -100

Repository: strawgate/memagent

Length of output: 4506

---

🏁 Script executed:

cat -n dashboard/src/app.tsx | tail -30

Repository: strawgate/memagent

Length of output: 989

---

🏁 Script executed:

rg "AbortController|timeout|fetch.*signal" dashboard/src --type ts

Repository: strawgate/memagent

Length of output: 44

---

Add a timeout to prevent hanging fetches in recurring poll requests.

The get<T> function is used in polling (2-second intervals) via Promise.all([api.pipelines(), api.stats()]). Without abort timeout, a stalled request blocks poll completion, degrading dashboard freshness.

Proposed change

 async function get<T>(url: string): Promise<T | null> {
+  const controller = new AbortController();
+  const timeout = setTimeout(() => controller.abort(), 5000);
   try {
-    const res = await fetch(url);
+    const res = await fetch(url, { signal: controller.signal });
     if (!res.ok) return null;
     return (await res.json()) as T;
   } catch {
     return null;
+  } finally {
+    clearTimeout(timeout);
   }
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	async function get<T>(url: string): Promise<T | null> {
	try {
	const res = await fetch(url);
	if (!res.ok) return null;
	return (await res.json()) as T;
	} catch {
	return null;
	}
	async function get<T>(url: string): Promise<T | null> {
	const controller = new AbortController();
	const timeout = setTimeout(() => controller.abort(), 5000);
	try {
	const res = await fetch(url, { signal: controller.signal });
	if (!res.ok) return null;
	return (await res.json()) as T;
	} catch {
	return null;
	} finally {
	clearTimeout(timeout);
	}
	}

🤖 Prompt for AI Agents

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

In `@dashboard/src/api.ts` around lines 3 - 10, The get<T> function can hang
during polling; modify get<T> to use an AbortController with a short timeout
(e.g., configurable default like 5s) so stalled fetches are aborted: create an
AbortController, start a setTimeout that calls controller.abort() after the
timeout, pass controller.signal into fetch(url, { signal }), clear the timeout
when fetch completes successfully, and in the catch return null for both network
errors and AbortError; update only the get<T> function to implement this
behavior.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Do not map 0 bytes to missing data.

On Line 17, returning "-" for 0 makes valid zero values indistinguishable from null/absent metrics.

Suggested fix

 export function fmtBytes(b: number | null | undefined): string {
-  if (b == null || b === 0) return "-";
+  if (b == null) return "-";
+  if (b === 0) return "0 B";
   if (b >= 1073741824) return (b / 1073741824).toFixed(1) + " GB";
   if (b >= 1048576) return (b / 1048576).toFixed(1) + " MB";
   if (b >= 1024) return (b / 1024).toFixed(1) + " KB";
   return b + " B";
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	export function fmtBytes(b: number | null | undefined): string {
	if (b == null || b === 0) return "-";
	if (b >= 1073741824) return (b / 1073741824).toFixed(1) + " GB";
	if (b >= 1048576) return (b / 1048576).toFixed(1) + " MB";
	if (b >= 1024) return (b / 1024).toFixed(1) + " KB";
	return b + " B";
	}
	export function fmtBytes(b: number | null | undefined): string {
	if (b == null) return "-";
	if (b === 0) return "0 B";
	if (b >= 1073741824) return (b / 1073741824).toFixed(1) + " GB";
	if (b >= 1048576) return (b / 1048576).toFixed(1) + " MB";
	if (b >= 1024) return (b / 1024).toFixed(1) + " KB";
	return b + " B";
	}

🤖 Prompt for AI Agents

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

In `@dashboard/src/lib/format.ts` around lines 16 - 22, The function fmtBytes
currently treats 0 as missing by returning "-" — update fmtBytes to only treat
null/undefined as missing (remove the b === 0 check) so that 0 bytes format
normally; i.e., ensure fmtBytes(b: number | null | undefined) returns "0 B" (or
the appropriate unit) for b === 0 by letting the value fall through to the
existing numeric formatting logic in fmtBytes.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use a consistent placeholder for missing duration values.

On Line 32, "" for null duration can render as a blank field; use the same missing-value token ("-") as other formatters.

Suggested fix

 export function fmtDuration(s: number | null | undefined): string {
-  if (s == null) return "";
+  if (s == null) return "-";
   if (s >= 86400) return Math.floor(s / 86400) + "d " + Math.floor((s % 86400) / 3600) + "h";
   if (s >= 3600) return Math.floor(s / 3600) + "h " + Math.floor((s % 3600) / 60) + "m";
   if (s >= 60) return Math.floor(s / 60) + "m " + Math.floor(s % 60) + "s";
   return s + "s";
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	export function fmtDuration(s: number | null | undefined): string {
	if (s == null) return "";
	if (s >= 86400) return Math.floor(s / 86400) + "d " + Math.floor((s % 86400) / 3600) + "h";
	export function fmtDuration(s: number | null | undefined): string {
	if (s == null) return "-";
	if (s >= 86400) return Math.floor(s / 86400) + "d " + Math.floor((s % 86400) / 3600) + "h";
	if (s >= 3600) return Math.floor(s / 3600) + "h " + Math.floor((s % 3600) / 60) + "m";
	if (s >= 60) return Math.floor(s / 60) + "m " + Math.floor(s % 60) + "s";
	return s + "s";
	}

🤖 Prompt for AI Agents

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

In `@dashboard/src/lib/format.ts` around lines 31 - 33, The fmtDuration function
returns an empty string for null/undefined which is inconsistent with other
formatters; update fmtDuration to return the missing-value token "-" when s is
null or undefined (i.e., replace the early return of "" with "-") and keep the
rest of the logic (the branch for >= 86400 and other formatting) unchanged so
the function name fmtDuration and its signature remain the same.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Quote the font family names so lint passes.

Stylelint is already failing on BlinkMacSystemFont and Roboto here under value-keyword-case. Quoting them preserves the stack and unblocks CI.

Suggested change

-  --sans: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
+  --sans: -apple-system, "BlinkMacSystemFont", "Segoe UI", "Roboto", sans-serif;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	--sans: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
	--sans: -apple-system, "BlinkMacSystemFont", "Segoe UI", "Roboto", sans-serif;

🧰 Tools

🪛 Stylelint (17.5.0)

[error] 19-19: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)

(value-keyword-case)

---

[error] 19-19: Expected "Roboto" to be "roboto" (value-keyword-case)

(value-keyword-case)

🤖 Prompt for AI Agents

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

In `@dashboard/src/style.css` at line 19, The CSS custom property --sans currently
lists unquoted font family names (e.g., BlinkMacSystemFont, Roboto) which fails
stylelint; update the --sans declaration in the stylesheet to wrap each
multi-word or non-keyword family name in quotes (e.g., "BlinkMacSystemFont",
"Roboto") so the font stack is preserved and linting for value-keyword-case
passes.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Use npm ci for deterministic dashboard builds.

dashboard-check runs this in CI, so lockfile-strict installs are safer and more reproducible than npm install.

Proposed change

 dashboard:
-    cd dashboard && npm install --prefer-offline && npm run build
+    cd dashboard && npm ci --prefer-offline && npm run build

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	dashboard:
	cd dashboard && npm install --prefer-offline && npm run build
	dashboard:
	cd dashboard && npm ci && npm run build

🤖 Prompt for AI Agents

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

In `@justfile` around lines 49 - 50, The dashboard recipe in the justfile uses
`npm install --prefer-offline`; update the `dashboard` target to use
lockfile-strict installs by replacing `npm install --prefer-offline` with `npm
ci` while keeping the `cd dashboard &&` and `npm run build` steps intact so the
`dashboard` recipe performs deterministic installs in CI.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (2)

crates/logfwd-io/src/dashboard.html (1)

8-8: ⚠️ Potential issue | 🟠 Major

Use real buttons for the pipeline cards.

The shipped bundle still renders the .pn cards as clickable divs with onClick only. Keyboard users cannot open the inspectors. Fix dashboard/src/components/PipelineView.tsx to use <button type="button"> (or equivalent button semantics) and regenerate this artifact.

🤖 Prompt for AI Agents

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

In `@crates/logfwd-io/src/dashboard.html` at line 8, The pipeline cards are
rendered as clickable divs (.pn) with onClick only, blocking keyboard users;
update the PipelineView component (the Pe / PipelineView.tsx logic that renders
elements with class `pn inp`, `pn xfm`, `pn out`) to use real <button
type="button"> elements (preserve the existing class names, children structure
and onClick handlers), ensure buttons have type="button" to avoid form
submission, and keep any visual styling intact; after changing the elements,
rebuild/regenerate the bundled dashboard artifact so the compiled dashboard.html
includes the button changes.

dashboard/src/app.tsx (1)

114-118: ⚠️ Potential issue | 🟠 Major

Serialize the polling loop.

setInterval does not wait for the previous async poll() to finish. Slow responses will overlap and race on the shared rates tracker and seriesRef.current mutations.

Possible fix

-  useEffect(() => {
-    poll();
-    const id = setInterval(poll, POLL_MS);
-    return () => clearInterval(id);
-  }, [poll]);
+  useEffect(() => {
+    let cancelled = false;
+    let timeoutId: number | undefined;
+
+    const loop = async () => {
+      await poll();
+      if (!cancelled) timeoutId = window.setTimeout(loop, POLL_MS);
+    };
+
+    void loop();
+    return () => {
+      cancelled = true;
+      if (timeoutId != null) window.clearTimeout(timeoutId);
+    };
+  }, [poll]);

🤖 Prompt for AI Agents

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

In `@dashboard/src/app.tsx` around lines 114 - 118, The polling loop currently
uses setInterval which can start a new async poll() before the previous one
finishes, causing races on rates and seriesRef.current; replace the setInterval
approach in the useEffect with a serialized async loop: create an async function
(e.g., runSerializedPoll) that while mounted awaits poll(), then waits POLL_MS
via setTimeout (or await sleep) before the next iteration, store the timeout id
and a mounted flag for cleanup, and on effect cleanup set mounted=false and
clear the timeout; ensure you reference the existing poll function and the
shared state (rates and seriesRef.current) so all invocations are awaited and
never overlap.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@crates/logfwd-io/src/diagnostics.rs`:
- Around line 388-389: The current diagnostics route returns the raw YAML config
via DiagnosticsServer::serve_config which leaks secrets; modify serve_config (or
replace it with serve_redacted_config) to never return the raw YAML by default —
either redact sensitive keys (match names like "password", "token", "secret",
"api_key", file paths) using a redaction helper and return a safe projection, or
require an explicit opt-in/auth flag on the DiagnosticsServer struct (e.g., an
allow_raw_config boolean checked in DiagnosticsServer::serve_config) before
serving full config; update the route mapping that currently points to
serve_config and any other handlers in the same file (around the other
occurrences noted) to use the redaction helper or enforce the opt-in check.

In `@dashboard/src/app.tsx`:
- Around line 99-106: The latency is being computed as total processing ms/sec
instead of ms per batch; replace the single call to rates.rate("lat", totalSec *
1000) with a per-batch computation: get totalMsRate = rates.rate("lat", totalSec
* 1000) and batchesRate = rates.rate("batches", 1) and compute latRate =
(batchesRate && batchesRate > 0) ? totalMsRate / batchesRate :
(statsData.batches > 0 ? (totalSec * 1000) / statsData.batches : null); then
push latRate into series[5].ring and set series[5].value = latRate.toFixed(1)
only when latRate is not null (keep existing symbols statsData.batches,
totalSec, rates.rate("lat", ...), rates.rate("batches", 1), series[5].ring,
series[5].value).

---

Duplicate comments:
In `@crates/logfwd-io/src/dashboard.html`:
- Line 8: The pipeline cards are rendered as clickable divs (.pn) with onClick
only, blocking keyboard users; update the PipelineView component (the Pe /
PipelineView.tsx logic that renders elements with class `pn inp`, `pn xfm`, `pn
out`) to use real <button type="button"> elements (preserve the existing class
names, children structure and onClick handlers), ensure buttons have
type="button" to avoid form submission, and keep any visual styling intact;
after changing the elements, rebuild/regenerate the bundled dashboard artifact
so the compiled dashboard.html includes the button changes.

In `@dashboard/src/app.tsx`:
- Around line 114-118: The polling loop currently uses setInterval which can
start a new async poll() before the previous one finishes, causing races on
rates and seriesRef.current; replace the setInterval approach in the useEffect
with a serialized async loop: create an async function (e.g., runSerializedPoll)
that while mounted awaits poll(), then waits POLL_MS via setTimeout (or await
sleep) before the next iteration, store the timeout id and a mounted flag for
cleanup, and on effect cleanup set mounted=false and clear the timeout; ensure
you reference the existing poll function and the shared state (rates and
seriesRef.current) so all invocations are awaited and never overlap.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 418a78dc-2138-4dc4-a7b5-5e96d366c453

📥 Commits

Reviewing files that changed from the base of the PR and between 34c1860 and db9b0f5.

📒 Files selected for processing (4)

• crates/logfwd-io/src/dashboard.html
• crates/logfwd-io/src/diagnostics.rs
• crates/logfwd/src/main.rs
• dashboard/src/app.tsx

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Compute batch latency from batch deltas.

rates.rate("lat", totalSec * 1000) gives accumulated processing milliseconds per second, not milliseconds per batch. At 100 batches/s with 5 ms of work each, this card reports ~500 ms instead of 5 ms. Divide the stage-time delta by the batch-count delta, or divide rate(total_ms) by rate(batches).

Possible fix

-      // Batch latency: total processing time / batches
-      if (statsData.batches > 0) {
-        const totalSec = statsData.scan_sec + statsData.transform_sec + statsData.output_sec;
-        const latRate = rates.rate("lat", totalSec * 1000);
-        if (latRate != null) {
-          series[5].ring.push(latRate);
-          series[5].value = latRate.toFixed(1);
-        }
-      }
+      // Batch latency: delta processing time / delta batches
+      const totalMs =
+        (statsData.scan_sec + statsData.transform_sec + statsData.output_sec) * 1000;
+      const totalMsPerSec = rates.rate("lat_ms", totalMs);
+      const batchesPerSec = rates.rate("batches", statsData.batches);
+      if (totalMsPerSec != null && batchesPerSec != null && batchesPerSec > 0) {
+        const latencyMs = totalMsPerSec / batchesPerSec;
+        series[5].ring.push(latencyMs);
+        series[5].value = latencyMs.toFixed(1);
+      }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Batch latency: total processing time / batches
	if (statsData.batches > 0) {
	const totalSec = statsData.scan_sec + statsData.transform_sec + statsData.output_sec;
	const latRate = rates.rate("lat", totalSec * 1000);
	if (latRate != null) {
	series[5].ring.push(latRate);
	series[5].value = latRate.toFixed(1);
	}
	// Batch latency: delta processing time / delta batches
	const totalMs =
	(statsData.scan_sec + statsData.transform_sec + statsData.output_sec) * 1000;
	const totalMsPerSec = rates.rate("lat_ms", totalMs);
	const batchesPerSec = rates.rate("batches", statsData.batches);
	if (totalMsPerSec != null && batchesPerSec != null && batchesPerSec > 0) {
	const latencyMs = totalMsPerSec / batchesPerSec;
	series[5].ring.push(latencyMs);
	series[5].value = latencyMs.toFixed(1);
	}

🤖 Prompt for AI Agents

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

In `@dashboard/src/app.tsx` around lines 99 - 106, The latency is being computed
as total processing ms/sec instead of ms per batch; replace the single call to
rates.rate("lat", totalSec * 1000) with a per-batch computation: get totalMsRate
= rates.rate("lat", totalSec * 1000) and batchesRate = rates.rate("batches", 1)
and compute latRate = (batchesRate && batchesRate > 0) ? totalMsRate /
batchesRate : (statsData.batches > 0 ? (totalSec * 1000) / statsData.batches :
null); then push latRate into series[5].ring and set series[5].value =
latRate.toFixed(1) only when latRate is not null (keep existing symbols
statsData.batches, totalSec, rates.rate("lat", ...), rates.rate("batches", 1),
series[5].ring, series[5].value).

[github-actions]

/api/logs currently emits invalid JSON for the capturing field.

At this line the response appends "capturing":" and then writes true/false without a closing quote, producing payloads like {"lines":[],"capturing":"true}. This breaks res.json() in the dashboard log viewer.

Please emit capturing as a JSON boolean instead:

• prefix should be r#"],\"capturing\":"#
• then append "true" / "false" (without surrounding JSON string quotes)

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (7)

dashboard/src/components/PipelineView.tsx (3)

67-84: ⚠️ Potential issue | 🟡 Minor

Missing key prop on Fragment for outputs.

Same issue as inputs—add key to the Fragment wrapper.

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/PipelineView.tsx` around lines 67 - 84, The Fragment
wrapper inside the p.outputs.map callback is missing a key which can cause React
rendering issues; add a unique key prop to the Fragment (e.g., key={`out-${i}`}
or use out.id if available) that corresponds to the mapped item so React can
track list items properly; locate the map over p.outputs in PipelineView.tsx
(the JSX that renders Arrow, the div with class "pn out" and uses sel and
toggle) and add the key on the surrounding Fragment element.

---

25-29: ⚠️ Potential issue | 🟠 Major

Calling rates.rate() and tick() during render corrupts rate calculations.

Both rate() and tick() mutate internal state. Calling them during render means every re-render (inspector toggle, selection change) will corrupt the EMA baselines and produce incorrect rates.

Move rate computation to a poll loop or add a read-only getter to RateTracker.

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/PipelineView.tsx` around lines 25 - 29, The compRate
helper is calling the mutating methods ratesRef.current.rate(...) and
ratesRef.current.tick() from render (in compRate), which corrupts the EMA
baselines; change this by moving rate updates out of render: either implement a
polling/effect loop (useEffect) that calls ratesRef.current.rate(name, value)
and ratesRef.current.tick() on an interval and writes computed formatted rates
into component state (e.g., a Map keyed by `${comp.name}_${field}`) that
compRate then reads, or add a non-mutating read-only accessor to RateTracker
(e.g., getRate(name, currentValue) or peekRate) and use that from compRate while
performing tick() only in the poll loop; update usages of compRate, ratesRef,
rate(), tick(), and RateTracker accordingly.

---

35-49: ⚠️ Potential issue | 🟡 Minor

Missing key prop on Fragment.

React/Preact requires a key on the outermost element in .map(). Use <Fragment key={inp.name}> or a keyed wrapper.

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/PipelineView.tsx` around lines 35 - 49, The
p.inputs.map rendering in PipelineView.tsx returns a top-level Fragment without
a key, which breaks React/Preact list rendering; fix by giving that outer
Fragment a stable key (e.g., key={inp.name} or key={i}), or remove the Fragment
and place the key on the outer <div class="pn inp"> instead, ensuring the Arrow
and the input node still render in the correct order; update the Fragment/outer
element where p.inputs.map maps over inputs (the block using Arrow, the div with
class "pn inp", toggle and sel) to include the key.

dashboard/src/app.tsx (2)

115-119: ⚠️ Potential issue | 🟠 Major

Polling loop can overlap if poll() takes >2s.

setInterval doesn't await the async poll(). Overlapping calls corrupt RateTracker state. Use a self-scheduling loop.

🔧 Suggested fix

   useEffect(() => {
-    poll();
-    const id = setInterval(poll, POLL_MS);
-    return () => clearInterval(id);
+    let cancelled = false;
+    let timeoutId: number | undefined;
+
+    const loop = async () => {
+      await poll();
+      if (!cancelled) timeoutId = window.setTimeout(loop, POLL_MS);
+    };
+
+    void loop();
+    return () => {
+      cancelled = true;
+      if (timeoutId != null) clearTimeout(timeoutId);
+    };
   }, [poll]);

🤖 Prompt for AI Agents

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

In `@dashboard/src/app.tsx` around lines 115 - 119, The polling currently uses
setInterval so async poll() calls can overlap and corrupt RateTracker; replace
the setInterval pattern in useEffect with a self-scheduling loop that awaits
poll() between iterations (e.g., an async function that while(!stopped) { await
poll(); await sleep(POLL_MS); }) and cancel it on unmount by flipping a stopped
flag or using an AbortController; ensure the loop creation/cleanup sits inside
the same useEffect and still depends on poll so only one non-overlapping
instance runs and RateTracker state remains consistent.

---

100-108: ⚠️ Potential issue | 🟠 Major

Batch latency formula yields ms/s, not ms/batch.

rates.rate("lat", totalSec * 1000) gives the rate of change of cumulative processing time (ms per second), not latency per batch. Divide by the batch rate to get actual latency.

🔧 Correct calculation

       // Batch latency: total processing time / batches
-      if (statsData.batches > 0) {
-        const totalSec = statsData.scan_sec + statsData.transform_sec + statsData.output_sec;
-        const latRate = rates.rate("lat", totalSec * 1000);
-        if (latRate != null) {
-          series[5].ring.push(latRate);
-          series[5].value = latRate.toFixed(1);
-        }
-      }
+      const totalMs = (statsData.scan_sec + statsData.transform_sec + statsData.output_sec) * 1000;
+      const msPerSec = rates.rate("lat_ms", totalMs);
+      const batchesPerSec = rates.rate("lat_batches", statsData.batches);
+      if (msPerSec != null && batchesPerSec != null && batchesPerSec > 0) {
+        const latencyMs = msPerSec / batchesPerSec;
+        series[5].ring.push(latencyMs);
+        series[5].value = latencyMs.toFixed(1);
+      }

🤖 Prompt for AI Agents

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

In `@dashboard/src/app.tsx` around lines 100 - 108, The code computes a
time-per-second rate with rates.rate("lat", totalSec * 1000) instead of
time-per-batch; compute the batch rate and divide the latency rate by it.
Specifically, call something like const batchRate = rates.rate("batches",
statsData.batches), ensure batchRate > 0, then compute const latPerBatch =
latRate / batchRate and push/assign latPerBatch (not latRate) to series[5].ring
and series[5].value (formatting with toFixed). Keep the existing guards
(statsData.batches > 0 and latRate != null) and add a guard for batchRate being
non-null and > 0.

crates/logfwd-io/src/dashboard.html (1)

1-15: ⚠️ Potential issue | 🟠 Major

Generated artifact—issues should be fixed in source components.

The keyboard accessibility issue (pipeline cards using <div onClick> instead of <button>) must be fixed in the source (dashboard/src/components/PipelineView.tsx) and this file regenerated.

🤖 Prompt for AI Agents

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

In `@crates/logfwd-io/src/dashboard.html` around lines 1 - 15, The pipeline cards
are rendered as clickable <div onClick> elements (see the Pe component and the
PN elements with class "pn" / pn.inp / pn.xfm / pn.out) which breaks keyboard
accessibility; update the source PipelineView component (the Pe function) to use
semantic interactive elements (replace those clickable <div class="pn"> blocks
with <button type="button"> or a styled <button>), move the onClick handlers to
the button (remove reliance on onKeyDown or role hacks), preserve existing class
names/styles so visuals are unchanged, add any necessary aria-labels or
aria-pressed state if selection is meaningful, then rebuild/regenerate this
dashboard.html artifact.

crates/logfwd-io/src/diagnostics.rs (1)

534-548: ⚠️ Potential issue | 🔴 Critical

Raw config exposed via /api/config leaks credentials.

serve_config returns config_yaml verbatim—any secrets (API keys, passwords, file paths) are exposed over the unauthenticated diagnostics port. Either redact sensitive fields or gate behind explicit opt-in.

🤖 Prompt for AI Agents

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

In `@crates/logfwd-io/src/diagnostics.rs` around lines 534 - 548, serve_config
currently returns self.config_yaml verbatim which leaks secrets; change
serve_config in diagnostics.rs so it never exposes raw config by default: either
(A) parse self.config_yaml (YAML/JSON), recursively redact or replace values for
keys matching sensitive patterns (password, secret, token, key, api, access,
private, secret_access_key, etc.) and serialize the redacted payload instead of
raw config_yaml, or (B) gate the endpoint behind an explicit opt-in flag on the
diagnostics struct (e.g., self.allow_expose_config) and return an error/403 when
the flag is false; update the response body to include only
esc(&self.config_path) plus the redacted config or a denial message and ensure
serve_config, config_yaml, and config_path references are used to locate and
implement the change.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@crates/logfwd-io/src/stderr_capture.rs`:
- Around line 146-151: The thread spawn error is being dropped by calling .ok()
on std::thread::Builder::new().spawn(...), so if the reader thread fails to
start stderr stays redirected and the pipe reader is never run; change this to
handle the Result from spawn, e.g. match or if let Err(e) =
std::thread::Builder::new().name("stderr-capture".into()).spawn(move || {
reader_loop(read_fd, orig, &state); }) { log or write the error to the original
stderr (orig) and restore stderr to orig (undo the redirection) and perform any
cleanup/early return }; on Ok(_) continue as before. Ensure you reference
reader_loop, read_fd, orig, and state when implementing the error handling and
restore logic.

---

Duplicate comments:
In `@crates/logfwd-io/src/dashboard.html`:
- Around line 1-15: The pipeline cards are rendered as clickable <div onClick>
elements (see the Pe component and the PN elements with class "pn" / pn.inp /
pn.xfm / pn.out) which breaks keyboard accessibility; update the source
PipelineView component (the Pe function) to use semantic interactive elements
(replace those clickable <div class="pn"> blocks with <button type="button"> or
a styled <button>), move the onClick handlers to the button (remove reliance on
onKeyDown or role hacks), preserve existing class names/styles so visuals are
unchanged, add any necessary aria-labels or aria-pressed state if selection is
meaningful, then rebuild/regenerate this dashboard.html artifact.

In `@crates/logfwd-io/src/diagnostics.rs`:
- Around line 534-548: serve_config currently returns self.config_yaml verbatim
which leaks secrets; change serve_config in diagnostics.rs so it never exposes
raw config by default: either (A) parse self.config_yaml (YAML/JSON),
recursively redact or replace values for keys matching sensitive patterns
(password, secret, token, key, api, access, private, secret_access_key, etc.)
and serialize the redacted payload instead of raw config_yaml, or (B) gate the
endpoint behind an explicit opt-in flag on the diagnostics struct (e.g.,
self.allow_expose_config) and return an error/403 when the flag is false; update
the response body to include only esc(&self.config_path) plus the redacted
config or a denial message and ensure serve_config, config_yaml, and config_path
references are used to locate and implement the change.

In `@dashboard/src/app.tsx`:
- Around line 115-119: The polling currently uses setInterval so async poll()
calls can overlap and corrupt RateTracker; replace the setInterval pattern in
useEffect with a self-scheduling loop that awaits poll() between iterations
(e.g., an async function that while(!stopped) { await poll(); await
sleep(POLL_MS); }) and cancel it on unmount by flipping a stopped flag or using
an AbortController; ensure the loop creation/cleanup sits inside the same
useEffect and still depends on poll so only one non-overlapping instance runs
and RateTracker state remains consistent.
- Around line 100-108: The code computes a time-per-second rate with
rates.rate("lat", totalSec * 1000) instead of time-per-batch; compute the batch
rate and divide the latency rate by it. Specifically, call something like const
batchRate = rates.rate("batches", statsData.batches), ensure batchRate > 0, then
compute const latPerBatch = latRate / batchRate and push/assign latPerBatch (not
latRate) to series[5].ring and series[5].value (formatting with toFixed). Keep
the existing guards (statsData.batches > 0 and latRate != null) and add a guard
for batchRate being non-null and > 0.

In `@dashboard/src/components/PipelineView.tsx`:
- Around line 67-84: The Fragment wrapper inside the p.outputs.map callback is
missing a key which can cause React rendering issues; add a unique key prop to
the Fragment (e.g., key={`out-${i}`} or use out.id if available) that
corresponds to the mapped item so React can track list items properly; locate
the map over p.outputs in PipelineView.tsx (the JSX that renders Arrow, the div
with class "pn out" and uses sel and toggle) and add the key on the surrounding
Fragment element.
- Around line 25-29: The compRate helper is calling the mutating methods
ratesRef.current.rate(...) and ratesRef.current.tick() from render (in
compRate), which corrupts the EMA baselines; change this by moving rate updates
out of render: either implement a polling/effect loop (useEffect) that calls
ratesRef.current.rate(name, value) and ratesRef.current.tick() on an interval
and writes computed formatted rates into component state (e.g., a Map keyed by
`${comp.name}_${field}`) that compRate then reads, or add a non-mutating
read-only accessor to RateTracker (e.g., getRate(name, currentValue) or
peekRate) and use that from compRate while performing tick() only in the poll
loop; update usages of compRate, ratesRef, rate(), tick(), and RateTracker
accordingly.
- Around line 35-49: The p.inputs.map rendering in PipelineView.tsx returns a
top-level Fragment without a key, which breaks React/Preact list rendering; fix
by giving that outer Fragment a stable key (e.g., key={inp.name} or key={i}), or
remove the Fragment and place the key on the outer <div class="pn inp"> instead,
ensuring the Arrow and the input node still render in the correct order; update
the Fragment/outer element where p.inputs.map maps over inputs (the block using
Arrow, the div with class "pn inp", toggle and sel) to include the key.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 12ee5524-044d-4bad-bce2-22f6838e81af

📥 Commits

Reviewing files that changed from the base of the PR and between db9b0f5 and 26c7e12.

📒 Files selected for processing (7)

• crates/logfwd-io/src/dashboard.html
• crates/logfwd-io/src/diagnostics.rs
• crates/logfwd-io/src/lib.rs
• crates/logfwd-io/src/stderr_capture.rs
• dashboard/src/app.tsx
• dashboard/src/components/LogViewer.tsx
• dashboard/src/components/PipelineView.tsx

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Thread spawn failure silently ignored.

.ok() discards the spawn error. If the thread fails to start, stderr is already redirected but nothing reads the pipe—writes will eventually block. At minimum log the failure; ideally restore stderr on spawn error.

🔧 Suggested fix

             std::thread::Builder::new()
                 .name("stderr-capture".into())
                 .spawn(move || {
                     reader_loop(read_fd, orig, &state);
                 })
-                .ok();
+                .unwrap_or_else(|e| {
+                    eprintln!("stderr-capture: failed to spawn reader thread: {e}");
+                    // Restore stderr since capture failed
+                    libc::dup2(orig, 2);
+                    libc::close(orig);
+                    libc::close(read_fd);
+                    state.active.store(false, Ordering::Relaxed);
+                });

Note: The eprintln! will go to the original stderr (via orig) since we restore it on failure.

🤖 Prompt for AI Agents

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

In `@crates/logfwd-io/src/stderr_capture.rs` around lines 146 - 151, The thread
spawn error is being dropped by calling .ok() on
std::thread::Builder::new().spawn(...), so if the reader thread fails to start
stderr stays redirected and the pipe reader is never run; change this to handle
the Result from spawn, e.g. match or if let Err(e) =
std::thread::Builder::new().name("stderr-capture".into()).spawn(move || {
reader_loop(read_fd, orig, &state); }) { log or write the error to the original
stderr (orig) and restore stderr to orig (undo the redirection) and perform any
cleanup/early return }; on Ok(_) continue as before. Ensure you reference
reader_loop, read_fd, orig, and state when implementing the error handling and
restore logic.

[github-actions]

Requesting changes due to a confirmed async polling race that can render stale log data out of order.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:#526 (pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)

---

What is this? | From workflow: AI: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[github-actions]

setInterval(poll, 2000) can start a new /api/logs request before the previous one finishes. Because poll is async and unguarded, slower older responses can arrive after newer ones and overwrite state via setLogs(data.lines), causing log output to jump backward/flicker.

Please serialize polling (e.g., self-scheduling setTimeout after await poll()), or guard against out-of-order updates (request sequence/in-flight flag).

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (6)

dashboard/src/components/LogViewer.tsx (2)

41-52: ⚠️ Potential issue | 🟠 Major

Use a real button for the log opener.

The clickable div at Line 41 is mouse-only. Keyboard users cannot open the panel from this control.

Suggested fix

-      <div
+      <button
+        type="button"
         class="log-prompt"
         onClick={() => setOpen(true)}
       >
@@
-      </div>
+      </button>

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/LogViewer.tsx` around lines 41 - 52, The log opener
is a non-interactive div which is not keyboard accessible; replace the <div
class="log-prompt" onClick={() => setOpen(true)}> opener in LogViewer with a
semantic <button> (or a component that renders a button) so it supports
keyboard/aria interaction, keep the existing onClick handler bound to setOpen,
preserve SVG and inner spans, and add an accessible label/aria-label (e.g.,
"Open logs") and type="button" so screen readers and keyboard users can open the
panel.

---

14-33: ⚠️ Potential issue | 🟠 Major

Serialize the /api/logs poller.

This setInterval can overlap async fetches. A slow response can arrive after a newer one and rewind the panel; cleanup also leaves the in-flight request running.

Suggested fix

   useEffect(() => {
     if (!open) return;
 
+    let cancelled = false;
+    let timeoutId: number | undefined;
+
     const poll = async () => {
       try {
         const res = await fetch("/api/logs");
         if (res.ok) {
           const data: LogsResponse = await res.json();
+          if (cancelled) return;
           setLogs(data.lines);
           setCapturing(data.capturing);
         }
       } catch {
         // ignore
       }
     };
 
-    poll();
-    const id = setInterval(poll, 2000);
-    return () => clearInterval(id);
+    const loop = async () => {
+      while (!cancelled) {
+        await poll();
+        if (cancelled) return;
+        await new Promise<void>((resolve) => {
+          timeoutId = window.setTimeout(resolve, 2000);
+        });
+      }
+    };
+
+    void loop();
+    return () => {
+      cancelled = true;
+      if (timeoutId != null) clearTimeout(timeoutId);
+    };
   }, [open]);

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/LogViewer.tsx` around lines 14 - 33, The poller in
useEffect (the poll function that fetches "/api/logs" and calls
setLogs/setCapturing) can overlap requests and allow stale responses to
overwrite newer state; modify poll to serialize requests by creating and using
an AbortController for each fetch and tracking an in-flight flag (or simple
mutex) so you skip starting a new fetch while one is pending, and on cleanup
abort any in-flight request via controller.abort(); ensure only responses from
the current non-aborted fetch update state (check controller.signal.aborted or
the in-flight flag before calling setLogs/setCapturing) and keep using
setInterval/clearInterval for scheduling.

dashboard/src/app.tsx (2)

100-107: ⚠️ Potential issue | 🟠 Major

Batch latency is still computed as ms/sec.

Line 103 samples total processing time as a rate and publishes it directly. That measures processing milliseconds per second, not milliseconds per batch, so the card inflates with throughput.

Suggested fix

-      // Batch latency: total processing time / batches
-      if (statsData.batches > 0) {
-        const totalSec = statsData.scan_sec + statsData.transform_sec + statsData.output_sec;
-        const latRate = rates.rate("lat", totalSec * 1000);
-        if (latRate != null) {
-          series[5].ring.push(latRate);
-          series[5].value = latRate.toFixed(1);
-        }
-      }
+      const totalMs =
+        (statsData.scan_sec + statsData.transform_sec + statsData.output_sec) * 1000;
+      const totalMsPerSec = rates.rate("lat_ms", totalMs);
+      const batchesPerSec = rates.rate("batches", statsData.batches);
+      if (totalMsPerSec != null && batchesPerSec != null && batchesPerSec > 0) {
+        const latencyMs = totalMsPerSec / batchesPerSec;
+        series[5].ring.push(latencyMs);
+        series[5].value = latencyMs.toFixed(1);
+      }

🤖 Prompt for AI Agents

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

In `@dashboard/src/app.tsx` around lines 100 - 107, The batch latency calculation
is using total processing time in ms per second instead of ms per batch; update
the computation so you divide the total time (totalSec * 1000) by
statsData.batches before calling rates.rate("lat", ...). In other words, compute
milliseconds per batch from statsData (using totalSec and statsData.batches),
pass that per-batch value into rates.rate, and then use the returned latRate to
push into series[5].ring and set series[5].value.

---

115-118: ⚠️ Potential issue | 🟠 Major

Serialize the main poll loop.

setInterval can start a new poll() before the previous one finishes. That lets older responses clobber newer state and makes rates.tick() run out of order.

Suggested fix

   useEffect(() => {
-    poll();
-    const id = setInterval(poll, POLL_MS);
-    return () => clearInterval(id);
+    let cancelled = false;
+    let timeoutId: number | undefined;
+
+    const loop = async () => {
+      while (!cancelled) {
+        await poll();
+        if (cancelled) return;
+        await new Promise<void>((resolve) => {
+          timeoutId = window.setTimeout(resolve, POLL_MS);
+        });
+      }
+    };
+
+    void loop();
+    return () => {
+      cancelled = true;
+      if (timeoutId != null) clearTimeout(timeoutId);
+    };
   }, [poll]);

🤖 Prompt for AI Agents

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

In `@dashboard/src/app.tsx` around lines 115 - 118, The current poll loop uses
setInterval which can call poll() again before the prior async invocation
finishes, causing out-of-order state updates (e.g., rates.tick()). Fix by
serializing poll calls: either replace the setInterval logic with an async
recursive loop that awaits poll() then waits POLL_MS via setTimeout (start by
calling runPollLoop() which awaits poll() then delays and repeats), or add a
running lock inside poll() (e.g., a boolean or mutex in the component scope
checked at start of poll and returned immediately if already running) so
concurrent executions cannot overlap; update references to poll() and
rates.tick() accordingly.

dashboard/src/style.css (1)

19-19: ⚠️ Potential issue | 🟠 Major

Quote the font-family names in --sans.

Line 19 still fails value-keyword-case for BlinkMacSystemFont and Roboto. Quote them here, then regenerate crates/logfwd-io/src/dashboard.html.

Suggested fix

-  --sans: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
+  --sans: -apple-system, "BlinkMacSystemFont", "Segoe UI", "Roboto", sans-serif;

🤖 Prompt for AI Agents

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

In `@dashboard/src/style.css` at line 19, The CSS variable --sans currently lists
unquoted font-family names causing value-keyword-case errors; update the
declaration of --sans to wrap BlinkMacSystemFont and Roboto in quotes (i.e.,
quote those font names in the --sans variable in style.css where --sans is
defined), then regenerate the dashboard HTML output
(crates/logfwd-io/src/dashboard.html) so the compiled HTML reflects the fixed
CSS.

crates/logfwd-io/src/dashboard.html (1)

8-8: ⚠️ Potential issue | 🟠 Major

The shipped bundle still uses clickable pipeline divs.

This file is served as-is, and the compiled .pn nodes are still div + onClick, so keyboard users cannot open inspectors. Fix dashboard/src/components/PipelineView.tsx in source and regenerate this artifact.

🤖 Prompt for AI Agents

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

In `@crates/logfwd-io/src/dashboard.html` at line 8, The compiled bundle shows
pipeline nodes rendered as <div> elements with onClick handlers (see produced
class `pn` and functions Pe/oe/Be), which are not keyboard accessible; update
the source component PipelineView (dashboard/src/components/PipelineView.tsx) to
render interactive pipeline items as semantic <button> (or an element with
role="button", tabindex="0") instead of plain <div>, move the onClick into an
onClick/onKeyDown pair that triggers the same handler (e.g., handleSelect / i =>
n(...)), add appropriate aria-label/aria-pressed or aria-expanded as needed,
ensure styles for `.pn` still apply to the new element, then rebuild/regenerate
the dashboard HTML so the bundle no longer emits clickable-only divs.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@dashboard/src/app.tsx`:
- Around line 28-36: The memory series in createSeries currently uses id "mem"
which doesn't match the byte-formatted cases in Chart.tsx; update the series
created in createSeries (function createSeries) to use the id that Chart.tsx
recognizes for memory bytes (replace id "mem" with "rss") so the Chart component
will apply the byte-scaling formatter to memory values and keep the
label/color/unit as intended.

In `@dashboard/src/components/LogViewer.tsx`:
- Around line 35-37: The auto-follow effect currently depends on logs.length
which stops firing once the buffer caps at 1000; update the effect so it watches
the logs array itself (or a monotonic update token you increment when snapshots
arrive) instead of logs.length — locate the useEffect that calls
bottomRef.current?.scrollIntoView({ behavior: "smooth" }) and change its
dependency from [logs.length] to [logs] (or to a logsVersion/update counter you
maintain when you setLogs) so new snapshots still trigger scrolling even when
length doesn't change.

---

Duplicate comments:
In `@crates/logfwd-io/src/dashboard.html`:
- Line 8: The compiled bundle shows pipeline nodes rendered as <div> elements
with onClick handlers (see produced class `pn` and functions Pe/oe/Be), which
are not keyboard accessible; update the source component PipelineView
(dashboard/src/components/PipelineView.tsx) to render interactive pipeline items
as semantic <button> (or an element with role="button", tabindex="0") instead of
plain <div>, move the onClick into an onClick/onKeyDown pair that triggers the
same handler (e.g., handleSelect / i => n(...)), add appropriate
aria-label/aria-pressed or aria-expanded as needed, ensure styles for `.pn`
still apply to the new element, then rebuild/regenerate the dashboard HTML so
the bundle no longer emits clickable-only divs.

In `@dashboard/src/app.tsx`:
- Around line 100-107: The batch latency calculation is using total processing
time in ms per second instead of ms per batch; update the computation so you
divide the total time (totalSec * 1000) by statsData.batches before calling
rates.rate("lat", ...). In other words, compute milliseconds per batch from
statsData (using totalSec and statsData.batches), pass that per-batch value into
rates.rate, and then use the returned latRate to push into series[5].ring and
set series[5].value.
- Around line 115-118: The current poll loop uses setInterval which can call
poll() again before the prior async invocation finishes, causing out-of-order
state updates (e.g., rates.tick()). Fix by serializing poll calls: either
replace the setInterval logic with an async recursive loop that awaits poll()
then waits POLL_MS via setTimeout (start by calling runPollLoop() which awaits
poll() then delays and repeats), or add a running lock inside poll() (e.g., a
boolean or mutex in the component scope checked at start of poll and returned
immediately if already running) so concurrent executions cannot overlap; update
references to poll() and rates.tick() accordingly.

In `@dashboard/src/components/LogViewer.tsx`:
- Around line 41-52: The log opener is a non-interactive div which is not
keyboard accessible; replace the <div class="log-prompt" onClick={() =>
setOpen(true)}> opener in LogViewer with a semantic <button> (or a component
that renders a button) so it supports keyboard/aria interaction, keep the
existing onClick handler bound to setOpen, preserve SVG and inner spans, and add
an accessible label/aria-label (e.g., "Open logs") and type="button" so screen
readers and keyboard users can open the panel.
- Around line 14-33: The poller in useEffect (the poll function that fetches
"/api/logs" and calls setLogs/setCapturing) can overlap requests and allow stale
responses to overwrite newer state; modify poll to serialize requests by
creating and using an AbortController for each fetch and tracking an in-flight
flag (or simple mutex) so you skip starting a new fetch while one is pending,
and on cleanup abort any in-flight request via controller.abort(); ensure only
responses from the current non-aborted fetch update state (check
controller.signal.aborted or the in-flight flag before calling
setLogs/setCapturing) and keep using setInterval/clearInterval for scheduling.

In `@dashboard/src/style.css`:
- Line 19: The CSS variable --sans currently lists unquoted font-family names
causing value-keyword-case errors; update the declaration of --sans to wrap
BlinkMacSystemFont and Roboto in quotes (i.e., quote those font names in the
--sans variable in style.css where --sans is defined), then regenerate the
dashboard HTML output (crates/logfwd-io/src/dashboard.html) so the compiled HTML
reflects the fixed CSS.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a63a48e3-47cc-4b13-8efe-271e9489d465

📥 Commits

Reviewing files that changed from the base of the PR and between 26c7e12 and 1aa51d1.

📒 Files selected for processing (4)

• crates/logfwd-io/src/dashboard.html
• dashboard/src/app.tsx
• dashboard/src/components/LogViewer.tsx
• dashboard/src/style.css

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Align the memory series contract with Chart.

Line 34 uses id: "mem", but dashboard/src/components/Chart.tsx only treats "bps" and "rss" as byte-formatted series. The Memory chart Y axis will therefore use the generic numeric formatter instead of byte scaling.

🤖 Prompt for AI Agents

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

In `@dashboard/src/app.tsx` around lines 28 - 36, The memory series in
createSeries currently uses id "mem" which doesn't match the byte-formatted
cases in Chart.tsx; update the series created in createSeries (function
createSeries) to use the id that Chart.tsx recognizes for memory bytes (replace
id "mem" with "rss") so the Chart component will apply the byte-scaling
formatter to memory values and keep the label/color/unit as intended.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Auto-follow stops once the log buffer fills.

stderr_capture caps the server buffer at 1000 lines. After that, new snapshots still have logs.length === 1000, so this effect stops firing and the viewer no longer follows live output.

Suggested fix

+  const scrollKey = `${logs.length}:${logs[0] ?? ""}:${logs[logs.length - 1] ?? ""}`;
   useEffect(() => {
     bottomRef.current?.scrollIntoView({ behavior: "smooth" });
-  }, [logs.length]);
+  }, [scrollKey]);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	useEffect(() => {
	bottomRef.current?.scrollIntoView({ behavior: "smooth" });
	}, [logs.length]);
	const scrollKey = `${logs.length}:${logs[0] ?? ""}:${logs[logs.length - 1] ?? ""}`;
	useEffect(() => {
	bottomRef.current?.scrollIntoView({ behavior: "smooth" });
	}, [scrollKey]);

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/LogViewer.tsx` around lines 35 - 37, The auto-follow
effect currently depends on logs.length which stops firing once the buffer caps
at 1000; update the effect so it watches the logs array itself (or a monotonic
update token you increment when snapshots arrive) instead of logs.length —
locate the useEffect that calls bottomRef.current?.scrollIntoView({ behavior:
"smooth" }) and change its dependency from [logs.length] to [logs] (or to a
logsVersion/update counter you maintain when you setLogs) so new snapshots still
trigger scrolling even when length doesn't change.

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (7)

dashboard/src/lib/format.ts (2)

31-33: ⚠️ Potential issue | 🟡 Minor

Use a visible missing-value token for null duration.

On Line 32, returning "" can render as a blank field and is inconsistent with other formatters using "-".

Proposed fix

 export function fmtDuration(s: number | null | undefined): string {
-  if (s == null) return "";
+  if (s == null) return "-";
   if (s >= 86400) return Math.floor(s / 86400) + "d " + Math.floor((s % 86400) / 3600) + "h";
   if (s >= 3600) return Math.floor(s / 3600) + "h " + Math.floor((s % 3600) / 60) + "m";
   if (s >= 60) return Math.floor(s / 60) + "m " + Math.floor(s % 60) + "s";
   return s + "s";
 }

🤖 Prompt for AI Agents

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

In `@dashboard/src/lib/format.ts` around lines 31 - 33, The fmtDuration function
returns an empty string for null/undefined which renders blank; change the
missing-value token to "-" to match other formatters by updating the null check
in fmtDuration (function name: fmtDuration) to return "-" instead of "" so
null/undefined durations show a visible placeholder; leave all other formatting
logic unchanged.

---

16-21: ⚠️ Potential issue | 🟡 Minor

Do not treat 0 bytes as missing.

On Line 17, b === 0 returning "-" hides valid zero-byte values as absent data.

Proposed fix

 export function fmtBytes(b: number | null | undefined): string {
-  if (b == null || b === 0) return "-";
+  if (b == null) return "-";
+  if (b === 0) return "0 B";
   if (b >= 1073741824) return (b / 1073741824).toFixed(1) + " GB";
   if (b >= 1048576) return (b / 1048576).toFixed(1) + " MB";
   if (b >= 1024) return (b / 1024).toFixed(1) + " KB";
   return b + " B";
 }

🤖 Prompt for AI Agents

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

In `@dashboard/src/lib/format.ts` around lines 16 - 21, The fmtBytes function
incorrectly treats 0 as missing; update the null-check in fmtBytes to only
consider null/undefined (remove the b === 0 check) so zero bytes are formatted
as "0 B" instead of "-"—locate the fmtBytes function and change the guard from
`if (b == null || b === 0) return "-"` to `if (b == null) return "-"`, leaving
the rest of the size-branching logic unchanged.

dashboard/src/components/PipelineView.tsx (2)

25-28: ⚠️ Potential issue | 🟠 Major

Render-time rate() call mutates tracker state.

RateTracker.rate() updates internal maps on every call. Invoking it during render (via compRate) corrupts tracking when re-renders occur between polls—subsequent calls see dt≈0 or wrong deltas.

Move rate computation to the poll cycle, or add a read-only getRate(key) accessor that returns cached EMA without mutation.

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/PipelineView.tsx` around lines 25 - 28, compRate is
calling RateTracker.rate() during render which mutates tracker state; stop
calling ratesRef.current.rate() from compRate and instead compute rates during
the polling/update cycle (where you already call the tracker) and store the
computed fmt values in component state or memo so render only reads them, or add
a read-only accessor getRate(key) on RateTracker that returns the cached EMA
without mutating maps and have compRate read from
ratesRef.current.getRate(`${comp.name}_${field}`); update usages of compRate to
read the precomputed or read-only values and remove any direct rate()
invocations from render paths.

---

34-48: ⚠️ Potential issue | 🟡 Minor

Missing key on Fragment in map iteration.

Preact/React requires keys on the outermost element returned from .map(). The Fragment <> lacks a key, which can cause reconciliation bugs.

🔧 Fix

+import { Fragment } from "preact";
 // ...
         {p.inputs.map((inp, i) => (
-          <>
+          <Fragment key={inp.name}>
             {i > 0 && <Arrow />}
             <div ...>
-          </>
+          </Fragment>
         ))}

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/PipelineView.tsx` around lines 34 - 48, The map
callback returns an unkeyed Fragment which breaks reconciliation; add a stable
key to the outer element returned by p.inputs.map — either give the Fragment a
key (e.g., use <Fragment key={`i${i}`}> and import Fragment from Preact/React)
or remove the Fragment and move a key onto the outermost <div class={`pn inp
...`}> (prefer using a stable identifier like inp.id if available rather than
the index). Ensure the key value matches the selection id pattern (`i${i}`) if
you keep using the index-based IDs.

dashboard/src/components/LogViewer.tsx (2)

35-37: ⚠️ Potential issue | 🟡 Minor

Auto-scroll breaks once log buffer caps at 1000 lines.

The server caps stderr at 1000 lines. Once full, logs.length stays constant and this effect stops firing—new content won't scroll into view.

🔧 Use content fingerprint instead of length

+  const scrollKey = logs.length > 0
+    ? `${logs.length}:${logs[logs.length - 1]}`
+    : "";
+
   useEffect(() => {
     bottomRef.current?.scrollIntoView({ behavior: "smooth" });
-  }, [logs.length]);
+  }, [scrollKey]);

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/LogViewer.tsx` around lines 35 - 37, The effect in
LogViewer uses [logs.length] so when the server caps stderr at 1000 lines the
dependency stops changing and auto-scroll breaks; change the dependency to a
content fingerprint that changes when new log content arrives (e.g., compute a
memoized key from logs such as the last log's id/timestamp or a short hash/JSON
of the logs via useMemo/useState) and use that fingerprint in the useEffect
dependency instead of logs.length; update the useEffect in the LogViewer
component that references bottomRef.current?.scrollIntoView to depend on the new
fingerprint (and ensure the fingerprint updates whenever logs change, even if
length is constant).

---

14-33: ⚠️ Potential issue | 🟠 Major

Polling race condition: overlapping requests can cause state jumps.

setInterval(poll, 2000) fires every 2s regardless of whether the previous poll() completed. If a slow response arrives after a newer one, setLogs(data.lines) overwrites fresh data with stale data, causing visible flicker/jumping.

🔧 Self-scheduling setTimeout pattern

   useEffect(() => {
     if (!open) return;
+    let cancelled = false;

     const poll = async () => {
       try {
         const res = await fetch("/api/logs");
         if (res.ok) {
           const data: LogsResponse = await res.json();
-          setLogs(data.lines);
-          setCapturing(data.capturing);
+          if (!cancelled) {
+            setLogs(data.lines);
+            setCapturing(data.capturing);
+          }
         }
       } catch {
         // ignore
       }
+      if (!cancelled) setTimeout(poll, 2000);
     };

     poll();
-    const id = setInterval(poll, 2000);
-    return () => clearInterval(id);
+    return () => { cancelled = true; };
   }, [open]);

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/LogViewer.tsx` around lines 14 - 33, The current
LogViewer useEffect uses setInterval to call poll which can overlap and let slow
responses overwrite newer state; replace the setInterval pattern in the
LogViewer component with a self-scheduling async loop: create an async function
(e.g., pollLoop) that awaits fetch("/api/logs"), updates setLogs/setCapturing
only when still mounted, then waits 2000ms before re-running (or uses setTimeout
to schedule the next iteration), and expose a cancellation flag or
AbortController in the useEffect cleanup to stop the loop; alternatively add a
monotonically increasing requestId inside poll and ignore responses whose id is
less than the latest to prevent stale responses from applying (ensure cleanup
clears any pending timeout/abort).

dashboard/src/style.css (1)

19-19: ⚠️ Potential issue | 🟡 Minor

Quote font family names to satisfy stylelint.

Stylelint's value-keyword-case rule flags unquoted BlinkMacSystemFont and Roboto.

🔧 Fix

-  --sans: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
+  --sans: -apple-system, "BlinkMacSystemFont", "Segoe UI", "Roboto", sans-serif;

🤖 Prompt for AI Agents

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

In `@dashboard/src/style.css` at line 19, The CSS variable --sans currently lists
unquoted font family identifiers that trip stylelint; update the variable
declaration (the --sans custom property) to quote the BlinkMacSystemFont and
Roboto family names (and any other multi-word or non-standard identifiers if
flagged) so the value becomes valid for stylelint's value-keyword-case
rule—i.e., edit the --sans declaration to use quoted names like
"BlinkMacSystemFont" and "Roboto" while preserving the existing order and other
values.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@dashboard/src/components/PipelineView.tsx`:
- Around line 37-46: Clickable pipeline node elements (the divs with class "pn"
in PipelineView using sel and toggle, and similar transform/output node blocks)
are not keyboard-accessible; update them to be semantic buttons or add proper
keyboard support: replace the interactive <div class="pn"> nodes with <button>
elements or add role="button", tabIndex={0}, and an onKeyDown handler that
invokes toggle when Enter/Space is pressed, and ensure aria-pressed or
aria-selected reflects the sel state; apply the same change to the transform and
output node blocks so keyboard users can focus and activate nodes.

In `@dashboard/src/style.css`:
- Around line 232-248: The .pn pipeline node styles lack a :focus-visible rule
for keyboard users; add a .pn:focus-visible selector that applies an accessible
focus ring (e.g., outline and outline-offset using --accent) so keyboard focus
is visible and consistent with .pn.selected and :hover states; update the CSS
near the .pn, .pn:hover, and .pn.selected rules to include this new
.pn:focus-visible rule to ensure keyboard accessibility.

In `@dashboard/src/types.ts`:
- Around line 19-29: The PipelineData interface is out of sync with the
/api/pipelines response: update the PipelineData type to replace the scalar
fields batches, scan_sec, transform_sec, output_sec and backpressure_stalls with
the actual nested shapes returned by the API — i.e., change batches from number
to an object with properties total, avg_rows, flush_by_size, flush_by_timeout,
dropped_batches_total, scan_errors_total; remove backpressure_stalls; and
replace scan_sec/transform_sec/output_sec with a stage_seconds object containing
scan, transform, and output numeric fields (keep name PipelineData and adjust
any usages expecting the old flat fields).

---

Duplicate comments:
In `@dashboard/src/components/LogViewer.tsx`:
- Around line 35-37: The effect in LogViewer uses [logs.length] so when the
server caps stderr at 1000 lines the dependency stops changing and auto-scroll
breaks; change the dependency to a content fingerprint that changes when new log
content arrives (e.g., compute a memoized key from logs such as the last log's
id/timestamp or a short hash/JSON of the logs via useMemo/useState) and use that
fingerprint in the useEffect dependency instead of logs.length; update the
useEffect in the LogViewer component that references
bottomRef.current?.scrollIntoView to depend on the new fingerprint (and ensure
the fingerprint updates whenever logs change, even if length is constant).
- Around line 14-33: The current LogViewer useEffect uses setInterval to call
poll which can overlap and let slow responses overwrite newer state; replace the
setInterval pattern in the LogViewer component with a self-scheduling async
loop: create an async function (e.g., pollLoop) that awaits fetch("/api/logs"),
updates setLogs/setCapturing only when still mounted, then waits 2000ms before
re-running (or uses setTimeout to schedule the next iteration), and expose a
cancellation flag or AbortController in the useEffect cleanup to stop the loop;
alternatively add a monotonically increasing requestId inside poll and ignore
responses whose id is less than the latest to prevent stale responses from
applying (ensure cleanup clears any pending timeout/abort).

In `@dashboard/src/components/PipelineView.tsx`:
- Around line 25-28: compRate is calling RateTracker.rate() during render which
mutates tracker state; stop calling ratesRef.current.rate() from compRate and
instead compute rates during the polling/update cycle (where you already call
the tracker) and store the computed fmt values in component state or memo so
render only reads them, or add a read-only accessor getRate(key) on RateTracker
that returns the cached EMA without mutating maps and have compRate read from
ratesRef.current.getRate(`${comp.name}_${field}`); update usages of compRate to
read the precomputed or read-only values and remove any direct rate()
invocations from render paths.
- Around line 34-48: The map callback returns an unkeyed Fragment which breaks
reconciliation; add a stable key to the outer element returned by p.inputs.map —
either give the Fragment a key (e.g., use <Fragment key={`i${i}`}> and import
Fragment from Preact/React) or remove the Fragment and move a key onto the
outermost <div class={`pn inp ...`}> (prefer using a stable identifier like
inp.id if available rather than the index). Ensure the key value matches the
selection id pattern (`i${i}`) if you keep using the index-based IDs.

In `@dashboard/src/lib/format.ts`:
- Around line 31-33: The fmtDuration function returns an empty string for
null/undefined which renders blank; change the missing-value token to "-" to
match other formatters by updating the null check in fmtDuration (function name:
fmtDuration) to return "-" instead of "" so null/undefined durations show a
visible placeholder; leave all other formatting logic unchanged.
- Around line 16-21: The fmtBytes function incorrectly treats 0 as missing;
update the null-check in fmtBytes to only consider null/undefined (remove the b
=== 0 check) so zero bytes are formatted as "0 B" instead of "-"—locate the
fmtBytes function and change the guard from `if (b == null || b === 0) return
"-"` to `if (b == null) return "-"`, leaving the rest of the size-branching
logic unchanged.

In `@dashboard/src/style.css`:
- Line 19: The CSS variable --sans currently lists unquoted font family
identifiers that trip stylelint; update the variable declaration (the --sans
custom property) to quote the BlinkMacSystemFont and Roboto family names (and
any other multi-word or non-standard identifiers if flagged) so the value
becomes valid for stylelint's value-keyword-case rule—i.e., edit the --sans
declaration to use quoted names like "BlinkMacSystemFont" and "Roboto" while
preserving the existing order and other values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c498051c-4b1c-44e1-ac95-3e57d2a8994b

📥 Commits

Reviewing files that changed from the base of the PR and between 1aa51d1 and b911300.

📒 Files selected for processing (7)

• crates/logfwd-io/src/dashboard.html
• dashboard/src/components/LogViewer.tsx
• dashboard/src/components/PipelineView.tsx
• dashboard/src/lib/format.ts
• dashboard/src/lib/rates.ts
• dashboard/src/style.css
• dashboard/src/types.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Pipeline nodes lack keyboard accessibility.

The .pn divs are clickable but unreachable via keyboard. Add button semantics or use actual <button> elements.

🔧 Option: Add keyboard support to divs

             <div
               class={`pn inp ${sel === `i${i}` ? "selected" : ""}`}
               onClick={() => toggle(`i${i}`)}
+              role="button"
+              tabIndex={0}
+              onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') toggle(`i${i}`); }}
             >

Apply similarly to transform and output nodes.

Also applies to: 69-81

🤖 Prompt for AI Agents

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

In `@dashboard/src/components/PipelineView.tsx` around lines 37 - 46, Clickable
pipeline node elements (the divs with class "pn" in PipelineView using sel and
toggle, and similar transform/output node blocks) are not keyboard-accessible;
update them to be semantic buttons or add proper keyboard support: replace the
interactive <div class="pn"> nodes with <button> elements or add role="button",
tabIndex={0}, and an onKeyDown handler that invokes toggle when Enter/Space is
pressed, and ensure aria-pressed or aria-selected reflects the sel state; apply
the same change to the transform and output node blocks so keyboard users can
focus and activate nodes.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Pipeline node styles need :focus-visible for keyboard users.

With keyboard support added to .pn elements, add focus styling:

.pn:focus-visible {
  outline: 2px solid var(--accent);
  outline-offset: 2px;
}

🤖 Prompt for AI Agents

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

In `@dashboard/src/style.css` around lines 232 - 248, The .pn pipeline node styles
lack a :focus-visible rule for keyboard users; add a .pn:focus-visible selector
that applies an accessible focus ring (e.g., outline and outline-offset using
--accent) so keyboard focus is visible and consistent with .pn.selected and
:hover states; update the CSS near the .pn, .pn:hover, and .pn.selected rules to
include this new .pn:focus-visible rule to ensure keyboard accessibility.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the actual /api/pipelines response structure from backend
rg -A 30 'fn serve_pipelines' crates/logfwd-io/src/diagnostics.rs | head -60

Repository: strawgate/memagent

Length of output: 1186

---

🏁 Script executed:

#!/bin/bash
# Get the complete serve_pipelines function to see response structure
rg -A 100 'fn serve_pipelines' crates/logfwd-io/src/diagnostics.rs | grep -A 80 'stage_seconds\|batches\|scan_sec\|output_sec\|transform_sec\|backpressure'

Repository: strawgate/memagent

Length of output: 2603

---

PipelineData interface does not match the actual /api/pipelines response structure.

The backend returns:

• batches as a nested object with total, avg_rows, flush_by_size, flush_by_timeout, dropped_batches_total, scan_errors_total — not a single number
• Timing data nested in stage_seconds object with scan, transform, output — not flat scan_sec, transform_sec, output_sec fields
• No backpressure_stalls field in the response

Update PipelineData to match the actual response structure or the frontend will fail to deserialize the API response correctly.

🤖 Prompt for AI Agents

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

In `@dashboard/src/types.ts` around lines 19 - 29, The PipelineData interface is
out of sync with the /api/pipelines response: update the PipelineData type to
replace the scalar fields batches, scan_sec, transform_sec, output_sec and
backpressure_stalls with the actual nested shapes returned by the API — i.e.,
change batches from number to an object with properties total, avg_rows,
flush_by_size, flush_by_timeout, dropped_batches_total, scan_errors_total;
remove backpressure_stalls; and replace scan_sec/transform_sec/output_sec with a
stage_seconds object containing scan, transform, and output numeric fields (keep
name PipelineData and adjust any usages expecting the old flat fields).

[github-actions]

Requesting changes for one confirmed functional issue: the chart lifecycle currently destroys/recreates uPlot on each render, which causes unnecessary churn and state reset under the regular polling re-renders.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:#526 (pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)

---

What is this? | From workflow: AI: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[github-actions]

useEffect runs after every render here and its cleanup destroys the plot (plot.destroy()), so the chart is torn down and recreated on each parent re-render instead of being incrementally updated.

In this PR, App calls forceUpdate on each poll tick, which means this component re-renders continuously; that repeatedly rebuilds uPlot/ResizeObserver, resets chart interaction state, and adds avoidable CPU churn.

Please split this into:

• a mount/unmount effect (useEffect(..., [])) that creates the plot + observer and only destroys on unmount
• an update effect that only calls setData/setScale when series data changes