Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions docs/reviews/procedural-benefit-run-prerequisite.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,52 @@ five that could actually produce one.

Until that lands, **attempt five's figures should be treated as void**, not as another null result.
The four before it are genuine nulls; this one is a bug.


---

## Sixth run — and it invalidates the earlier conclusion, not just attempt five

Fixed the shared-state bug: the task is now constructed inside the agent factory, once per attempt.
The arithmetic is consistent again — six calls for both arms, which is the five-step chain plus one
stale refusal. Both arms hit the refusal, discovered the refresh, and finished.

```
procedures completion=100% meanSteps=6.3 meanToolCalls=6.0
control completion=100% meanSteps=6.0 meanToolCalls=6.0
```

**Both arms paid the discovery cost on every attempt.** The procedural arm did not skip it — and the
reason is not the task.

### The agent never had a memory provider

`ProceduralBenefitProgram.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, it is unit-tested — and then had no
way whatsoever to **read** them back. It was a plain agent writing traces nobody consulted.

### This retracts the "task class" conclusion

The write-up after attempt four concluded:

> *"A competent model does not explore on this class of task at all… procedural memory has no
> exploration cost to remove."*

**That conclusion is not supported by any run performed here.** 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 system in which recall was never wired — which is a statement about this
harness, 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 the claim and did not check it, which
is the sixth instance of the same failure in this sequence and by far the most consequential, because
it silently converted five runs into measurements of nothing.

### What is actually needed

Attach `Neo4jMemoryContextProvider` to the procedural arm's agent, with
`AutomaticRecallCategories.ReasoningTraces` enabled and `MaxTraces > 0`, and leave the control arm
without it. Then — and only then — the arms differ in the feature. Every figure recorded in this
document up to that point should be treated as void.
16 changes: 11 additions & 5 deletions tools/AgentMemory.LongMemEval/ProceduralBenefitProgram.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,24 @@ internal static async Task<int> RunAsync(string[] args, CancellationToken cancel
log,
cancellationToken).ConfigureAwait(false);

var task = new ProceduralBenchmarkTask();
// Prompt and completion only -- both are pure, and this instance is never given to an agent.
var template = new ProceduralBenchmarkTask();
var traces = profile.Services.GetRequiredService<IReasoningTraceRepository>();

// The arm switch, and the only difference between the two agents. The control arm is handed
// no trace repository, so it neither reads nor writes procedures.
var runner = new MafAgentTaskRunner(
proceduralMemoryEnabled => BuildAgent(chatClient, task, proceduralMemoryEnabled),
task.Prompt,
task.IsComplete,
// A FRESH task per attempt. Sharing one leaked the stale-session flag from the procedural
// arm into the control, which completed a five-call chain in four calls -- a void run whose
// only tell was that the arithmetic was impossible. The environment must start stale every
// time or the control arm inherits the discovery instead of paying for it.
proceduralMemoryEnabled =>
BuildAgent(chatClient, new ProceduralBenchmarkTask(), proceduralMemoryEnabled),
template.Prompt,
template.IsComplete,
traces);
Comment on lines +69 to 73

log.WriteLine($"procedural-benefit: {attempts} attempts per arm, task='{task.Prompt}'");
log.WriteLine($"procedural-benefit: {attempts} attempts per arm, task='{template.Prompt}'");
var result = await ProceduralBenefitResult
.MeasureAsync(runner, "procedural-benchmark", attempts, cancellationToken)
.ConfigureAwait(false);
Expand Down