Skip to content

feat(notes): require an explicit llm.model; enum-validate provider - #564

Merged
goosewobbler merged 2 commits into
mainfrom
feat/llm-model-defaults
Jul 19, 2026
Merged

goosewobbler merged 2 commits into
mainfrom
feat/llm-model-defaults

Conversation

@goosewobbler

@goosewobbler goosewobbler commented Jul 19, 2026 •

Copy link
Copy Markdown
Owner

Stacked on #563 (base branch feat/unify-ecosystem-enablement) — review/merge #563 first.

What (revised approach)

Instead of shipping curated per-provider default models (the original plan), require the consumer to name a model and fail loud if they don't. The evidence for the pivot: the openai default we would have shipped, gpt-4o-mini, is already deprecated/removed from OpenAI's API — proving any default releasekit ships will rot and become a per-release maintenance treadmill (and a silent failure mode for consumers relying on it).

  • llm.model is required — a missing model fails at config validation (Zod), not silently inside the notes soft-fail catch. releasekit ships no model IDs, so there is nothing to rot or maintain.
  • provider is a Zod enum (openai | openai-compatible | anthropic | ollama), not a free-form string — a typo errors at config load.
  • openai-compatible requires baseURL at the schema level.
  • createProvider enforces the required model for in-process configs too (the CLI --llm-* flags bypass schema parsing), with a clear message.
  • Dropped the curated-default machinery: LLM_DEFAULTS.models, resolveModelId, and the per-provider ?? default fallbacks.
  • Regenerated releasekit.schema.json + docs/configuration.md.

Why require, not default

LLM enhancement is already an explicit opt-in (needs an API key, or a running Ollama with a pulled model), so also naming the model is a marginal, honest addition — there is no true zero-config LLM path to protect. The consumer is best placed to pick a current model for their account/budget, and forcing an explicit, fail-loud choice is more robust than releasekit guessing with an ID that rots.

Tests

  • Schema: model required for every provider; unknown provider rejected (enum); openai-compatible requires baseURL.
  • createProvider: a missing model throws a clear llm.model is required error; openai-compatible requires baseURL; valid configs construct the right provider.

Closes #541

Greptile Summary

This PR makes llm.model an explicit requirement and validates provider as a Zod enum, replacing the earlier approach of shipping curated per-provider default model IDs that were already rotting (e.g., gpt-4o-mini deprecated). Both the config-file path (Zod at load time) and the CLI --llm-* flag path (new LLMConfigSchema.parse block in command.ts) now fail loudly with non-zero exit codes rather than silently degrading to non-LLM notes.

  • Schema hardening: LLMConfigSchema becomes LLMConfigBaseSchema.superRefine(...) — model gets .min(1) (empty string rejected), provider becomes an enum, and openai-compatible requires baseURL at config-validation time.
  • CLI path closed: command.ts now runs LLMConfigSchema.parse(llm) on the CLI-assembled object before entering the pipeline; Zod errors are surfaced as a plain Error caught by handleError (non-zero exit), not swallowed by the soft-fail catch.
  • Defaults removed: LLM_DEFAULTS.models, resolveModelId, and all per-provider ?? default fallbacks are dropped; provider constructors now require model: string (non-optional).

Confidence Score: 5/5

Safe to merge — all three validation gaps flagged in the previous review round are now closed.

The CLI path now runs through LLMConfigSchema.parse before entering the pipeline, so a bad provider or missing model exits non-zero instead of silently degrading. The Zod schema adds .min(1) so an empty-string model is caught at config load. The provider field is an enum so typos are rejected at validation time. The JSON schema required array is correct. New tests cover all the new constraints. No residual gaps identified.

No files require special attention.

Important Files Changed

Filename Overview
packages/config/src/schema.ts LLMConfigSchema split into base + superRefine; model gets .min(1), provider becomes z.enum, openai-compatible baseURL enforced at validation time. Logic is correct.
packages/notes/src/command.ts CLI-assembled LLM config now parsed through LLMConfigSchema before runPipeline; bad provider or missing model throws a plain Error caught by handleError (non-zero exit). Closes the silent-degradation gap from the previous review.
packages/notes/src/llm/index.ts createProvider retains a runtime model guard as a safety net for programmatic callers that bypass schema parsing; the guard is now redundant for the two primary paths (config file + CLI) but is a reasonable defense layer.
packages/notes/src/llm/defaults.ts Dropped the models sub-object; LLM_DEFAULTS now only contains retry defaults. Clean removal.
packages/config/test/unit/schema.spec.ts New tests cover: missing model for all four providers, empty-string model rejection, unknown provider (enum), and openai-compatible requiring baseURL. Coverage matches the new schema constraints.
packages/notes/test/unit/llm/createProvider.spec.ts New test file covering missing model, missing baseURL for openai-compatible, and successful construction for openai and openai-compatible. OpenAI client is correctly mocked to avoid network setup.
releasekit.schema.json Regenerated correctly: provider gains enum, model gains minLength:1, both are in the required array. Conditional baseURL requirement for openai-compatible is not expressible in the generated JSON Schema but is enforced at Zod runtime.
packages/notes/src/core/types.ts LLMConfig.provider narrowed from string to the explicit union type, matching the Zod enum.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User provides LLM config] --> B{Source?}
    B -->|Config file| C[loadConfig - Zod parses full config]
    B -->|CLI --llm flags| D[Assemble llm object in command.ts]
    C --> E{LLMConfigSchema valid?}
    D --> F[LLMConfigSchema.parse llm]
    F --> G{Valid?}
    E -->|No| H[ZodError - hard exit non-zero]
    E -->|Yes| I[config.releaseNotes.llm set]
    G -->|No| J[Error caught by handleError - hard exit non-zero]
    G -->|Yes| I
    I --> K[runPipeline - createProvider]
    K --> L{model truthy?}
    L -->|No| M[throw LLMError - soft-catch warn]
    L -->|Yes| N[Construct provider]
    N --> O[LLM enhancement runs]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User provides LLM config] --> B{Source?}
    B -->|Config file| C[loadConfig - Zod parses full config]
    B -->|CLI --llm flags| D[Assemble llm object in command.ts]
    C --> E{LLMConfigSchema valid?}
    D --> F[LLMConfigSchema.parse llm]
    F --> G{Valid?}
    E -->|No| H[ZodError - hard exit non-zero]
    E -->|Yes| I[config.releaseNotes.llm set]
    G -->|No| J[Error caught by handleError - hard exit non-zero]
    G -->|Yes| I
    I --> K[runPipeline - createProvider]
    K --> L{model truthy?}
    L -->|No| M[throw LLMError - soft-catch warn]
    L -->|Yes| N[Construct provider]
    N --> O[LLM enhancement runs]
Loading

Reviews (6): Last reviewed commit: "feat(notes): require an explicit llm.mod..." | Re-trigger Greptile

@goosewobbler
goosewobbler force-pushed the feat/unify-ecosystem-enablement branch from 9ec230f to 930a4ab Compare July 19, 2026 15:38
@goosewobbler
goosewobbler force-pushed the feat/llm-model-defaults branch from de2c837 to 779356f Compare July 19, 2026 15:38
@goosewobbler
goosewobbler force-pushed the feat/unify-ecosystem-enablement branch from 930a4ab to dfb88c5 Compare July 19, 2026 16:08
@goosewobbler
goosewobbler force-pushed the feat/llm-model-defaults branch from 779356f to 0daf56d Compare July 19, 2026 16:17
@goosewobbler goosewobbler changed the title feat(notes): optional llm.model with curated per-provider defaults; enum-validate provider feat(notes): require an explicit llm.model; enum-validate provider Jul 19, 2026
Comment thread packages/notes/src/llm/index.ts
@goosewobbler
goosewobbler force-pushed the feat/unify-ecosystem-enablement branch from dfb88c5 to 61f1373 Compare July 19, 2026 16:34
@goosewobbler
goosewobbler force-pushed the feat/llm-model-defaults branch from 0daf56d to 2cbb186 Compare July 19, 2026 16:43
@goosewobbler
goosewobbler force-pushed the feat/unify-ecosystem-enablement branch from 61f1373 to 037e4db Compare July 19, 2026 16:45
@goosewobbler
goosewobbler force-pushed the feat/llm-model-defaults branch from 2cbb186 to 8a7837f Compare July 19, 2026 16:45
@goosewobbler

Copy link
Copy Markdown
Owner Author

Both P1s (the CLI validation gap) addressed in the latest push:

The --llm-* flags assemble an LLM config in command.ts that bypasses loadConfig's Zod pass. It's now validated through LLMConfigSchema.parse before use, so an invalid --llm-provider or a missing --llm-model throws — caught by the top-level handleError, which exits non-zero — instead of reaching createProvider and being swallowed by the pipeline's soft-fail as a silent degradation. LLMConfigSchema is now exported from @releasekit/config for this.

Comment thread packages/config/src/schema.ts
@goosewobbler
goosewobbler force-pushed the feat/unify-ecosystem-enablement branch from 037e4db to 65e6237 Compare July 19, 2026 16:53
@goosewobbler
goosewobbler force-pushed the feat/llm-model-defaults branch from 8a7837f to 920e7bc Compare July 19, 2026 16:53
@goosewobbler
goosewobbler force-pushed the feat/unify-ecosystem-enablement branch from 65e6237 to 046a069 Compare July 19, 2026 17:10
@goosewobbler
goosewobbler force-pushed the feat/llm-model-defaults branch from 920e7bc to 7e27af4 Compare July 19, 2026 17:11
@goosewobbler

Copy link
Copy Markdown
Owner Author

Addressed the follow-up P1 (empty-string model): added .min(1) to model so "model": "" is rejected at config validation (and by the CLI's LLMConfigSchema.parse) rather than passing z.string() and being soft-caught later. Regenerated the schema (now carries minLength: 1) and added a test that { provider: 'openai', model: '' } throws.

…nfig opts out

Every detected ecosystem is versioned and, where publishing is configured and
authenticated, published by default; explicit config is only ever an opt-out.

- publish.cargo.enabled / publish.pub.enabled now default true (was false)
- add version.npm.enabled (default true) so package.json handling is opt-out-able
  like Cargo.toml/pubspec.yaml; wired into every version-stage package.json write
- keep Cargo.toml (root and hybrid packages) versioned when npm handling is
  disabled — the npm gate must not skip or abandon cargo updates
- reframe the enabled describe() text as opt-outs; regenerate schema + config docs

Default-on is safe: an enabled registry that finds zero targets no-ops, and
publishing without credentials fails/skips at the auth check.

Refs #554

BREAKING CHANGE: crates.io and pub.dev publishing now default on for detected
packages. Set publish.cargo.enabled / publish.pub.enabled to false to opt out
(version and tag only).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011i1KvoyxATU3bz5V5Sf1Xu
@goosewobbler
goosewobbler force-pushed the feat/unify-ecosystem-enablement branch from 046a069 to 56166dd Compare July 19, 2026 17:35
Resolve model-ID rot by requiring the consumer to name a current model rather than
shipping curated defaults that rot every release — the openai default we would have
shipped, gpt-4o-mini, is already deprecated. LLM enhancement is already an explicit
opt-in (it needs an API key, or a running Ollama with a pulled model), so naming the
model is a marginal, honest addition, and it removes a maintenance treadmill.

- llm.model is required; a missing model fails at config validation, not silently
  inside the notes soft-fail catch.
- provider is a Zod enum (openai | openai-compatible | anthropic | ollama), not a
  free-form string.
- openai-compatible requires baseURL at the schema level.
- the CLI --llm-* flags assemble a config that bypasses loadConfig's Zod pass, so
  validate it through LLMConfigSchema before use — a bad provider or missing model
  fails loud (non-zero exit) instead of degrading silently in the pipeline.
- drop the curated-default machinery (LLM_DEFAULTS.models, resolveModelId, the
  per-provider `?? default` fallbacks).
- docs: use a `<your-model>` placeholder in config examples (a real id would drift).
- regenerate schema + config docs.

Refs #541

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011i1KvoyxATU3bz5V5Sf1Xu
@goosewobbler
goosewobbler force-pushed the feat/llm-model-defaults branch from 7e27af4 to 25bc9e9 Compare July 19, 2026 17:36
@goosewobbler

Copy link
Copy Markdown
Owner Author

Addressed the error-message P2: the CLI validation catch now surfaces the actual Zod issue messages (.issues[].message) instead of a static string. A missing --llm-base-url for openai-compatible now reports "baseURL is required when provider is 'openai-compatible'", an empty model reports the min(1) message, and a bad provider reports the enum error — each naming the real failing field.

Base automatically changed from feat/unify-ecosystem-enablement to main July 19, 2026 17:47
@goosewobbler
goosewobbler merged commit 5373744 into main Jul 19, 2026
5 checks passed
@goosewobbler
goosewobbler deleted the feat/llm-model-defaults branch July 19, 2026 17:49
This was referenced Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LLM notes: require an explicit llm.model (no curated defaults); enum-validate provider

1 participant