feat(agentry): add conflict-rebase git code-mode eval scenario#526
feat(agentry): add conflict-rebase git code-mode eval scenario#5260xpatrickbot wants to merge 12 commits into
Conversation
c783959 to
5cc4e36
Compare
3eb74e3 to
ec66e12
Compare
0xpatrickdev
left a comment
There was a problem hiding this comment.
I didn't think this needed its own PR, but it's fine to leave the current split.
-
The second eval exposes some gaps in our file/folder structure. With everything in a single files (outcome.js,scenario.js, _eval fixture etc, files will grow to large and be harder to read and maintain. Wdyt about a folder per eval, each with outcome, scenario, etc, that seems better.
-
Let's have a designer spend some time on scenario design. My direction was "something with rebasing or worktrees". Let me be more concrete:
- these evals are specifically about the
@endo/agentryharness: system prompt, tools, exos available in code mode - the git evals are mainly to ensure we've implemented enough api functionality to be useful - it's be great if we can source gaps
- in the future we might have evals for testing that agents understand E, petNames, and other runtime specifics, but that's out of scope for this PR
- a few scenarios with deeper breadth are more valuable than a laundry list of the basics (rebase with no conflicts in the PR's current iteration).
- Note: we have tool gaps for richer scenarios (git remotes, npm registry, testing and linting code), but that's ok. The goal of this work is to expose so we can prioritize
5cc4e36 to
8f352a0
Compare
ec66e12 to
f73cb62
Compare
8f352a0 to
131bbe4
Compare
b5f5560 to
fc402ba
Compare
fc402ba to
2ee2e92
Compare
0xpatrickbot
left a comment
There was a problem hiding this comment.
Must fix before un-draft
-
[must-fix-loop]
packages/agentry/src/eval/scenarios/rebase/outcome.js:72resolvesexpected.ontoBranchafter the agent has already run, so the target base is mutable by the model. A bad run can move or recreatemain, then putfeatureon top of that new tip with the expected summaries and file contents;onto-is-ancestorwill still pass even though the original onto tip was not preserved. Capture the pre-run onto tip OID in the fixture and make the scorer assert that exact commit is still the onto branch tip and the ancestor/base used for the replay. [rule: roles/jurors/assessor/AGENT.md] -
[must-fix-loop]
packages/agentry/src/eval/scenarios/rebase/outcome.js:136only checks a whitelist of expected paths at the final feature tip. A run can add an extra tracked file, or otherwise change unchecked tree state, while still matching the selected file contents and keeping the worktree clean. Since this is an outcome scorer, it needs an exact tree or diff invariant for the expected post-rebase state, not only positive assertions for a few paths. [rule: roles/jurors/prover/AGENT.md]
Should fix in this PR
- [summary-fix]
packages/agentry/src/eval/scenarios/rebase/outcome.js:100checks replay identity through summaries plus fresh OIDs, but it does not prove the replayed commits carry the original patches. A run could resetfeaturetomain, create a fresh empty commit with the first expected summary, then commit the finalfeature.txtcontent with the second expected summary; the current summary, OID, final file, clean-tree, and branch checks can all pass without preserving the original per-commit patch sequence. Either compare each replayed commit's patch or tree against the original feature commit sequence, or narrow the documented scenario contract to final-tree-plus-subject matching. [rule: roles/jurors/wire-watcher/AGENT.md]
|
Addressed the 2026-06-30 panel findings in 744c0ec:
There were no inline review threads exposed for the 0xpatrickbot review, so I am replying on the PR with the per-finding update. |
744c0ec to
e28e909
Compare
e28e909 to
e5fd609
Compare
|
Reworked this draft to address the 2026-06-25 review by replacing the clean-rebase-only row with the deeper conflict-rebase scenario. The clean replay case is still covered inside the scenario by the second feature commit. |
Refs: #525 ## Description Revises the git code-mode eval design to a three-scenario set: `stage-and-commit` as the smoke floor, `conflict-rebase` as the buildable rebase-conflict row, and `stack-surgery` as the centerpiece whose fixture and scorer can land behind a pending live row. The main review point is whether this is the right granularity for the matrixed eval suite. `conflict-rebase` absorbs the clean-rebase signal from PR #526, `stack-surgery` absorbs the multi-file signal from PR #626, and the stack-surgery live row now depends on the five verbs proposed in PR #611 rather than a generic future stack-edit API. The latest revision also adds an `Anticipated Needs: Bounded Reads` section. It records that the recovered session's heavy `sed -n` use was context-bounded reading, not stream editing, and proposes shared bounded-read options for fs and git string-returning reads as companion API work rather than an eval blocker. ### Security Considerations No runtime authority changes. The design keeps scoring on final cap-visible repository state and authority boundaries, keeps shell access out of the code-mode pass path, and defers remote push and provider triage policy to separate eval lanes. ### Scaling Considerations The design reduces scenario count because PR #627 multiplies scenarios across conditions, models, and repeats. It also calls out bounded reads as a scaling pressure point: without shared line/character-bounded read affordances, confined agents have to compensate with full-file or full-diff slurps as runs multiply. ### Documentation Considerations This is documentation-only. It updates the design contract and the design index to describe the trimmed set, the `agentry-git-verb-gaps` dependency, the pending-row plan for stack-surgery, and the remaining bounded-read companion API question. ### Testing Considerations Verified markdown formatting with `npx prettier --check designs/agentry-git-eval-scenarios.md designs/README.md` and checked the committed diff with `git diff --check HEAD^ HEAD`. No runtime tests were run because this PR changes design documentation only. ### Compatibility Considerations No shipped API or runtime behavior changes. The design intentionally stops treating clean rebase and multi-file edit-and-commit as separate eval rows; their signals survive inside `conflict-rebase` and `stack-surgery` respectively. Bounded-read options are described as future compatible companion surface work, not as behavior required by this design. ### Upgrade Considerations No production upgrade concerns. Known cross-PR friction: PR #611 edits the same `designs/README.md` region, including the Totals line, M3 graph/table, and estimates; whichever lands second should regenerate those hunks.
0xpatrickbot
left a comment
There was a problem hiding this comment.
Garden panel verdict
Panel: code-panel. Round: 1.
Findings
- [must-fix-loop] eval-export-docs | Document the three new conflict-rebase eval exports and their roles. [rule: roles/jurors/surfacer/AGENT.md § Operating norms]
- [summary-fix] conflict-scenario-target | Copy only declared target fields into the scenario instead of hardening and exposing the provisioned repository object. [rule: roles/jurors/purist/AGENT.md § Side-channel closure and Family-consistency]
- [summary-fix] pipelined-file-lookup | Chain nested
lookupcalls withE(nodeP)so filesystem path traversal remains CapTP-pipelined. [rule: packages/platform/src/fs/extended/helpers.js] - [summary-fix] integration-tip-negative-test | Add a test that moves
integrationafter a correct rebase and verifiesintegration-branch-tipfails. [rule: skills/adversarial-tests/SKILL.md § Boundary]
|
PR #526 reviewer context
|
932c7ea to
eef5ec9
Compare
Live model eval resultsThe PR now contains the three split fixes:
There are six distinct completed model campaigns in the retained evidence, so all six are listed below, one table per scenario, ordered by model. conflict-rebase (3 of 6 passed)
stage-and-commit (6 of 6 passed)
Paid costs are calculated from the current OpenRouter catalog prices and recorded input, output, and cache-read usage. Free-model cost is shown as N/A. ² The Tencent conflict-rebase result is from a 2026-07-11 rerun. The run now passes under the standard budget, which this job raised to 5 minutes, completing all eleven outcome checks in 2m43.1s. The original 2-minute "timed out" row was a budget artifact, not a model failure: the event stream showed steady tool-call progress when killed, not hanging. An earlier Tencent run had also reached both passing outcomes but exited afterward because of reader-pump unhandled rejections (since fixed in this PR). Failure analysis
The post-fix NVIDIA reruns and both Gemini runs exited for ordinary outcome assertions, not unhandled promise or reader-pump crashes. The durable event artifacts retain sanitized API errors rather than full generated source. |
| /** | ||
| * The end-state a conflict-rebase scenario is scored against. | ||
| */ | ||
| export interface GitConflictRebaseTarget { |
There was a problem hiding this comment.
scenario-specific types should go in a types.ts file in that folder. Include an additional refactor commit to address existing stage-and-commit specific types.
Also - scenario specific code/types/consts should not be part of any public api surface in this package. Only eval building/testing machinery. Please adjust code added in this PR and add refactor commits for the existing scenario in this package.
Refs: #525
Description
Adds a conflict-rebase git code-mode eval scenario for
@endo/agentry/eval. The scenario provisions a feature branch with one commit that conflicts onapp.txtand one clean note commit, asks the agent to rebase ontointegration, and scores only the final repository outcome.The fixture precomputes the successful rebase by running the real conflict resolution once, capturing the expected feature tree and replayed commit patches, then resetting the repository so the eval starts on
feature/conflict-rebase. The scorer checks thatintegrationis still the same tip and is now an ancestor, the feature commits replay in their original order with fresh oids, the caller-supplied post-resolution patches match,app.txthas the requested combined wording, both notes are present, the worktree and index are clean, no rebase is left in progress, and HEAD is on the feature branch.This replaces the earlier clean-rebase-only iteration with a deeper row: the clean replay case remains covered by the second feature commit, while the first commit exercises a real conflict resolution. That keeps the eval set smaller while making the rebase scenario more representative.
Security Considerations
The scenario exercises existing writable git authority: starting and continuing a rebase, writing a resolved file through the workspace, and staging it before continuing. It introduces no new capability surface or credential flow. The live eval remains gated by the existing
ENDO_LLM_*/LAL_*environment contract.Scaling Considerations
None. The scenario creates a small temporary git repository with a few commits and files, then tears it down with the test.
Documentation Considerations
The eval README now lists the conflict-rebase scenario and updates the no-credentials test description to cover multiple eval rows. There is no downstream data or deployment impact.
Testing Considerations
The no-credentials AVA tests cover the faux-provider pass path and negative scorer states for never rebased, wrong conflict resolution text, dropped feature commit, replayed commits out of order, and a dirty/conflicted rebase left in progress. Regression-test note: temporarily forcing the conflict-rebase scorer to return
pass: truemade those five negative tests fail withValue is not false: true, then the temporary change was reverted and the scenario tests passed again.Compatibility Considerations
Additive. The existing stage-and-commit scenario and runner API continue to work; the shared scenario type is widened so eval rows can carry scenario-specific expected target shapes.
Upgrade Considerations
None.