Fix the shared task; find the procedural arm never had memory at all - #203
Merged
Conversation
Fixed the state leak from attempt five -- the task is now constructed inside the agent factory, once per attempt -- and the arithmetic is consistent again: six calls for both arms, the five-step chain plus one stale refusal. Both arms hit the refusal, discovered the refresh, and finished. Both arms paid the discovery cost on every attempt, and the reason is not the task. BuildAgent composes chatClient.AsAIAgent(...) with the benchmark tools and no AIContextProviders. There is no Neo4jMemoryContextProvider on either arm. So the procedural arm stored procedures -- promotion works and is unit-tested -- and had no way to read them back. It was a plain agent writing traces nobody consulted. This retracts the conclusion drawn after attempt four. That write-up said a competent model does not explore on this task class and so procedural memory has no exploration cost to remove. No run performed here supports it. Attempts one through four could not have shown a benefit under any task design, because the procedural arm had no procedural memory. The nulls were real observations of a harness in which recall was never wired -- a statement about this code, not about the feature or the task class. The doc comment on ProceduralBenefitProgram claims the arms differ in exactly two things, trace recall and promotion. Only promotion was implemented. I wrote that claim and did not check it. That is the sixth instance of the same failure in this sequence and the most consequential, because it silently turned five runs into measurements of nothing. What is needed is named: attach Neo4jMemoryContextProvider to the procedural arm with reasoning-trace recall enabled and MaxTraces > 0, leaving the control without it. Every figure in the review note up to this point is marked void. 453 LongMemEval tests green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PgDgctPpbTziBNE2RT8VdE
There was a problem hiding this comment.
Pull request overview
This PR updates the procedural-benefit LongMemEval harness to remove cross-attempt shared state in the benchmark task and records the resulting findings in the procedural-benefit run write-up.
Changes:
- Construct a fresh
ProceduralBenchmarkTaskper attempt (via the agent factory) to prevent stale-session state leaking across attempts/arms. - Retain a separate “template” task instance solely for stable prompt/completion predicates and logging.
- Extend the procedural-benefit run prerequisite document with a “sixth run” section explaining why prior conclusions were invalid under the current harness wiring.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/AgentMemory.LongMemEval/ProceduralBenefitProgram.cs | Fixes shared task-state leakage by creating a new task per attempt and adjusts prompt/logging to use a template task. |
| docs/reviews/procedural-benefit-run-prerequisite.md | Documents the sixth run outcome and identifies missing procedural-memory recall wiring as the root cause of earlier null results. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+69
to
73
| proceduralMemoryEnabled => | ||
| BuildAgent(chatClient, new ProceduralBenchmarkTask(), proceduralMemoryEnabled), | ||
| template.Prompt, | ||
| template.IsComplete, | ||
| traces); |
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.
Fixed attempt five's state leak — the task is now constructed inside the agent factory, once per attempt. The arithmetic is consistent again: 6 calls both arms = five-step chain + one stale refusal. Both arms hit the refusal, discovered the refresh, finished.
Both arms paid discovery on every attempt — and the reason is not the task.
BuildAgentcomposeschatClient.AsAIAgent(...)with the benchmark tools and noAIContextProviders. There is noNeo4jMemoryContextProvideron either arm. The procedural arm stored procedures — promotion works, it's unit-tested — and had no way to read them back. A plain agent writing traces nobody consulted.This retracts my conclusion from attempt four
That write-up said a competent model doesn't explore on this task class, so procedural memory has no exploration cost to remove. No run performed here supports that. Attempts 1–4 could not have shown a benefit under any task design, because the procedural arm had no procedural memory. The nulls were real observations of a harness in which recall was never wired — a statement about this code, not the feature.
The doc comment on
ProceduralBenefitProgramclaims the arms "differ in exactly two things": trace recall and promotion. Only promotion was implemented. I wrote that claim and didn't check it — the sixth instance of the same failure in this sequence, and the most consequential, because it silently turned five runs into measurements of nothing.What's needed
Attach
Neo4jMemoryContextProviderto the procedural arm withAutomaticRecallCategories.ReasoningTracesandMaxTraces > 0; leave the control without it. Every figure in the review note up to this point is marked void.🤖 Generated with Claude Code
https://claude.ai/code/session_01PgDgctPpbTziBNE2RT8VdE