Conversation
|
Claude finished @lewtun's task in 3m 12s —— View job PR Review: Expose OpenAI models in the UI
No blocking issues — 2 P1 Verdict: ready to merge What I checked
P1 — Double session-model fetch on mount
The second The fix is to decouple the session-fetch from const modelOptionsRef = useRef(modelOptions);
useEffect(() => { modelOptionsRef.current = modelOptions; }, [modelOptions]);
useEffect(() => {
if (!sessionId) return;
let cancelled = false;
apiFetch(`/api/session/${sessionId}`)
.then((res) => (res.ok ? res.json() : null))
.then((data) => {
if (cancelled) return;
if (data?.model) {
const model = findModelByPath(data.model, modelOptionsRef.current);
if (model) setSelectedModelId(model.id);
}
})
.catch(() => { /* ignore */ });
return () => { cancelled = true; };
}, [sessionId]); // modelOptions removedP1 —
|
|
Claude finished @lewtun's task in 2m 17s —— View job PR Review: Expose OpenAI models in the UI
LGTM What I checked
|
|
Claude finished @lewtun's task in 2m 51s —— View job PR Review: Expose OpenAI models in the UI
1 P0 What I checked
P0 —
|
|
Claude finished @lewtun's task in 2m 1s —— View job PR Review: Expose OpenAI models in the UI
LGTM What I checked
lewtun's inline question —
|
| "premium_used_today": used, | ||
| "premium_daily_cap": cap, | ||
| "premium_remaining": remaining, | ||
| # Backward-compatible aliases for the existing frontend and clients. |
There was a problem hiding this comment.
We don't need to be backwards compatible
|
Claude finished @lewtun's task in 1m 52s —— View job PR Review: Expose OpenAI models in the UI
LGTM What I checked
lewtun's inline note —
|
|
Claude finished @lewtun's task in 1m 25s —— View job PR Review: Expose OpenAI models in the UI
LGTM What I checked
|
|
|
||
| def test_premium_model_predicate_includes_bedrock_claude_and_gpt55(): | ||
| assert agent._is_premium_model("bedrock/us.anthropic.claude-opus-4-6-v1") | ||
| assert agent._is_premium_model("anthropic/claude-opus-4-6") |
There was a problem hiding this comment.
This is just for local dev and does not need to be tested
| }, | ||
| ] | ||
| BEDROCK_CLAUDE_MODEL_ID = "bedrock/us.anthropic.claude-opus-4-6-v1" | ||
| PREMIUM_GATED_MODEL_IDS = {"openai/gpt-5.5"} |
There was a problem hiding this comment.
Wouldn't it be cleaner to drop BEDROCK_CLAUDE_MODEL_ID in favour of having all models in PREMIUM_GATED_MODEL_IDS? Then the claude model picker would only be overidden if the developer sets ML_INTERN_CLAUDE_MODEL_ID
|
Claude finished @lewtun's task in 1m 38s —— View job PR Review: Expose OpenAI models in the UI
LGTM What I checked
lewtun's inline comments
The
The suggestion would replace the |
|
Claude finished @lewtun's task in 2m 20s —— View job PR Review: Expose OpenAI models in the UI
LGTM What I checked
|
|
Claude finished @lewtun's task in 56s —— View job PR Review: Expose OpenAI models in the UI
LGTM What I checked
|
There was a problem hiding this comment.
Added this to streamline agentic usage
| } | ||
|
|
||
|
|
||
| def _claude_picker_model_id() -> str: |
There was a problem hiding this comment.
@akseljoonas @cmpatino this is handy to bypass the Bedrock hard-coded endpoint for local dev
|
Non-blocking nit: This does not affect runtime behavior, but the PR description lists |
Summary
Deployment Notes
Validation
Closes #192