Skip to content

fix(benchmarks): split BENCHMARK_HUB_DATA_DIR on the platform path delimiter - #471

Open
Tyagiquamar wants to merge 1 commit into
understudylabs:mainfrom
Tyagiquamar:fix/hub-data-dir-platform-delimiter
Open

fix(benchmarks): split BENCHMARK_HUB_DATA_DIR on the platform path delimiter#471
Tyagiquamar wants to merge 1 commit into
understudylabs:mainfrom
Tyagiquamar:fix/hub-data-dir-platform-delimiter

Conversation

@Tyagiquamar

Copy link
Copy Markdown

Problem

BENCHMARK_HUB_DATA_DIR is documented as a multi-root list, but every consumer parsed it by splitting on ":" — the POSIX path-list separator:

  • src/benchmark-hub-core.ts (slugRoots)
  • src/benchmarks-mcp.ts (configureBenchmarksMcpRoots, hubPrimaryRoot)

On Windows there is no way to spell a colon-separated list of absolute directories: an absolute path like C:\Users\me\.understudy\benchmarks splits into "C" and "\Users\me\.understudy\benchmarks". The first fragment becomes the data root (resolving to a nonexistent <cwd>\C directory) and the real directory lands under the data2 prefix. Every slug lookup then fails with unknown benchmark slug: data--<name> (use list_benchmarks) — for the MCP server, the CLI, and the hub loaders alike.

I found this by running the test suite on Windows: ~31 tests across tests/benchmarks-mcp.test.mjs, tests/experiment-lineage.test.mjs, and tests/benchmark-extension.test.mjs failed with exactly this signature; the fixture sets BENCHMARK_HUB_DATA_DIR to a single absolute temp dir, which is enough to trigger the mis-split.

Fix

Parse and join the env var with path.delimiter (: on POSIX, ; on Windows). Behavior on POSIX is byte-for-byte unchanged; on Windows, single absolute paths now parse correctly and the documented multi-root list works with the platform separator. Also updated the contract comment/README wording and one test assertion that hardcoded the : join.

Testing

On Windows 11, Node 24 (before → after):

node --test tests/benchmarks-mcp.test.mjs tests/experiment-lineage.test.mjs tests/benchmark-extension.test.mjs

before: exit 1 — 31 failures ("unknown benchmark slug: data--promoted", "fixture benchmark discovered through the shared loader", ...)
after:  exit 0 — tests 58, pass 58, fail 0, skipped 0

npm run typecheck
exit 0

@Tyagiquamar

Copy link
Copy Markdown
Author

Hi @lluisinthedesert, gentle review ping when convenient. #471, #472, and #473 are three small platform-compatibility fixes that are ready for review. Happy to make any changes you'd like.

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