feat: add provider factory#8
Closed
gnanam1990 wants to merge 1 commit into
Closed
Conversation
Resolve provider type from explicit config, model registry metadata, or OpenAI-compatible endpoint defaults. Route headless execution through the factory, add provider type validation to config, and cover OpenAI-compatible creation plus reserved Anthropic/Gemini provider errors with focused tests.
Collaborator
Author
|
Closing this stacked PR so the team can restart from the modified WorkSplit PRD. |
This was referenced Jun 2, 2026
gnanam1990
added a commit
that referenced
this pull request
Jun 27, 2026
Addresses the correctness findings from the PR review. self-report (#1): drop behavior-describing phrases ("fall back to", "placeholder value", bare "best guess", "as a fallback", "without proper") that also match legitimate final answers; keep first-person/uncertainty admissions only, since these are matched without a context guard. self-report (#5): scan every occurrence of each inability stem so an early success-negation ("could not find any examples") no longer masks a later genuine admission with the same stem ("could not implement it"). continuation cue (#6): require a trailing colon AND an action lead-in on the final clause; stop flagging recommendations, plain summary colons, and sign-offs. Still catches the mid-line "...Let me check the config:". gate order (#8): check the self-report admission BEFORE pending-plan, so an admitted-impossible task downgrades immediately with the accurate reason instead of burning continue-nudges. pending plan (#3): treat a pending/in_progress update_plan item as a NUDGE-only weak signal -- it no longer forces INCOMPLETE on its own (a completed run that left stale plan bookkeeping is trusted). Only a continuation cue or a self-report admission finalizes INCOMPLETE. max-turns (#4): a run cut off at the MaxTurns ceiling now finalizes INCOMPLETE under the gate instead of being reported as success. exec json/cron (#2, #7): for -o json, emit the terminal done with exit 4 on an incomplete run (final() pre-emits a success done:0 for json that would otherwise mask it); emit an error event -- not just a warning -- so the cron failure extractor can recover the reason. Deferred (noted on the PR): acceptance-only-when-mutated (#9, cost) and reusing tools.normalizePlanStatus (#10, would widen scope to internal/tools). Tests: add TestContinuationCueMatching and TestMaxTurnsCutoffIsIncompleteUnderGate, extend TestSelfReportedIncompletionMatching with the #1/#5 cases, and replace the in_progress=>incomplete test with TestPendingPlanAloneDoesNotForceIncomplete. make build / go vet / make lint / go test ./... -race all green.
gnanam1990
added a commit
that referenced
this pull request
Jun 27, 2026
… on no-tool-call / self-reported-incomplete turns) (#325) * fix(agent): don't end a run as success on a no-tool-call turn mid-task A turn that produced text but no tool call was always accepted as the final answer, so the loop reported success even when the model stopped mid-task (e.g. ended on "...Let me check the SSH configuration:" with plan steps still pending). Add an opt-in completion gate (Options.RequireCompletionSignal): when a turn has no tool call and work clearly remains -- pending update_plan items, or the message ends on a continuation cue -- re-prompt the model to continue instead of finalizing. Bounded by maxContinueNudges and still by MaxTurns/the deadline; once the budget is spent the run finalizes as INCOMPLETE (Result.Incomplete) rather than success. Default off, so the interactive path is byte-identical. Genuine single-turn completions (no pending plan, no cue) still finalize as success. Covered by internal/agent/completion_gate_test.go. * feat(exec): report stalled headless runs as INCOMPLETE (exit 4) Enable the agent completion gate for headless exec (RequireCompletionSignal) and map Result.Incomplete to run_end status "incomplete" with a new exit code 4, so a run that stalled mid-task (model stopped without a tool call while work remained, continue budget exhausted) is no longer reported as success. Interactive callers are unaffected. * fix(agent): downgrade self-reported non-completion; advisory task-grounded acceptance Reduce -- not eliminate -- false-success on headless runs. Two DETERMINISTIC, unit-tested gates (with the plan gate from the prior commit): (a) self-report downgrade: if the final message admits the model guessed or could not meet the objective, finalize INCOMPLETE (exit 4), never success. Inability is matched by first-person STEMS generalized over verb/tense ("I cannot/can't/could not/am unable to/do not have/unable to ...") plus guess/fallback/uncertainty phrases, with a guard so success-y negations ("could not find any issues", "cannot reproduce") are not misread. (b7bc0b8's plan gate already forces INCOMPLETE on pending/in_progress update_plan items at termination.) (b) task-grounded acceptance is ADVISORY, not a guarantee. When --self-correct is on it demands one bounded acceptance pass that re-derives the task's stated criterion and runs a concrete check, discouraging three false-success patterns (well-formed==correct, existing-tests-pass==objective-met, result==baseline-it-was-told-to-beat). But it is a prompt: a model that ignores it and confidently claims "PASS, all requirements met" still slips, because ZERO has no general oracle to verify correctness against a task's hidden criterion. Empirically (TB-2, qwen3-coder:480b) this reliably catches admissions and incomplete plans and REDUCES false-success, but a confident false PASS on a model-ceiling task is a residual, fundamental gap -- not a tuning miss. Default off (RequireCompletionSignal); interactive callers are byte-identical. Covered by internal/agent/{acceptance_gate_test.go,completion_gate_test.go}. * feat(exec): surface the INCOMPLETE reason in run_end and logs When a headless run finalizes as INCOMPLETE, include Result.IncompleteReason in the session error event and a stderr warning so an honestly-incomplete run (e.g. "the final message admits the objective was not met") is debuggable rather than an opaque exit 4. * fix(agent): harden completion gate per PR review Addresses the correctness findings from the PR review. self-report (#1): drop behavior-describing phrases ("fall back to", "placeholder value", bare "best guess", "as a fallback", "without proper") that also match legitimate final answers; keep first-person/uncertainty admissions only, since these are matched without a context guard. self-report (#5): scan every occurrence of each inability stem so an early success-negation ("could not find any examples") no longer masks a later genuine admission with the same stem ("could not implement it"). continuation cue (#6): require a trailing colon AND an action lead-in on the final clause; stop flagging recommendations, plain summary colons, and sign-offs. Still catches the mid-line "...Let me check the config:". gate order (#8): check the self-report admission BEFORE pending-plan, so an admitted-impossible task downgrades immediately with the accurate reason instead of burning continue-nudges. pending plan (#3): treat a pending/in_progress update_plan item as a NUDGE-only weak signal -- it no longer forces INCOMPLETE on its own (a completed run that left stale plan bookkeeping is trusted). Only a continuation cue or a self-report admission finalizes INCOMPLETE. max-turns (#4): a run cut off at the MaxTurns ceiling now finalizes INCOMPLETE under the gate instead of being reported as success. exec json/cron (#2, #7): for -o json, emit the terminal done with exit 4 on an incomplete run (final() pre-emits a success done:0 for json that would otherwise mask it); emit an error event -- not just a warning -- so the cron failure extractor can recover the reason. Deferred (noted on the PR): acceptance-only-when-mutated (#9, cost) and reusing tools.normalizePlanStatus (#10, would widen scope to internal/tools). Tests: add TestContinuationCueMatching and TestMaxTurnsCutoffIsIncompleteUnderGate, extend TestSelfReportedIncompletionMatching with the #1/#5 cases, and replace the in_progress=>incomplete test with TestPendingPlanAloneDoesNotForceIncomplete. make build / go vet / make lint / go test ./... -race all green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OpenAIProviderdirectly.Tests
npx --yes bun test ./tests --timeout 15000npx --yes tsc --noEmitnpx --yes bun run build./zero --helpgit diff --checkNotes
This PR is stacked on
feat/m1-model-registrywhile #7 is open because it depends on the model registry APIs. Once #7 merges, this branch can be rebased or retargeted tomain.Anthropic and Google provider types are intentionally reserved with clear errors until their concrete provider modules land in the next M1 slices.