Repository navigation
Attempt five: add the failure-discoverable convention, and void the run - #202
Merged
Merged
Conversation
Added what the fourth run's finding called for: a convention that cannot be inferred from any interface. PlaceHold refuses on a stale session and the refusal never names refresh_session, so the step is discoverable only by being refused. The run's numbers are void, and the arithmetic is what exposes it. The required chain is now five calls, yet the control arm averaged 4.0 calls at 100% completion. That is impossible for a five-call chain: the only way to finish in four is to skip the refresh, and the only way to skip it is for the session to already be refreshed. Cause: ProceduralBenefitProgram constructs one ProceduralBenchmarkTask and shares it across all six attempts, so _sessionRefreshed set by the first procedural attempt persisted into every later attempt and into the control arm. The control did not discover the convention, it inherited it. This is my own error and it is the same shape as the others in this sequence. The task's own test constructs a fresh instance per call and therefore passed, while the program that runs the benchmark shared one. I verified the property I was thinking about and not the object lifetime beside it. The fix is named: construct the task inside the agent factory, once per attempt, so the environment starts stale every time. Then the control arm must discover the refresh on every attempt while the procedural arm pays once -- which is the difference the harness exists to measure, and the first design in five that could produce one. Attempt five's figures are recorded as VOID rather than as another null result. The four before it are genuine nulls; this one is a bug, and conflating them would misrepresent both. 453 LongMemEval tests green (+2 guards on the new convention). Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01PgDgctPpbTziBNE2RT8VdE
There was a problem hiding this comment.
Pull request overview
This PR evolves the LongMemEval procedural benchmark by introducing a new failure-discoverable environment convention (a booking session must be refreshed before placing a hold) and documenting why the fifth run’s recorded numbers are invalid due to state leakage between benchmark arms.
Changes:
- Added a
RefreshSessiontool and a “stale session” refusal guard toPlaceHold. - Updated/expanded unit tests to include the refresh step and to assert the stale-session refusal doesn’t name the remedy.
- Appended a new documentation section explaining why attempt five’s metrics are void and what fix is required in the harness.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/AgentMemory.LongMemEval/ProceduralBenchmarkTask.cs | Adds session-refresh state, a new tool, and a stale-session refusal guard for PlaceHold. |
| tests/AgentMemory.Tests.Unit.LongMemEval/ProceduralBenchmarkTaskTests.cs | Updates the happy-path chain and adds/adjusts tests for the new stale-session convention. |
| docs/reviews/procedural-benefit-run-prerequisite.md | Documents the fifth run as invalid due to state leakage and records the needed fix direction. |
Suppressed comments (1)
tools/AgentMemory.LongMemEval/ProceduralBenchmarkTask.cs:99
CreateTools()is documented as returning tools "in the order a correct procedure uses them", but the list currently placesRefreshSessionafterCheckServiceBulletinand afterPlaceHold. With the new stale-session guard, the correct chain needsRefreshSessionbeforePlaceHold, so the current ordering contradicts the method contract and can mislead both the benchmark and its tests.
/// <summary>The tools, in the order a correct procedure uses them.</summary>
internal IReadOnlyList<AITool> CreateTools() =>
[
AIFunctionFactory.Create(LookUpTraveller),
AIFunctionFactory.Create(PlaceHold),
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
220
to
+225
| public void ToolsAreExposedInProcedureOrder() | ||
| { | ||
| // Four real tools plus twelve plausible decoys. The decoys are the third run's finding made | ||
| // concrete: with a small tool set an agent calls everything and skips discovery, so a stored | ||
| // procedure saves nothing and both arms tie. | ||
| new ProceduralBenchmarkTask().CreateTools().Should().HaveCount(16); | ||
| new ProceduralBenchmarkTask().CreateTools().Should().HaveCount(17); |
Comment on lines
+66
to
+73
| /// This one is not inferable from anything. Nothing in <c>refresh_session</c>'s name or | ||
| /// description connects it to holds; the requirement exists only in the environment's behaviour | ||
| /// and is discoverable only by being refused. That is what a human writes a runbook for, and a | ||
| /// runbook is what a procedure is. | ||
| /// </para> | ||
| /// </remarks> | ||
| private bool _sessionRefreshed; | ||
|
|
This was referenced Aug 27, 2026
Open
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.
Added what the fourth run's finding called for: a convention that cannot be inferred from any interface.
PlaceHoldrefuses on a stale session, and the refusal never namesrefresh_session.The run's numbers are void, and the arithmetic exposes it. The required chain is now five calls — refresh, look up, hold, bulletin, book — yet the control arm averaged 4.0 calls at 100% completion. Impossible for a five-call chain: the only way to finish in four is to skip the refresh, and the only way to skip it is for the session to already be refreshed.
Cause:
ProceduralBenefitProgramconstructs oneProceduralBenchmarkTaskand shares it across all six attempts._sessionRefreshedset by the first procedural attempt persisted into every later attempt and into the control arm. The control didn't discover the convention — it inherited it.This is my own error, of the same shape as the rest of this sequence. The task's own
TheEnvironmentAnswersIdenticallyEveryTimetest constructs a fresh instance per call and therefore passed, while the program that runs the benchmark shared one. I verified the property I was thinking about and not the object lifetime beside it.Fix, named: construct the task inside the agent factory, once per attempt, so the environment starts stale every time. Then the control arm must discover the refresh on every attempt while the procedural arm pays once — the difference the harness exists to measure, and the first design in five that could produce one.
Attempt five's figures are recorded as VOID, not as another null. The four before it are genuine nulls; this one is a bug, and conflating them would misrepresent both.
453 LongMemEval green (+2 guards on the new convention).
🤖 Generated with Claude Code
https://claude.ai/code/session_01PgDgctPpbTziBNE2RT8VdE