MemberJunction / MemberJunction/MJ

Normalize ChatResult.finish_reason across LLM drivers

Open
#4,335 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

`ChatResultChoice.finish_reason` is typed as a bare `string` with no mapping layer anywhere in the repo, so each driver reports whatever its SDK hands back — or a hardcoded constant:

| Driver | Ordinary turn |
|---|---|
| OpenAI + the 7 OpenAI-shaped drivers | `'stop'` (passthrough) |
| Anthropic | `"completed"` — hardcoded; the real `stop_reason` is discarded into `modelSpecificResponseDetails` |
| Gemini | `"STOP"` — uppercase passthrough |
| Mistral | `choice.finishReason` (different field name) |
| LM Studio | hardcoded `'stop'`, with a comment noting it has no real reasons |

The providers genuinely differ, which is *why* a mapping is warranted — but nothing forced this; we just never wrote one.

Native tool calling (#4082) added the first normalized value — `'tool_calls'`, on tool-call turns only — and deliberately left every other value untouched, since normalizing the rest is a behavior change with a different risk profile. This issue is the rest of it.

## Constraint: do not use a closed union

[`packages/Actions/CoreActions/src/custom/ai/betty.action.ts:169`](https://github.com/MemberJunction/MJ/blob/next/packages/Actions/CoreActions/src/custom/ai/betty.action.ts#L169) branches on `finish_reason === 'references_json'` from a real `ChatResult`:

```ts
const result = await betty.ChatCompletion(chatParams);
const referencesJsonChoice = result.data.choices[2];
if (referencesJsonChoice.message.content && referencesJsonChoice.finish_reason === 'references_json') {
bettyReferences = JSON.parse(referencesJsonChoice.message.content);
}
```

BettyBot overloads `finish_reason` as a **channel selector** rather than a stop reason: `'references_json'` marks `choices[2]` as the structured-references payload. Mapping every driver onto a fixed union would break this — and break it *silently*, because the action falls through to text-parsing `choices[1]` instead of throwing, so the only symptom is structured references quietly becoming parsed text.

Normalization must therefore map the **known** stop reasons and **pass unrecognized values through untouched**, with that rule stated explicitly in the type's doc comment — otherwise someone tightens it to a union later and hits the same trap.

## Scope

- Map the known reasons (`stop` / `length` / `content_filter` / `tool_calls`) per driver; pass anything else through unchanged.
- Anthropic is the biggest gap: it discards `stop_reason` entirely on the non-streaming path and hardcodes `"completed"`.
- Extend `RunLLMConformanceSuite` to assert the normalized value per driver. Its current `expect(choice.finish_reason).toBe('stop')` passes only because the 8 providers wired into it are all OpenAI-shaped — Anthropic, OpenAI and Gemini are not in that suite, so nothing has ever compared them.
- Once the values agree, drop the `OrdinaryFinishReason` knob from `RunLLMToolCallingConformanceSuite`, which exists only to record today's divergence in one place.

## Priority

Low. Exactly one consumer, and it reads a value that normalization would preserve. Nothing in the agent framework, prompt runner, engine or UI reads the field at all, and the Phase 2 hybrid loop branches on the presence of `toolCalls` rather than on the stop reason — so this is not on the critical path for #4082.

Contributor guide

Open the contributing guide

Research direction

Start by locating ChatResultChoice.finish_reason, the driver implementations, and RunLLMConformanceSuite; inspect the Anthropic non-streaming path where stop_reason is currently discarded. Compare each driver's known stop reasons, preserve unrecognized values including references_json, and update the conformance assertions; done when the suites agree on normalized values without breaking the BettyBot branch in packages/Actions/CoreActions/src/custom/ai/betty.action.ts.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend-api-design, testing-qa
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
62/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.