Improve LLGo binary-size dashboard

[zhouguangyuan0718]

Summary

• Transpose the binary-size matrix so commits are paginated rows and stable benchmark/build-mode fields are horizontally scrollable columns.
• Keep each LLGo result relative to Go, and show per-cell A→B deltas on the selected commit row.
• Add interactive history filters for benchmarks, build modes, metrics, and history range.
• Run the full five-way benchmark matrix on PRs that change the LLGo pin, Bent, LLGo-size cases/configuration, or referenced suite definitions; publishing remains restricted to main.

Validation

• Node syntax check, workflow YAML parse, and git diff check.
• Local browser validation against the real Pages history data, including A/B selection and the commit-delta metric.

[fennoai]

FennoAI review — PR #6 · Improve LLGo binary-size dashboard

This PR reworks the client-side dashboard (app.js), the CI workflows, and publish.sh. Overall the change is solid: the security review found no XSS or injection issues (all untrusted JSON fields go through escapeHtml; the repository_dispatch handler validates the source repo and 40-hex commit SHA and passes values via env: rather than ${{ }} interpolation; publish.sh sanitizes the run key against a strict regex before using it as a path component). Findings below; the inline comments carry the highest-confidence items.

Highest priority

• app.js:309 — likely TypeError when the baseline run is null (see inline). renderInspector guards only !newer, then reads baseline.run directly; loadRun returns null for an unresolved meta, so a stale/empty A-side selection throws before the || {} fallback applies.

Documentation (handoff doc only — README is accurate)

• docs/llgo-binary-size-handoff.md:25 (outside the diff hunk, so noted here): the doc still says concurrency.group: llgo-binary-size-pages, but the workflow now uses llgo-binary-size-build (.github/workflows/llgo-binary-size.yml:40) with a separate Pages queue. This also contradicts the newly-added lines 43–45 in the same file. Consider bumping the 更新时间 (line 3) and updating step 1's "启动一次构建和 Pages 发布" wording — update-pin triggers a workflow_dispatch build; Pages publish is a conditional later step, not directly started by the dispatch.
• The stale run-directory keying (<run>-<attempt>) is flagged inline at line 35.

publish.sh (maintainability / robustness — non-blocking)

• Consolidation runs on every publish: the python3 block re-scans and re-parses every historical run directory on each publish, effectively a one-time migration executed indefinitely. Consider gating it to run only when a legacy (non-commit-keyed) layout is detected.
• Lexicographic createdAt tie-break (publish.sh:~59): when two dirs map to the same commit key, "newer wins" uses str(createdAt) >= str(...). This is only correct for consistently-formatted zero-padded ISO-8601 timestamps; an empty or differently-formatted createdAt silently breaks the intent. A short comment asserting the format (or parsing to a datetime) would harden it.

CI (defense-in-depth — informational)

• llgo-binary-size.yml:129-136: the binary-size job declares job-level permissions: contents: write, but the write is only needed by the main-gated publish step. The build/clone/CMake steps execute untrusted upstream LLGo/Bent code while a write-capable token is in the environment. Splitting publication into its own job (as done for update-pin) would remove the write token during untrusted-build execution. On fork PRs the token is read-only, so this affects same-repo/dispatch runs.

Performance (minor)

• The redundant benchmarkMap rebuild is flagged inline at app.js:262. Other minor items (no debounce on the filter input handler; full renderHistory re-render on A/B-only changes; cache: "no-store" on immutable per-run JSON) are low-impact at current history sizes.

No blocking issues. Non-approving review (COMMENT).

Findings without inline locations

• docs/llgo-binary-size-handoff.md:35: Stale run-directory keying. This still says results are archived to pages/data/runs/<run>-<attempt>/, but publish.sh:79 now keys the directory by the full LLGo commit (run.llgoCommit || sourceCommit || id || "manual"). This contradicts the newly-added lines 44–45 of this same file. Suggest updating to pages/data/runs/<llgoCommit>/.
• ci/llgo-size/llgo-version.env:6: Missing trailing newline (\ No newline at end of file in the diff). Harmless for source, but a POSIX text-file smell and a risk for the sed/awk edits the update-pin job performs on this file. Suggest adding the trailing newline.

[fennoai]

Likely TypeError when the baseline run is null. renderInspector guards only !newer (line 298), then here reads baseline.run directly. baseline is loadRun(selected.baselineMeta), and loadRun returns null when its meta is falsy (line 108). If baselineSelect.value doesn't resolve to a meta (e.g. a stale value after the run list is filtered, or an empty A-side), baseline is null and baseline.run throws before the || {} fallback can apply — the || {} only protects a missing .run property, not a null baseline.

Suggest guarding explicitly, e.g. commitLabel((baseline && baseline.run) || {}) (and likewise (newer && newer.run) || {}).

[fennoai]

Two small things on this line:

1. Redundant benchmarkMap rebuild (performance). renderHistory calls metricValue once per active benchmark × active config, and each call rebuilds benchmarkMap(document) for every document. That's benchmarks × configs × documents Map constructions per history render instead of one per document. Consider building documents.map(benchmarkMap) once in renderHistory and passing the pre-built maps in.
2. Parameter named document shadows the global document (also in renderHistory/renderComparison/benchmarkMap). It works today only because these closures don't touch the DOM; a future edit needing document.querySelector here would silently operate on a run object. Consider renaming to doc/runDoc.