Skip to content

webchannel/benchmark: tweak reports - #105

Merged
pmodular merged 1 commit into
mainfrom
push-qnvouqqrtkyw
Oct 14, 2026
Merged

pmodular merged 1 commit into
mainfrom
push-qnvouqqrtkyw

Conversation

@pmodular

Copy link
Copy Markdown
Collaborator

No description provided.

@pmodular
pmodular requested a review from wenbozhu October 14, 2026 14:27

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request moves the "Benchmark elapsed time by delay" table from the benchmark reports (report.md) to the artifacts metadata (artifacts.md) across several benchmark runs, updating the generator script summarize-delay-sweep.py and documentation (AGENTS.md, README.md) accordingly. The review feedback suggests using defensive dictionary lookups with fallback values in summarize-delay-sweep.py to prevent potential KeyError exceptions when parsing incomplete or modified timing data.

Comment on lines +223 to +224
for delay in delays:
artifacts.append(f"| {delay} | {timing['delays'][str(delay)]['elapsedSeconds']:.3f} |")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential KeyError exceptions when processing incomplete or manually modified timing.json files, use defensive .get() lookups with a fallback placeholder (e.g., "—") for missing delay timing data.

Suggested change
for delay in delays:
artifacts.append(f"| {delay} | {timing['delays'][str(delay)]['elapsedSeconds']:.3f} |")
for delay in delays:
delay_seconds = timing.get("delays", {}).get(str(delay), {}).get("elapsedSeconds")
val = f"{delay_seconds:.3f}" if delay_seconds is not None else "—"
artifacts.append(f"| {delay} | {val} |")

@pmodular
pmodular merged commit 23f809d into main Oct 14, 2026
8 checks passed
@pmodular
pmodular deleted the push-qnvouqqrtkyw branch October 14, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant