fix(test): isolate benchmark harness daemon runtimes - #2198
Conversation
scripts/benchmark-index.sh and scripts/benchmark-search-graph.sh ran the product with no runtime or cache of their own: the benchmark repository was indexed into the operator's live store and every one-shot joined the operator's account daemon. Source scripts/test-runtime.sh in both, start the private daemon before timing, record setup-time.txt and total-time.txt beside index-time.txt, and index from a repository path instead of querying a project in the live store. Add tests/test_benchmark_runtime_isolation_contract.sh, which fails before this change, and wire it into scripts/test.sh. Part of DeusData#1696. Signed-off-by: Anton Standrik <astandrik@yandex-team.ru>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
This PR updates the benchmark harness scripts to run against a harness-owned daemon runtime and cache, preventing benchmark runs from contaminating or depending on the operator’s live store/daemon state, and adds a contract test to enforce that isolation.
Changes:
- Add a benchmark runtime isolation contract test that verifies both harnesses don’t leak caller
CBM_*dirs into product processes and thatbenchmark-index.shwrites new timing artifacts. - Update
benchmark-index.shto initialize/cleanup a private runtime, start a private daemon before timing, and emitsetup-time.txtandtotal-time.txt. - Update
benchmark-search-graph.shto accept<repo-path>, index into a private cache (untimed), and run queries against a warm private daemon.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/test_benchmark_runtime_isolation_contract.sh | Adds an isolation contract test + timing file presence checks for benchmark harnesses. |
| scripts/test.sh | Wires the new contract test into the shell test runner. |
| scripts/benchmark-search-graph.sh | Switches to repo-path input and makes indexing/queries use a harness-owned runtime/daemon. |
| scripts/benchmark-index.sh | Runs index benchmark under harness-owned runtime/cache, starts private daemon before timing, and writes setup/total timing files. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| echo "private daemon did not start" >&2 | ||
| exit 1 | ||
| fi | ||
| INDEX_JSON=$("$BINARY" cli index_repository "{\"repo_path\":\"$REPO\",\"mode\":\"full\"}" 2>/dev/null || echo '{}') |
| # alone. setup-time.txt keeps the activation cost attributable and | ||
| # total-time.txt is their sum — the figure comparable with earlier runs, which | ||
| # paid activation inside the index timing whenever no daemon was already warm. | ||
| SETUP_START_MS=$(python3 -c "import time; print(int(time.time()*1000))") |
| fi | ||
|
|
||
| # Index via CLI and capture timing | ||
| START_MS=$(python3 -c "import time; print(int(time.time()*1000))") |
| INDEX_JSON=$("$BINARY" cli index_repository "{\"repo_path\":\"$REPO\",\"mode\":\"full\"}" 2>/dev/null || echo '{}') | ||
| PROJECT=$(echo "$INDEX_JSON" | python3 -c " | ||
| import json, sys | ||
| d = json.load(sys.stdin) | ||
| if 'content' in d: | ||
| d = json.loads(d['content'][0]['text']) | ||
| print(d.get('project', '')) | ||
| " 2>/dev/null || echo "") | ||
| if [ -z "$PROJECT" ]; then | ||
| echo "index of $REPO did not report a project" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
What does this PR do?
Part of #1696 (audit ledger), follow-up to #1691/#1695.
scripts/benchmark-index.shandscripts/benchmark-search-graph.shran the productwith no runtime or cache of their own: the benchmark repository was indexed into the
operator's live store, every one-shot joined the operator's account daemon, and the
timings depended on whatever that daemon was doing.
scripts/test-runtime.sh, callcbm_test_runtime_initandrun
cbm_test_runtime_cleanup "$BINARY"from the EXIT trap.benchmark-index.shstarts the private daemon before timing, soindex-time.txtmeasures the index alone, and recordssetup-time.txt(daemonactivation) and
total-time.txt(their sum — the figure comparable with earlierruns, which paid activation inside the index timing whenever no daemon was warm).
benchmark-search-graph.shnow takes<repo-path>instead of<project-name>:it indexes into the private cache (untimed) and times the queries against a daemon
it keeps warm, so no timing includes activation. No performance claim is made.
tests/test_benchmark_runtime_isolation_contract.sh: environment-probe fixturefor both scripts plus the presence of the three timing files. Fails on
mainwith
FAIL: benchmark-index exposed the caller CBM_RUNTIME_DIR to a product process; passes with this change. Wired as Step 0i2 inscripts/test.sh.To decide:
docs/EVALUATION_PLAN.md§7 runsbenchmark-index.shand then answersgraph questions from the main MCP session, i.e. it assumes the index stays in the
live store (
rm -f ~/.cache/codebase-memory-mcp/*.dbbetween languages). With aharness-owned cache that assumption no longer holds; either the graph phase runs
against the harness root, or the harness needs an explicit keep-root mode. Not
changed here.
Checklist
git commit -s)make -f Makefile.cbm test) — shell-only change; the newcontract plus
test_venue_parity_contractandtest_runtime_isolation_contractwere run; the full C suite was not run on this host
make -f Makefile.cbm lint-ci) — no C changes