MemberJunction / MemberJunction/MJ
Normalize ChatResult.finish_reason across LLM drivers
- 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
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