Skip to content

fix: touchstone empty results table for branches with a slash#2689

Merged
schochastics merged 1 commit into
mainfrom
fix-touchstone-slash-branches
Jun 4, 2026
Merged

fix: touchstone empty results table for branches with a slash#2689
schochastics merged 1 commit into
mainfrom
fix-touchstone-slash-branches

Conversation

@schochastics
Copy link
Copy Markdown
Contributor

Problem

Touchstone now posts PR comments, but the results table is empty — only the header/footer render (see example).

Root cause

The benchmarks run fine and records are uploaded, but benchmark_analyze() discards all of them with:

Ignoring all benchmarks that don't have exactly those two branches.

The downloaded results artifact shows why — records for a head branch like feat/print-style-modern are stored as:

records/graph_from_data_frame/main
records/graph_from_data_frame/feat/print-style-modern   ← the "/" nests a directory

The receive action installs the {touchstone} R package from its touchstone_ref input, which defaults to @v1. That tag predates the upstream commits "support branch names with slashes" / "encode all filesystem-unsafe characters in branch names" (2025-11-27), so it has no branch_encode(). The raw / in the branch name nests a subdirectory, benchmark_analyze() can't match a benchmark that has exactly the base+head branches, and every benchmark is dropped → empty table.

We previously pinned the action wrapper to a default-branch SHA (7e6072a) to get artifact@v4/v5, but the R package was still installed from @v1. This is the remaining half of that version mismatch.

Fix

Pin touchstone_ref to the same commit the action wrapper already uses, so the installed R package includes branch_encode()/branch_decode().

Only affects R/** etc. via the receive trigger; this PR's own branch (fix-touchstone-slash-branches, no slash) wouldn't have surfaced the bug, so once merged a slash-named branch is the real end-to-end test.

🤖 Generated with Claude Code

Benchmark comments rendered with an empty results table for any PR whose
head branch contains a "/" (e.g. feat/print-style-modern). The receive
action installs the {touchstone} R *package* from `touchstone_ref`, which
defaults to `@v1` -- a tag cut before the upstream fix that percent-encodes
filesystem-unsafe characters in branch names. Without it, records are
written to records/<benchmark>/feat/print-style-modern, the slash nests a
directory, and benchmark_analyze() can't find a benchmark with exactly the
base+head branches, so it discards them all ("Ignoring all benchmarks that
don't have exactly those two branches") and emits only the header/footer.

Pin touchstone_ref to the same commit the action wrapper already uses, which
includes branch_encode()/branch_decode().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@schochastics schochastics enabled auto-merge (squash) June 4, 2026 17:38
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

This is how benchmark results would change (along with a 95% confidence interval in relative change) if c4cf214 is merged into main:

  • ✔️as_adjacency_matrix: 783ms -> 780ms [-1.48%, +0.74%]
  • ✔️as_biadjacency_matrix: 793ms -> 791ms [-1.25%, +0.72%]
  • ✔️as_data_frame_both: 1.62ms -> 1.6ms [-3.58%, +0.81%]
  • ✔️as_long_data_frame: 4.03ms -> 4.04ms [-2.03%, +2.52%]
  • ✔️es_attr_filter: 3ms -> 2.94ms [-5.85%, +2.21%]
  • ✔️graph_from_adjacency_matrix: 145ms -> 144ms [-2.07%, +0.16%]
  • ❗🐌graph_from_data_frame: 3.6ms -> 3.64ms [+0.23%, +2.18%]
  • ✔️vs_attr_filter: 1.73ms -> 1.75ms [-1.47%, +4.12%]
  • ✔️vs_by_name: 1.15ms -> 1.16ms [-4.82%, +7.76%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

auto-merge was automatically disabled June 4, 2026 17:56

Repository rule violations found

@schochastics schochastics merged commit a8ff9f7 into main Jun 4, 2026
9 checks passed
@schochastics schochastics deleted the fix-touchstone-slash-branches branch June 4, 2026 18:56
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