Skip to content

Fix benchmark comparison table rendering#1399

Merged
sbfnk merged 1 commit into
mainfrom
fix/benchmark-table-formatting
May 11, 2026
Merged

Fix benchmark comparison table rendering#1399
sbfnk merged 1 commit into
mainfrom
fix/benchmark-table-formatting

Conversation

@sbfnk-bot
Copy link
Copy Markdown
Collaborator

@sbfnk-bot sbfnk-bot commented May 11, 2026

Description

This PR closes #1398.

knitr::kable() returns a character vector with one element per row. cat() was joining those rows with its default single-space separator, so the markdown table came out as a single long line and rendered as plain text in the benchmark comment (see the table in #1396 (comment)). Passing sep = "\n" puts each row on its own line so GitHub renders it as a table.

Initial submission checklist

  • My PR is based on a package issue and I have explicitly linked it.
  • I have tested my changes locally (using devtools::test() and devtools::check()).
  • I have added or updated unit tests where necessary.
  • I have updated the documentation if required and rebuilt docs if yes (using devtools::document()).
  • I have followed the established coding standards (and checked using lintr::lint_package()).
  • I have added a news item linked to this PR.

The change is to a developer-only script under inst/dev/ that is used by CI to post benchmark comments on PRs — it is not user-facing, so no NEWS item and no unit tests.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed Markdown report formatting in benchmark result generation to ensure proper line separation in the output file.

Review Change Stack

cat() defaults to joining with a single space, so knitr::kable() output was emitted as one line. Use sep = "\n" so each table row stays on its own line.

Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
@sbfnk-bot sbfnk-bot requested a review from sbfnk May 11, 2026 09:20
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8af02956-8650-46d0-a256-5eefd4cb8985

📥 Commits

Reviewing files that changed from the base of the PR and between 2d45ab9 and 709f55b.

📒 Files selected for processing (1)
  • inst/dev/benchmark-compare.R

Walkthrough

The benchmark comparison script's Markdown report output is adjusted to include a separator parameter in the cat() function when writing the knitr::kable table. This affects how table elements are concatenated into the benchmark-results.md file.

Changes

Benchmark Report Output Formatting

Layer / File(s) Summary
Markdown Report Output
inst/dev/benchmark-compare.R
The cat(knitr::kable(...), ...) call is adjusted with sep = "\n" to control how the table output is joined in the sink stream.

Possibly related PRs

  • epiforecasts/EpiNow2#1189: Earlier modifications to the same benchmark comparison script; this PR refines the markdown output formatting introduced or modified in that earlier PR.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarises the main change: fixing how benchmark comparison tables are rendered in the markdown output.
Linked Issues check ✅ Passed The code change directly addresses issue #1398 by adding proper newline separators to the cat() call, ensuring markdown tables render correctly in benchmark comments.
Out of Scope Changes check ✅ Passed The change is confined to a single developer-only script under inst/dev/ and directly addresses the linked issue with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/benchmark-table-formatting

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

❤️ Share

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

@sbfnk sbfnk marked this pull request as ready for review May 11, 2026 09:22
@sbfnk
Copy link
Copy Markdown
Contributor

sbfnk commented May 11, 2026

minor workflow change, all fine

@sbfnk sbfnk merged commit e580eab into main May 11, 2026
11 checks passed
@sbfnk sbfnk deleted the fix/benchmark-table-formatting branch May 11, 2026 09:24
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.

Benchmarking formatting is broken

2 participants