chore(benchmarks): scrub committed home-directory paths and stop the harnesses re-emitting them - #7165
Conversation
📝 WalkthroughWalkthroughBenchmark tooling now sanitizes absolute paths into repository-relative or home-relative forms. Existing benchmark artifacts and documentation were updated to remove machine-specific paths without changing measurements. ChangesPath portability
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BenchmarkScript
participant PathNormalizer
participant MetadataArtifact
BenchmarkScript->>PathNormalizer: provide repository root and paths
PathNormalizer->>PathNormalizer: resolve and sanitize paths
PathNormalizer->>MetadataArtifact: write portable executable and command paths
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
3fb241d to
ff7a46d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/compare.sh`:
- Around line 153-159: Update the command expansion before the Python heredoc to
use the Bash array-preserving form "${NODE_CMD[@]}" instead of "${NODE_CMD[*]}".
Keep the existing argument ordering and Python command-normalization logic
unchanged so multi-argument commands such as node --experimental-strip-types
remain separate arguments.
In `@benchmarks/public_baseline.py`:
- Around line 96-113: Update portable_path in benchmarks/public_baseline.py
(lines 96-113) and the corresponding normalizers in
benchmarks/gc_ratchet/gc_ratchet.py (lines 181-195), benchmarks/compare.sh
(lines 161-170), and benchmarks/honest_bench/harness/run_bench.sh (lines 97-106)
to expand ~ before realpath and anchor relative inputs to the repository root
before resolving. Use ROOT/REPO_ROOT in the Python scripts, root in compare.sh,
and repo_root in run_bench.sh; preserve existing repo/home portability behavior
for absolute inputs.
In `@Cargo.toml`:
- Line 295: Revert the release metadata edits: restore the workspace package
version in Cargo.toml at lines 295-295 and restore the documented current
version in CLAUDE.md at lines 11-11. Leave the PR-keyed changelog fragment
unchanged; these version fields are maintainer-owned.
In `@CHANGELOG.md`:
- Line 5686: Revert all historical portability edits in CHANGELOG.md: restore
the original entries at lines 5686, 8938, 9020, 10506, 10521, and 10561. Keep
portability updates in the current changelog fragment or documentation instead;
do not modify the frozen top-level changelog release entries.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4132f12a-2f6b-4bcc-8ccd-b95764b1a50d
📒 Files selected for processing (14)
CHANGELOG.mdCLAUDE.mdCargo.tomlbenchmarks/compare.shbenchmarks/gc_ratchet/baseline/gc-ratchet-v1.jsonbenchmarks/gc_ratchet/gc_ratchet.pybenchmarks/honest_bench/harness/run_bench.shbenchmarks/honest_bench/results/results.jsonbenchmarks/public_baseline.pybenchmarks/results/public-node-bun-v1.jsonchangelog.d/7165-scrub-absolute-home-paths.mddocs/ecs-perf-case-study.mddocs/history/OPTIMIZATION_PLAN.mddocs/history/UI_ADJSUTMENTS.md
| "${NODE_CMD[*]}" "$ROOT" <<'PY' | ||
| import json | ||
| import os | ||
| import sys | ||
|
|
||
| path, perry, has_node, has_bun, perry_version, node_version, bun_version, node_command = sys.argv[1:] | ||
| path, perry, has_node, has_bun, perry_version, node_version, bun_version, node_command, root = sys.argv[1:] | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -C 5 'NODE_CMD' benchmarks/compare.shRepository: PerryTS/perry
Length of output: 1855
🌐 Web query:
Bash array expansion ${array[*]} ${array[@]} behavior spaces elements
💡 Result:
In Bash, the difference between ${array[]} and ${array[@]} relates to how array elements are expanded into words, particularly when enclosed in double quotes [1][2]. Unquoted behavior: When not enclosed in double quotes, both ${array[]} and ${array[@]} behave identically [3]. They expand all elements of the array into separate words, where word splitting is performed based on the characters in the IFS (Internal Field Separator) variable [4][5]. Consequently, if array elements contain spaces, they will be split into multiple words [1][3]. Quoted behavior: When enclosed in double quotes, the expansion behavior differs significantly [2][6]: "${array[@]}" This form expands each element of the array into a separate word [1][2]. It preserves the boundaries of each element, even if the element contains spaces, tabs, or newlines [7][8]. This is the recommended method for iterating over arrays or passing all array elements to a command while maintaining their integrity [7][8]. "${array[]}" This form expands the entire array into a single word [1][2]. The elements are concatenated, separated by the first character of the IFS special variable (which defaults to a space) [4][2]. This is primarily useful when you intend to convert the entire array into a single string for output or processing [7][8]. Summary table: Expansion | Quoted ("") | Unquoted --- | --- | --- ${array[@]} | Expands to separate words (preserves elements) | Expands to separate words (subject to word splitting) ${array[]} | Expands to a single word (joined by IFS) | Expands to separate words (subject to word splitting) As a best practice, always quote array expansions (e.g., "${array[@]}") to prevent unintended word splitting and globbing, unless you specifically require the single-string concatenation behavior of "${array[*]}" [5][7][8].
Citations:
- 1: https://unix.stackexchange.com/questions/689596/what-is-the-difference-between-array-and-array-when-use-each-one-ov
- 2: https://www.gnu.org.cach3.com/software/bash/manual/html_node/Arrays.html
- 3: https://stackoverflow.com/questions/16627986/bash-array-variables-or
- 4: https://unix.stackexchange.com/questions/135010/what-is-the-difference-between-and-when-referencing-bash-array-values
- 5: https://unix.stackexchange.com/questions/562659/why-is-array-expanded-into-multiple-words-when-its-quoted
- 6: https://bash.chrootid.com/Arrays.html
- 7: https://mywiki.wooledge.org/BashGuide/Arrays
- 8: https://mywiki.wooledge.org/BashFAQ/005
Use "${NODE_CMD[@]}" before passing the command to Python.
NODE_CMD is a Bash array, so "${NODE_CMD[*]}" expands to one argument. If the chosen command is node --experimental-strip-types, Python receives sys.argv[7] as node --experimental-strip-types instead of a two-argument command list, breaking the command normalization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/compare.sh` around lines 153 - 159, Update the command expansion
before the Python heredoc to use the Bash array-preserving form "${NODE_CMD[@]}"
instead of "${NODE_CMD[*]}". Keep the existing argument ordering and Python
command-normalization logic unchanged so multi-argument commands such as node
--experimental-strip-types remain separate arguments.
|
|
||
| [workspace.package] | ||
| version = "0.5.1269" | ||
| version = "0.5.1266" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked file relevant lines:\n'
python3 - <<'PY'
for path, lines in (('Cargo.toml', [295]), ('CLAUDE.md', [11])):
print(f'-- {path} --')
try:
with open(path, 'r', encoding='utf-8') as f:
data = f.read()
for line in lines:
content = data.splitlines()[line-1]
print(f'{line}: {content}')
except Exception as e:
print(f'ERROR reading {path}: {e}')
PY
printf '\nChangelog fragments and release metadata mentions:\n'
for pat in 'changelog.d/*' 'CLAUDE.md' 'Cargo.toml'; do
printf '\n-- glob: %s --\n' "$pat"
git ls-files "$pat" | sed -n '1,200p'
done
printf '\nVersion mentions (limited):\n'
rg -n "0\.5\.1266|version = \"|Current Version|Current:" Cargo.toml CLAUDE.md changelog.d 2>/dev/null | sed -n '1,200p'
printf '\nDiff stat and changed files:\n'
git diff --stat HEAD 2>/dev/null || true
git diff --name-only HEAD 2>/dev/null | sed -n '1,200p' || trueRepository: PerryTS/perry
Length of output: 10915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'PR metadata:\n'
printf 'PULL_REQUEST_NUMBER=%s\n' "${PULL_REQUEST_NUMBER:-<missing>}"
printf 'PULL_REQUEST_TITLE=%s\n' "${PULL_REQUEST_TITLE:-<missing>}"
printf 'PULL_REQUEST_HEAD_SHA=%s\n' "${PULL_REQUEST_HEAD_SHA:-<missing>}"
printf 'PULL_REQUEST_BASE_SHA=%s\n' "${PULL_REQUEST_BASE_SHA:-<missing>}"
if [ -n "${PULL_REQUEST_NUMBER:-}" ]; then
printf '\nChangelog fragment matching PR number:\n'
rg -n "^$PULL_REQUEST_NUMBER[-_]|$PULL_REQUEST_NUMBER" changelog.d Cargo.toml CLAUDE.md 2>/dev/null || true
fi
printf '\nWorkspace package section:\n'
sed -n '288,298p' Cargo.toml | cat -n
printf '\nCLAUDE metadata section:\n'
sed -n '8,18p' CLAUDE.md | cat -nRepository: PerryTS/perry
Length of output: 2241
Keep release metadata maintainer-owned.
The PR already adds the required PR-keyed changelog fragment. Revert these version edits unless this is an explicitly maintainer-owned release update.
Cargo.toml#L295: restore the workspace package version.CLAUDE.md#L11: restore the documented current version.
External contributors must not edit release metadata fields; maintainers apply version metadata during merge or release when a changelog.d/<PR>-<slug>.md fragment is present.
📍 Affects 2 files
Cargo.toml#L295-L295(this comment)CLAUDE.md#L11-L11
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Cargo.toml` at line 295, Revert the release metadata edits: restore the
workspace package version in Cargo.toml at lines 295-295 and restore the
documented current version in CLAUDE.md at lines 11-11. Leave the PR-keyed
changelog fragment unchanged; these version fields are maintainer-owned.
Sources: Coding guidelines, Learnings
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Line 11: Revert the version metadata in CLAUDE.md from 0.5.1270 to 0.5.1269,
leaving the user-facing change in changelog.d/7165-scrub-absolute-home-paths.md
unchanged and avoiding any updates to frozen CHANGELOG.md.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c354c7f2-2270-4abd-a30e-f0e9b1d1d274
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
CHANGELOG.mdCLAUDE.mdCargo.tomlbenchmarks/compare.shbenchmarks/gc_ratchet/baseline/gc-ratchet-v1.jsonbenchmarks/gc_ratchet/gc_ratchet.pybenchmarks/honest_bench/harness/run_bench.shbenchmarks/honest_bench/results/results.jsonbenchmarks/public_baseline.pybenchmarks/results/public-node-bun-v1.jsonchangelog.d/7165-scrub-absolute-home-paths.mddocs/ecs-perf-case-study.mddocs/history/OPTIMIZATION_PLAN.mddocs/history/UI_ADJSUTMENTS.md
🚧 Files skipped from review as they are similar to previous changes (13)
- docs/history/OPTIMIZATION_PLAN.md
- benchmarks/results/public-node-bun-v1.json
- docs/ecs-perf-case-study.md
- benchmarks/public_baseline.py
- benchmarks/gc_ratchet/gc_ratchet.py
- benchmarks/honest_bench/harness/run_bench.sh
- docs/history/UI_ADJSUTMENTS.md
- Cargo.toml
- benchmarks/honest_bench/results/results.json
- benchmarks/compare.sh
- CHANGELOG.md
- benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json
- changelog.d/7165-scrub-absolute-home-paths.md
| Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. | ||
|
|
||
| **Current Version:** 0.5.1269 | ||
| **Current Version:** 0.5.1270 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Revert the contributor-owned version bump.
If this is an external-contributor PR, restore **Current Version:** 0.5.1269. The file's own “External contributor PRs” section says contributors must not change this line; the maintainer updates it at merge time. Keep the user-facing change in changelog.d/7165-scrub-absolute-home-paths.md.
Based on learnings, contributors must not update release/version metadata. As per coding guidelines, keep detailed change history in changelog.d/ and do not append to frozen CHANGELOG.md.
Proposed fix
-**Current Version:** 0.5.1270
+**Current Version:** 0.5.1269📝 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.
| **Current Version:** 0.5.1270 | |
| **Current Version:** 0.5.1269 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` at line 11, Revert the version metadata in CLAUDE.md from 0.5.1270
to 0.5.1269, leaving the user-facing change in
changelog.d/7165-scrub-absolute-home-paths.md unchanged and avoiding any updates
to frozen CHANGELOG.md.
Sources: Coding guidelines, Learnings
…nesses re-emitting them
Four tracked artifacts and three docs recorded absolute filesystem paths
containing the account names of three different developers. This is a public
repository, so those paths were published.
Scrubbed the committed data:
- benchmarks/honest_bench/results/results.json (180 sites) -> repo-relative
- benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json (5) -> repo-relative
- benchmarks/results/public-node-bun-v1.json (1) -> ~-relative
- docs/ecs-perf-case-study.md, docs/history/*, CHANGELOG.md -> ~-relative
or reworded
Every consumer of the scrubbed fields was checked first. The gc-ratchet
baseline's toolchain paths are pure provenance that neither `gc_ratchet.py
validate` nor the regression comparison reads, and the honest_bench `binary`
field has no reader at all.
Fixed the generators so this cannot recur. `portable_path` records a path
inside the checkout relative to the repository root, a path under the
operator's home directory with a `~` prefix, and anything else unchanged. It
is applied in gc_ratchet.py, honest_bench/harness/run_bench.sh and compare.sh,
and again in public_baseline.py at assemble time -- the single funnel every
benchmark component passes through on its way to the committed artifact.
public_baseline.py required `resolved_executable` and `measured_command[0]` to
be absolute. The intent is to reject an unresolved bare word such as `bun`,
not to require an absolute path, so that is now `_is_resolved_path`: absolute
or containing a path separator. A bare `bun` is still rejected.
ff7a46d to
99c0b50
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@changelog.d/7165-scrub-absolute-home-paths.md`:
- Line 3: Revise the changelog statement beginning “Every harness that
serializes a path” to exclude the polyglot harness, which can still emit
resolved executable paths directly. Clarify that public_baseline.py remains the
sanitization funnel for the committed public artifact, without claiming all
harnesses sanitize paths before serialization.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 77c7708b-d59f-4e70-9b74-d29d80f40e7c
📒 Files selected for processing (12)
CHANGELOG.mdbenchmarks/compare.shbenchmarks/gc_ratchet/baseline/gc-ratchet-v1.jsonbenchmarks/gc_ratchet/gc_ratchet.pybenchmarks/honest_bench/harness/run_bench.shbenchmarks/honest_bench/results/results.jsonbenchmarks/public_baseline.pybenchmarks/results/public-node-bun-v1.jsonchangelog.d/7165-scrub-absolute-home-paths.mddocs/ecs-perf-case-study.mddocs/history/OPTIMIZATION_PLAN.mddocs/history/UI_ADJSUTMENTS.md
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/history/UI_ADJSUTMENTS.md
- benchmarks/honest_bench/harness/run_bench.sh
- benchmarks/honest_bench/results/results.json
- benchmarks/gc_ratchet/gc_ratchet.py
- docs/history/OPTIMIZATION_PLAN.md
- CHANGELOG.md
- docs/ecs-perf-case-study.md
- benchmarks/compare.sh
- benchmarks/results/public-node-bun-v1.json
- benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json
- benchmarks/public_baseline.py
| @@ -0,0 +1,5 @@ | |||
| Removed absolute home-directory paths belonging to three different developers from the tracked benchmark artifacts and docs, and fixed the three harnesses that kept writing them back in. `benchmarks/honest_bench/results/results.json` recorded the full filesystem path of every workload binary it timed (180 of them), `benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json` recorded the compiler and static-archive paths it fingerprinted, `benchmarks/results/public-node-bun-v1.json` recorded a home-installed Bun executable, and `docs/ecs-perf-case-study.md`, `docs/history/`, and `CHANGELOG.md` quoted build commands verbatim. This is a public repository, so each of those published the operator's account name. | |||
|
|
|||
| Every harness that serializes a path now runs it through a `portable_path` helper first: a path inside the checkout is recorded relative to the repository root, a path under the operator's home directory is recorded with a `~` prefix, and anything else (a system executable such as `/usr/bin/env`) is recorded unchanged. The helper is applied in `benchmarks/gc_ratchet/gc_ratchet.py` (binary fingerprints), `benchmarks/honest_bench/harness/run_bench.sh` (the measured binary and any absolute argument in its command line), and `benchmarks/compare.sh` (the recorded Perry compile command). `benchmarks/public_baseline.py` sanitizes again at assemble time, which is the single funnel every benchmark component passes through on its way to the committed public artifact, so a component harness that is missed in future still cannot leak. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files matching polyglot and benchmark paths:"
git ls-files | rg '(^|/)(benchmarks|changelog\.d)/|benchmarks/polyglot|benchmarks/public_baseline.py|gc_ratchet.py|run_bench.sh|compare.sh' || true
echo
echo "changelog fragment:"
if [ -f changelog.d/7165-scrub-absolute-home-paths.md ]; then
cat -n changelog.d/7165-scrub-absolute-home-paths.md
else
echo "missing changelog.d/7165-scrub-absolute-home-paths.md"
fi
echo
echo "Search portable_path:"
rg -n "portable_path|resolve\(\)|shutil\.which|pubic_baseline|public_baseline|polyglot/run_all.sh" -S . || trueRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "benchmarks/polyglot/run_all.sh relevant sections:"
if [ -f benchmarks/polyglot/run_all.sh ]; then
wc -l benchmarks/polyglot/run_all.sh
sed -n '1,260p' benchmarks/polyglot/run_all.sh | cat -n
else
echo "missing benchmarks/polyglot/run_all.sh"
fi
echo
echo "benchmarks/public_baseline.py sanitize/assemble path:"
if [ -f benchmarks/public_baseline.py ]; then
wc -l benchmarks/public_baseline.py
rg -n "sanitize|serial|portable_path|resolved_executable|measured_command" benchmarks/public_baseline.py || true
sed -n '1,260p' benchmarks/public_baseline.py | cat -n
fi
echo
echo "focused portable_path occurrences:"
rg -n "\bportable_path\b|\bfrozenset\(|public_baseline" benchmarks -S || true
echo
echo "focused executable path writes in polyglot:"
python3 - <<'PY'
import pathlib, re
p = pathlib.Path("benchmarks/polyglot/run_all.sh")
if p.exists():
s = p.read_text()
for name in ["resolve(", "shutil.which(", "exec"]:
print(f"\n{name} references:")
for i,line in enumerate(s.splitlines(),1):
if name in line:
print(f"{i}: {line.strip()}")
PYRepository: PerryTS/perry
Length of output: 30367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "benchmarks/polyglot/run_all.sh produce/assemble section:"
sed -n '350,460p' benchmarks/polyglot/run_all.sh | cat -n
echo
echo "public_baseline.py assemble output handling:"
sed -n '280,355p' benchmarks/public_baseline.py | cat -nRepository: PerryTS/perry
Length of output: 9307
Update the changelog wording for the public baseline funnel.
The polyglot harness writes resolved executable paths directly, and the current changelog says this harness still leaks those values. Either narrow “every harness that serializes a path” or fix benchmarks/polyglot/run_all.sh to avoid publishing str(Path(...).resolve()) / shutil.which(...) without public_baseline.py sanitization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@changelog.d/7165-scrub-absolute-home-paths.md` at line 3, Revise the
changelog statement beginning “Every harness that serializes a path” to exclude
the polyglot harness, which can still emit resolved executable paths directly.
Clarify that public_baseline.py remains the sanitization funnel for the
committed public artifact, without claiming all harnesses sanitize paths before
serialization.
Brings in 39 upstream commits (a3b31c0..7d1dc9c) on top of the local context-mode experimental patches (b72f6f9, c3d1141, 6d28ca7). Resolution notes: * The local CHANGELOG.md diverged only in home-directory paths (`/Users/amlug/...` vs `~/projects/...`) that upstream scrubbed in PerryTS#7165. CHANGELOG.md is FROZEN at v0.5.1264 (see `changelog.d/README.md`), so the file is restored to its state at the merge base (df7214b, the v0.5.1264 version bump). The new context-mode entries go in `changelog.d/7220-context-mode-patches.md` instead, per the per-PR fragment policy. * The five perry-related file changes (eval_classifier, expr_new, alias_tracking, freshness, lib) all auto-merged cleanly. The workspace reorg from `perry/...` to `crates/perry/...` is picked up via git's rename detection, so the `freshness.rs` patch rides along at its new path. * No upstream-side changes were dropped or hand-edited.
2 participants
What is wrong
Four tracked benchmark artifacts and three documentation files record absolute
filesystem paths that start with a home directory. Because a macOS home
directory is
/Users/<account-name>, every one of those paths publishes theaccount name of the developer who produced the file. Three different people are
affected, and this repository is public.
The largest offender is the honest-bench results file, which stores the full
path of every workload binary it timed — 180 of them in one file. The rest are
one to five sites each.
This is not only a leftover. One of the harnesses still produces these paths on
every run, so the artifacts would have regrown them the next time anybody
refreshed a baseline.
What this changes
The committed data is scrubbed. Paths that point inside the repository are
now recorded relative to the repository root, which is both shorter and more
useful than the original —
benchmarks/honest_bench/workloads/.../image_convtells a reader exactly which binary was measured, whereas the absolute path told
them that plus somebody's account name. One path pointed at a tool installed
under a home directory rather than inside the repository; that one is recorded
with a
~prefix. The prose in the docs was reworded.The harnesses can no longer emit an absolute home path. Each one now passes
paths through a small
portable_pathhelper before serializing them. The ruleis the same everywhere: inside the checkout, record it relative to the
repository root; under the operator's home directory, record it with a
~prefix; anything else — a system executable such as
/usr/bin/env— recordunchanged.
The helper is applied in three generators, and then once more in
benchmarks/public_baseline.pyat assemble time. That last one matters: assembleis the single funnel every benchmark component passes through on its way to the
committed public artifact, so a component harness that gets added or missed
later still cannot leak through it.
I checked what reads these fields before changing them
The instruction here was not to silently drop a field something depends on, so
each field was traced to its readers first.
Consumer trace, field by field
gc-ratchet baseline (
toolchain.binaries.*.path,runtime_dir, and thesuite's
compile_command). Nothing reads them.validate_artifactchecks theschema version, kind, commit, platform, probe set, metric distributions,
correctness status and minor-cycle count;
evaluatecompares probe metrics.Neither function touches the
toolchainblock, and thesuiteblock is storedas an opaque pass-through. These fields are pure provenance.
Proof the gate still parses the scrubbed file:
Those are the two commands the
gc-ratchetworkflow runs in its validate step.honest-bench results (
binary). No reader anywhere.report.py,summary.pyandplot.pynever look at it, and neither doespublic_baseline.py. The committed snapshot predates the harness'scommandfield and does not contain one.
Public artifact (
runtimes.bun.resolved_executable). This is the top-levelruntimes block, which is written by
assembleand never read back. Theper-component
runtime_metadatablocks are validated, but those contained nohome paths. Rendering is unaffected —
suite_results()andreadme_block()produce byte-identical output before and after the scrub, verified by
substituting a sentinel value and diffing.
One validation rule had to move
public_baseline.pyrequired every recordedresolved_executableand everymeasured_command[0]to be an absolute path. That directly blocks recordinga repo-relative path.
Reading the check in context, its purpose is to catch a harness that recorded an
unresolved bare word such as
buninstead of resolving it to a concreteexecutable. Requiring absoluteness was a proxy for that, not the goal. The rule
is now written as
_is_resolved_path: a value passes if it is absolute orcontains a path separator. A bare
bunis still rejected, which is the case thecheck existed for.
tests/test_public_baseline.pypasses unchanged (7 tests), and every componentplus the suite in the committed artifact still validates against the new rule.
Verification
What was run
The requested sweep now returns only intentional placeholders:
/home/ubuntuis the guest mount point inside a Multipass Ubuntu VM, which iscorrect as written.
/Users/meis the documentation placeholder used throughoutthe
perry.tomlreference; the.pocatalogues are machine-generatedtranslations of that same page. Everything else in the repository that still
matches is a synthetic test fixture (
/Users/test,/home/user,/home/u), aCI runner path, or an elision that was already anonymous.
The generators were exercised live rather than just read:
run_bench.shbenchmarks/honest_bench/workloads/_probe_binrun_bench.sh$HOME~/.perry-pathscrub-proberun_bench.sh/bin/echo/bin/echorun_bench.sh--flag--flag(non-path arguments untouched)compare.shtarget/release/perrygc_ratchet.pywt-ratchet-target/release/perryBoth modified shell scripts pass
bash -n, and the extracted Python heredocscompile. All three scrubbed JSON files still parse.
Gates:
gc_ratchet.py validatepasses,test_gc_ratchet.py34/34 passes,test_public_baseline.py7/7 passes.check_file_size.shand the addr-classratchet only inspect
*.rsfiles and this branch changes none, so they areunaffected.
One gate is red on this branch, and it was already red on
mainPlease read this before judging the checks.
benchmarks/ci_public_baseline_check.pyfails here with:
It fails identically on a clean, unmodified checkout of
main. Both recordedfingerprints — source and harness — have already drifted from what the committed
artifact records, and clearing that means re-running the full Node/Bun/Perry
suite on a quiet host, which is not something a data scrub can or should do.
Two details that matter for diagnosing it. The fingerprint deliberately
normalizes the workspace version line in
Cargo.toml, so routine version bumpsare not what broke it — something in the benchmark sources or the harness set
genuinely changed. And the
lintjob never reaches this step today anyway,because it exits earlier at "Audit workspace architecture", which is also red on
main.This red predates the change; nothing here caused it and nothing here can clear
it.
One judgement call to flag
Pushing to
PerryTS/perryreturned 403, so this arrives as a fork PR under theexternal-contributor rule in
CLAUDE.md. Accordingly it does not touchCargo.toml,Cargo.lock, or theCLAUDE.mdversion line — the maintainerapplies the bump at merge. It does add the PR-keyed
changelog.d/fragment,which contributors are expected to write.
The one hunk worth a second look is the
CHANGELOG.mdedit. That file is afrozen archive and the PR template says not to edit it, so I want to be explicit
about what this is: it adds nothing and removes nothing. It replaces a
home-directory prefix in six historical entries that quote build commands
verbatim. Leaving them would have meant leaving the same leak in place in one of
the most-read files in the repository, which seemed like the wrong trade against
a freeze rule aimed at stopping the file from growing. It is one hunk away
from being dropped if you disagree.
Suggested follow-up: a CI guard
The repository has no hook or CI check that would catch this, and no
.claude/settings.jsonat all. A guard would be a small script in the shape ofscripts/check_file_size.sh, grepping tracked files for/Users/<name>and/home/<name>and failing on anything outside a short allowlist — the allowlistis genuinely needed, since
/Users/test,/Users/me,/home/user,/home/u,/home/ubuntuand the CI runner paths are all legitimate. Adding it here wouldhave meant shipping a new gate alongside a data scrub, so it is proposed rather
than included. Happy to open it as a separate PR.
Summary by CodeRabbit
Documentation
Benchmarking