docs: honest results framing, CODEOWNERS, and a stale eval fixture - #120
Open
theDakshJaitly wants to merge 3 commits into
Open
docs: honest results framing, CODEOWNERS, and a stale eval fixture#120theDakshJaitly wants to merge 3 commits into
theDakshJaitly wants to merge 3 commits into
Conversation
…ings The Results table led with 916.38x and 10.74x under a bare "Results" heading. Both numbers are real but neither measures what a reader assumes. The denominator in evaluate/lib/grep-baseline.mjs is ceil(chars/4) over the ENTIRE contents of grep's top-3 files, and no agent runs in that harness at all. It is a retrieval-payload ratio against a favourable baseline — not end-to-end token savings, which evaluate/README.md already says the harness cannot support. Independent measurement on a 1,600-file Rails codebase (#115) found mex spending 1.3-1.7x grep's total session tokens across three tasks, because extra round-trips re-send full context and outweigh a smaller payload per call. - Lead with the correctness result: 23/23 call sites found where a regex found 15/23, missing bare self-calls with no explicit receiver. That is the argument the data actually supports. - State the denominator inline in the table so the ratio cannot be misread. - Replace the soft closing caveat with the measured 1.3-1.7x finding and a pointer to the three-arm experiment evaluate/README.md specifies.
Contributors had no way to tell who reviews what and were asking in issue threads (#115). Starts minimal with a single default owner; the header notes the three rules that make CODEOWNERS silently no-op (last match wins, owners need write access, advisory unless branch protection requires it).
The 'budget' task expected BudgetedEmitter, which has not existed in src/ for some time — it appeared only in this fixture. The task was therefore unpassable and every NL retrieval measurement was understated against a target that could not be hit. The budget is enforced by BudgetLedger (src/graph/agent-protocol.ts, constructed in cli-agent.ts#beginResponse).
There was a problem hiding this comment.
Pull request overview
Updates project docs and evaluation fixtures to more accurately represent what the current benchmarks measure, clarifies review ownership, and fixes an unpassable NL eval expectation.
Changes:
- Reframe the top-level README “Results” section to lead with the demonstrated correctness win, clarify the retrieval-size denominators inline, and explicitly state the lack of end-to-end token-savings evidence.
- Add a minimal
.github/CODEOWNERSwith a default owner and a brief “how CODEOWNERS can be ignored” header note. - Fix
evaluate/fixtures/nl-tasks.jsonto expect the real budget-enforcement symbol (BudgetLedger) instead of a nonexistent one.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| README.md | Reframes benchmark claims and clarifies what the reported ratios and harness do/do not measure. |
| evaluate/fixtures/nl-tasks.json | Updates the budget task expected/rubric symbol to match the implementation (BudgetLedger). |
| .github/CODEOWNERS | Introduces a default code owner and documents key CODEOWNERS matching/eligibility caveats. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three separable commits, all following from the findings in #115.
1. README results framing
The Results table led with 916.38x and 10.74x under a bare "Results" heading. Both numbers are accurate for what they compute, but neither measures what a reader assumes.
The denominator in
evaluate/lib/grep-baseline.mjsisceil(chars/4)over the entire contents of grep's top-3 files, and no agent runs in that harness at all. It is a retrieval-payload ratio against a favourable baseline.evaluate/README.mdalready states plainly that the harness cannot support an end-to-end graph-versus-no-graph claim; the top-level README did not carry that across.Independent measurement on a 1,600-file Rails codebase found mex spending 1.3-1.7x grep's total session tokens across three real tasks. A smaller payload per call does not recover the cost of extra round-trips, each of which re-sends full context.
Changes:
\.method_nameregex found 15/23, the missing eight being bare self-calls with no explicit receiver. That is the claim the data supports, and it is the stronger argument.evaluate/README.mdspecifies.2. CODEOWNERS
Contributors had no way to tell who reviews what, and were asking directly in issue threads. Starts minimal with a single default owner. The header comment records the three rules that make CODEOWNERS silently do nothing: last match wins, owners need write access, and it is advisory unless branch protection requires it.
3. Stale eval fixture
evaluate/fixtures/nl-tasks.jsonexpectedBudgetedEmitterfor the budget task. That symbol does not exist anywhere insrc/and appears only in the fixture. The task was unpassable, so every natural-language retrieval measurement — including ones taken before this branch — was scored against a target that could not be hit.The budget is enforced by
BudgetLedger(src/graph/agent-protocol.ts, constructed incli-agent.ts#beginResponse). Correcting it raises measured baseline recall on the NL fixture set from 3/7 to 4/7 with no code change.Scope
Documentation, config, and one test fixture. No source changes.
Verification
npm run buildclean,npx vitest run369/369 passing.