diff --git a/docs/issue#0544.html b/docs/issue#0544.html new file mode 100644 index 0000000..742f30e --- /dev/null +++ b/docs/issue#0544.html @@ -0,0 +1,116 @@ + + + + + + Implementation Notes — Issue #0544 + + + +
+

Implementation Notes

+

Issue #544 — Banner 显示所选 OpenAI wire 变体 (US-008) — 2026-08-02

+ +

Design Decisions

+ +
+

Decision Presentation-only ProtocolLabel, separate from NormalizeProtocol

+

Ambiguity: The spec said the banner should show openai/chat or openai/resp_api, but the functional protocol string (fed to LiveConfig.Protocol and subagent provider resolution) must remain the raw user value.

+

Choice: Added a new pure function ProtocolLabel(raw string) string in protocol.go that reuses NormalizeProtocol internally, and called it only at the banner render site — the functional value stays untouched.

+

Rationale: Mutating the protocol string in Options would leak a display concern into functional resolution (subagent.go reads cfg.Protocol). Keeping label mapping presentation-only isolates the change to display and cannot regress wire selection.

+
+ +
+

Decision Bare openai surfaces as openai/chat

+

Ambiguity: The spec asked that input openai "still display meaningfully (normalized label documented)".

+

Choice: ProtocolLabel maps both openai and openai/chat to the label openai/chat, making the Chat Completions wire variant explicit rather than ambiguous.

+

Rationale: A user who typed the shorthand openai still sees exactly which wire protocol is in use, matching the actual /chat/completions endpoint pigo speaks.

+
+ +
+

Decision Empty input returns empty, banner falls back to em dash

+

Ambiguity: How to render the Protocol row when no protocol is set (named/inferred provider path).

+

Choice: ProtocolLabel("") returns ""; the banner's existing firstNonEmpty(..., "—") then shows the em dash, exactly as before.

+

Rationale: Preserves the prior no-protocol display; anthropic and provider-name rows do not regress.

+
+ +

Deviations

+

None — implementation followed the spec as written.

+ +

Tradeoffs

+ +
+

Tradeoff Unrecognized value returns verbatim rather than erroring

+

Alternatives: (a) return the trimmed raw string for unknown input; (b) propagate an error / show a placeholder.

+

Chosen: (a) — the label is presentation-only and must never fail. A genuine typo is already rejected upstream by NormalizeProtocol during resolution, long before the banner renders.

+

Why: The banner should never panic or block startup on a display concern; duplicating the error path here would be redundant.

+
+ +

Open Questions

+ +
+

Question Should the banner also reflect a heuristically-inferred protocol?

+

Assumption: When protocol is unset and the provider is resolved by model-id heuristics, the Protocol row shows the em dash (or provider name context is enough).

+

Verify: Whether users want the banner to display the effective wire protocol even when it was inferred rather than explicitly passed — out of scope for this issue but a possible follow-up.

+
+ +

Generated by goal-workflow /note-it

+
+ + diff --git a/internal/cli/tui/banner.go b/internal/cli/tui/banner.go index ec6639b..852555a 100644 --- a/internal/cli/tui/banner.go +++ b/internal/cli/tui/banner.go @@ -6,6 +6,7 @@ import ( "charm.land/lipgloss/v2" + "github.com/smallnest/pigo/internal/provider" "github.com/smallnest/pigo/internal/selfupdate" ) @@ -61,7 +62,7 @@ func renderBanner(theme Theme, opts Options, cwd string) string { {"Version", firstNonEmpty(opts.Version, "dev")}, {"Model", firstNonEmpty(opts.Model, "—")}, {"Provider", firstNonEmpty(opts.ProviderName, "—")}, - {"Protocol", firstNonEmpty(opts.Protocol, "—")}, + {"Protocol", firstNonEmpty(provider.ProtocolLabel(opts.Protocol), "—")}, {"Thinking", firstNonEmpty(string(opts.ThinkingLevel), "off")}, {"Directory", firstNonEmpty(cwd, "—")}, } diff --git a/internal/cli/tui/banner_test.go b/internal/cli/tui/banner_test.go index e2aeb9f..2734ac1 100644 --- a/internal/cli/tui/banner_test.go +++ b/internal/cli/tui/banner_test.go @@ -64,3 +64,27 @@ func TestRenderBannerUpToDate(t *testing.T) { t.Error("up-to-date build must not show an upgrade hint") } } + +// TestRenderBannerProtocolLabel verifies the Protocol row shows the concrete +// OpenAI wire variant: a bare "openai" is surfaced as "openai/chat" (explicit +// Chat Completions), "openai/resp_api" passes through, and an unset protocol +// falls back to the em dash rather than showing an empty row. +func TestRenderBannerProtocolLabel(t *testing.T) { + cases := []struct { + protocol string + want string + }{ + {"openai", "openai/chat"}, + {"openai/chat", "openai/chat"}, + {"openai/resp_api", "openai/resp_api"}, + {"anthropic", "anthropic"}, + {"", "—"}, + } + for _, c := range cases { + t.Setenv("PIGO_HOME", t.TempDir()) + out := renderBanner(DefaultTheme(), Options{Version: "dev", Protocol: c.protocol}, "/tmp/proj") + if !strings.Contains(out, c.want) { + t.Errorf("protocol %q: banner should show %q, got: %q", c.protocol, c.want, out) + } + } +} diff --git a/internal/provider/protocol.go b/internal/provider/protocol.go index fd16fb2..26c909c 100644 --- a/internal/provider/protocol.go +++ b/internal/provider/protocol.go @@ -46,3 +46,32 @@ func NormalizeProtocol(raw string) (string, error) { return "", fmt.Errorf("unknown --protocol %q (want openai|openai/chat|openai/resp_api|anthropic)", raw) } } + +// ProtocolLabel maps a raw --protocol value to the human-facing label shown in +// the startup banner's Protocol row, so the displayed wire format matches what +// pigo actually speaks. It differs from NormalizeProtocol in one deliberate way: +// the bare "openai" input is surfaced as "openai/chat", making the Chat +// Completions variant explicit rather than ambiguous. "openai/resp_api" and +// "anthropic" pass through as themselves. +// +// An empty input returns empty (the banner then falls back to "—" or the +// provider name, so an unset protocol on a named/inferred provider is not +// mislabeled). An unrecognized value returns the trimmed input verbatim — the +// label is presentation-only and must never fail; a real typo is already +// rejected upstream by NormalizeProtocol during resolution. +func ProtocolLabel(raw string) string { + canonical, err := NormalizeProtocol(raw) + if err != nil { + return strings.TrimSpace(raw) + } + switch canonical { + case ProtocolOpenAI: + return "openai/chat" + case ProtocolOpenAIResponses: + return ProtocolOpenAIResponses + case ProtocolAnthropic: + return ProtocolAnthropic + default: + return "" + } +}