Four tool-calling run-fns accumulate on `finish`, and the cactus one is now pinned by a test

[sroussey]

Defect

.claude/CLAUDE.md states the rule: "Never accumulate output. A provider stream function yields text-delta / object-delta events and a finish carrying {} as Output. StreamingAiTask / TaskRunner does the accumulating. Do not 'helpfully' put accumulated data on the finish event."

Four run-fns emit text-deltas and then hand the accumulated text back on finish, so the accumulator receives it twice. Verified at 9a80450:

site	payload on finish
providers/huggingface-transformers/src/ai/common/HFT_ToolCalling.ts:546	{ text: cleanedText, toolCalls: validToolCalls }
providers/node-llama-cpp/src/ai/common/LlamaCpp_ToolCalling.ts:453 and :592	data: output (accumulated)
providers/llamacpp-server/src/ai/common/LlamaCppServer_ToolCalling.ts:152	{ text: accumulatedText, toolCalls: finalToolCalls }
providers/cactus/src/ai/common/Cactus_ToolCallingCore.ts:88	{ text: raw, toolCalls: validToolCalls }

This has been carried in the review series for six cycles. Two things make it worth a tracked issue now rather than another recommendation line.

1. The cactus site was rewritten this window and copied forward verbatim

1e00e1c / 6b22438 reduced Cactus_ToolCalling.ts to a two-line delegation and moved the body into the new Cactus_ToolCallingCore.ts. The function was retyped, re-documented, given a new engine-resolver parameter and eight new tests — and the finish payload was carried through unchanged. A full rewrite of the exact function is the moment the convention should have been applied.

2. It is now pinned by a passing assertion

packages/test/src/test/ai-provider-cactus/Cactus_ToolCalling.test.ts:131-136:

const { names, finishText } = await runToolCalling({ … });
expect(names).toEqual(["lookup_weather"]);
expect(finishText).toContain("lookup_weather");   // :136

finishText is read from ev.data.text on the finish event (:77). Fixing the convention now requires deleting a green test, which is the state where a documented rule stops being one.

Proposed fix

1. Change all five emit points to emit({ type: "finish", data: {} as ToolCallingTaskOutput, usage }), keeping the object-delta on toolCalls that already precedes them. Update Cactus_ToolCalling.test.ts:136 to assert on the joined textDeltas instead of finishText — it already collects them (:65-67).
2. Add the generic guard. There are now three working conformance assertions in packages/test/src/contract/ai-provider/assertions/ to sit beside (inferAdvertisesRegistered, usageNormalization, capabilityHonesty). The shape: over a recorded event list, assert that no finish payload field repeats content a text-delta or object-delta already emitted. Without it, the next provider added by copying one of these four inherits the bug.

Why it matters

The accumulator has to decide which copy wins. StreamProcessor already does the accumulating, so a finish that repeats it is at best redundant and at worst authoritative over the deltas the consumer actually streamed — and the four providers carrying it are the four local ones, where the delta stream is the only progress signal a user sees.

Found during the 2026-08-31 review (6th cycle). Snapshot: workglow-dev/prd → analysis/grades/2026-08-31/libs-providers.md.

[sroussey]

Still open — 7th cycle. Re-verified at fb861bb; all four sites byte-identical, same line numbers.

$ grep -n 'type: "finish"' \
    providers/llamacpp-server/src/ai/common/LlamaCppServer_ToolCalling.ts \
    providers/huggingface-transformers/src/ai/common/HFT_ToolCalling.ts \
    providers/node-llama-cpp/src/ai/common/LlamaCpp_ToolCalling.ts \
    providers/cactus/src/ai/common/Cactus_ToolCallingCore.ts
providers/llamacpp-server/.../LlamaCppServer_ToolCalling.ts:152:        type: "finish",
providers/huggingface-transformers/.../HFT_ToolCalling.ts:546:      type: "finish",
providers/node-llama-cpp/.../LlamaCpp_ToolCalling.ts:453:          emit({ type: "finish", data: output });
providers/node-llama-cpp/.../LlamaCpp_ToolCalling.ts:592:        emit({ type: "finish", data: output });
providers/cactus/.../Cactus_ToolCallingCore.ts:88:    emit({ type: "finish", data: { text: raw, toolCalls: validToolCalls } });

LlamaCppServer_ToolCalling.ts:152 is the same line this review has cited since 2026-07-27.

What changed this window is that the argument for a generic assertion got stronger, not weaker. 8d6ea69 shipped assertInferServesInferred — a second conformance direction added to the AI-provider contract suite, wired into all fifteen providers, with justified exception sets and a failure message naming the offending fixture. That is the third working assertion to sit beside, and it is proof the pattern this issue asks for is one the repo can and does build.

The concrete ask is unchanged and now has a template: a recorded-event assertion inside runAiProviderConformance — no finish payload may repeat text a text-delta already emitted — plus deleting the assertion at packages/test/src/test/ai-provider-cactus/Cactus_ToolCalling.test.ts:136 that currently pins the violation.

Verified during the 2026-09-07 review of providers/.

---

Generated by Claude Code