feat(catalog): support image-generation models (registry modality field)#3074
Conversation
Adds the `Modality` enum (`text` / `image` / `audio`) and three new
`ModelCatalogEntry` fields that mirror the librefang-registry schema
extension:
- `modality: Modality` — defaults to `Text` for back-compat
- `image_input_cost_per_m: Option<f64>`
- `image_output_cost_per_m: Option<f64>`
`context_window` and `max_output_tokens` now carry `#[serde(default)]`
so image-generation entries (e.g. OpenAI's `gpt-image-2`) that have no
conventional context window parse cleanly instead of failing the whole
provider file and silently dropping every sibling model.
The `/api/models`, `/api/models/{id}`, and `/api/providers/{id}`
responses now expose `modality` and the two image-cost fields so the
dashboard can render image-generation pricing correctly.
Unblocks librefang-registry#71 — without these changes a registry pull
containing a `modality = "image"` entry panics `toml::from_str::<ProviderCatalogFile>`
in `catalog_sync.rs` at daemon start, wiping the OpenAI provider from
the catalog.
github-actions
Bot
added
ready-for-review
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 522d7126f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[serde(default)] | ||
| pub context_window: u64, | ||
| /// Maximum output tokens. | ||
| /// Maximum output tokens. `0` or absent means "not applicable". | ||
| #[serde(default)] | ||
| pub max_output_tokens: u64, |
There was a problem hiding this comment.
Marking context_window and max_output_tokens with #[serde(default)] makes missing values deserialize to 0 for every model type, not just image/audio entries. That means a malformed text model now loads silently and later propagates a 0 context window into runtime execution (via model lookup in kernel paths), which can cause aggressive overflow handling instead of failing fast at catalog parse time. Please gate these defaults with modality-aware validation so Modality::Text still requires real limits.
Useful? React with 👍 / 👎.
| .and_then(|v| v.as_bool()) | ||
| .unwrap_or(false), | ||
| aliases: vec![], | ||
| ..Default::default() |
There was a problem hiding this comment.
The custom model creation handler now relies on ..Default::default(), but it never reads modality, image_input_cost_per_m, or image_output_cost_per_m from the request body. As a result, posting a custom image-generation model through /api/models/custom will always be stored as text modality with missing image pricing, and later /api/models//api/providers/{id} responses misrepresent the model’s capabilities and costs.
Useful? React with 👍 / 👎.
…m-model API Address review feedback on the modality schema change: - kernel: filter `m.context_window as usize` to >0 in the four lookup sites so image/audio entries (no context window) fall through to the caller's 200K default instead of feeding 0 into compaction thresholds and budget math. - providers route: `add_custom_model` now reads `modality` / `image_input_cost_per_m` / `image_output_cost_per_m` from the request body, so custom image models can be registered end-to-end. - catalog_sync: add `test_provider_catalog_parse_mixed_text_and_image` fixture that exercises a mixed text+image `[[models]]` file via `ProviderCatalogFile` — locks in the actual regression path (pre-fix, one image entry would cause `toml::from_str::<ProviderCatalogFile>` to fail and silently drop every provider model). - types: tighten `context_window` / `max_output_tokens` doc to require consumers treat `0` as unknown and supply their own default.
github-actions
Bot
added
area/kernel
The pre-commit hook ran `cargo xtask codegen --openapi` (1-2 minutes, test-profile compile) whenever any file under crates/librefang-api/src/routes/ was staged — including pure function-body / comment edits that cannot move the spec. Add a heuristic that scans the staged diff for spec-relevant lines (utoipa attributes, derives, item declarations, use statements, function signatures) and only runs codegen when at least one matches. Body-only diffs now skip the codegen step entirely. Also: stage openapi.json only if codegen actually changed it, so cosmetic touches do not produce empty "updated and staged" noise. There is no CI OpenAPI drift check today, so the heuristic is intentionally broad — false positives only cost compile time, false negatives would ship stale specs. Documented in the hook header alongside the manual override (`cargo xtask codegen --openapi`).
Adds an `openapi-drift` job to .github/workflows/ci.yml that runs `cargo xtask codegen --openapi` and `python3 scripts/codegen-sdks.py`, then fails the build if the regenerated openapi.json or any sdk/* file differs from what was committed. Gates on `rust == 'true' || ci == 'true'` because schema changes can originate anywhere in the workspace (utoipa derives in librefang-types, new ToSchema-implementing structs, etc.) — not just routes/. The pre-commit hook's spec-relevance heuristic is fast and catches the common case locally; this CI job is the authoritative gate. Update the hook header comment to reflect the new layered model: heuristic for local speed, CI job for correctness guarantee.
github-actions
Bot
added
size/L
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 642926586d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run: python3 scripts/codegen-sdks.py | ||
| - name: Verify no drift | ||
| run: | | ||
| if ! git diff --exit-code -- openapi.json sdk/; then |
There was a problem hiding this comment.
This check only runs git diff --exit-code, which ignores untracked files, so regenerated artifacts can still slip through. For example, if a tracked SDK file is removed in a PR, scripts/codegen-sdks.py will recreate it as untracked and this step still exits successfully, allowing missing/stale generated files to merge even though the job is intended to be authoritative.
Useful? React with 👍 / 👎.
…_tokens Codex flagged that #[serde(default)] on context_window and max_output_tokens silently accepts a `0` value for every modality, including text. The earlier d98ddbb fix only guarded the kernel-side consumers — a malformed text catalog entry would still land in the in-memory catalog and surface 0s to anyone reading directly. Add ModelCatalogEntry::validate(), called from both TOML parse paths (from_sources for boot, merge_catalog_file for incremental loads) and from the /api/models/custom handler. Image and audio entries continue to legitimately omit token limits; text entries with 0 are dropped with a tracing::warn at parse and rejected with 400 at the API. Add unit tests in librefang-types covering both the failing-text and passing-image/audio paths.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e912f5ef38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[serde(default)] | ||
| pub context_window: u64, |
There was a problem hiding this comment.
Adding #[serde(default)] to token-limit fields means serde_json::from_str::<Vec<ModelCatalogEntry>> in load_custom_models can now deserialize malformed text models with missing limits as 0 instead of failing. That loader path then calls add_custom_model without validate(), so invalid text entries from ~/.librefang/data/custom_models.json are accepted on boot and can reintroduce zero-limit models into runtime behavior after restart. Please enforce validate() (or equivalent strict deserialization) on the JSON custom-model load path.
Useful? React with 👍 / 👎.
| openapi-drift: | ||
| name: OpenAPI Drift | ||
| needs: changes | ||
| if: needs.changes.outputs.rust == 'true' || needs.changes.outputs.ci == 'true' |
There was a problem hiding this comment.
The new drift job only runs when changes.rust or changes.ci is true, but edits to scripts/codegen-sdks.py, openapi.json, or committed sdk/* files do not satisfy that predicate. In those PRs, the “authoritative gate” is skipped entirely, so stale generated SDK outputs can merge without regeneration checks. The job condition (or change detector outputs) should include SDK/codegen inputs so drift validation always runs when generator inputs change.
Useful? React with 👍 / 👎.
… / providers extras Final batch of catch-up docs covering the remaining Apr 22-25 user-facing features: - agent/hooks: transform_tool_result hook contract (#3003) - agent/templates: tool_search / tool_load lazy loading + lazy_tools opt-out (#3047) - integrations/mcp-a2a: TaintRuleId enum + per-tool skip_rules policy (#2999) - configuration/features: compaction summaries in user's conversation language (#3007) - configuration/providers/local: embedding auto-detection priority list (#3099) + Ollama capability detection (Modality / metadata pipeline, #3074 / #3133 / #3134 / #3140) - configuration/providers/management: custom image-gen provider via generic OpenAI-compat driver (#2998) + Moonshot file-upload helper via /v1/files (#2966) All sections mirrored to docs/src/app/zh/.
…pi / observability (#3189) * docs(providers): add Novita / SearXHG / Bedrock entries * docs(config): document [parallel_tools] section + max_history_messages * docs(scheduler): cron multi-delivery + pre_script + silent_marker * docs: live context.md, chat attachments, CLI slash commands Salvages three of the six items from the (quota-killed) misc-features agent run. Each is a focused append, no rewrite of existing content. - agent/templates: live Live Context section explains the per-turn re-read of `context.md` (#3115), the .identity/ vs root path lookup, the 32 KB cap with stale-cache fallback, and the cache_context opt-out. - integrations/api/agents: attachment-resolution table (image / PDF / text+code) covering the supported MIME / extension list and the 200 KB truncation, plus a note about adjacent-text coalescing for small chat-tuned local models (#3094). - integrations/cli/commands: 'Slash Commands in Chat' section listing the registry-backed CLI commands (/model /status /help /clear /kill /exit) and pointing at channel-side commands for the broader set (#3122 #3123). * docs: tool invoke API, trajectory export, doctor audit, CLI default-pick, SDK codegen Picks up the items the second misc-features agent didn't get to before hitting the org quota. - integrations/api/system: POST /api/tools/{name}/invoke (#3025) — fail- closed allowlist, agent_id requirement for approval-gated tools. - integrations/api/agents: GET /sessions/{id}/trajectory (#3097) — json/jsonl formats, redactor pipeline, audit use case. - operations/troubleshooting: 'Audit checks' subsection enumerating the three registered AuditChecks (#3082) — VaultKey, ApiListenAddr, ConfigTomlSchema — with what each asserts and how to extend. - configuration/providers/tools: 'CLI logins as first-class default providers' section (#3061) — detection priority claude-code > codex- cli > gemini-cli > qwen-code, override via [default_model] in config.toml. - integrations/sdk: 'Auto-generated from openapi.json' section (#3046) — scripts/codegen-sdks.py, pre-commit hook, openai-tag skip rule. * docs(zh): mirror providers / config-core / cron / features-index sections * docs(zh): mirror agent / api / cli / providers-tools / troubleshooting / sdk additions Round 2 of the zh translation pass — picks up the seven mid-priority files that the first commit didn't cover. All sections mirror the EN content from #3189 but read naturally as Chinese (terms like agent / session / fallback / NoEntry left in English where they're API surface, prose translated to 中文). - agent/templates: Live Context section, .identity/ vs root fallback, cache_context = true opt-out. - api/agents: attachment-resolution table + trajectory export endpoint (#3025 / #3094 / #3097). - api/system: POST /api/tools/{name}/invoke endpoint + fail-closed allowlist explanation. - cli/commands: 'Slash Commands in Chat' section listing CLI registry commands and pointing at channel commands. - providers/tools: 'CLI logins as first-class default providers' detection priority and override. - troubleshooting: 'Audit checks' subsection + table. - sdk: 'Auto-generated from openapi.json' section. * docs: refresh /new slash-command semantics (#3071 review fix) PR review on #3189 (houko) caught two stale references describing the pre-#3071 behavior of /new ('wipe the session' / 'clear history'). The new semantics fork off a fresh session while the previous one stays resumable on the same channel — phrasing updated in 4 places (en + zh × 2 files), plus a one-line clarifier in the slash-command roster section so the cross-reference doesn't silently inherit the old mental model: - configuration/channels: 'wipe the session' → 'fork off into a new session (the prior conversation stays resumable on the channel)' - integrations/api/realtime: '(clear history)' → 'forks into a fresh session — the prior session is preserved and remains resumable' - integrations/cli/commands roster paragraph: explicit note that /new is a fork, not a wipe. Same wording mirrored to docs/src/app/zh/. * docs(channels): 9 channel-related Apr 22-25 PRs (en + zh) Covers the channel + bridge feature wave: - #3020 per-agent ChannelOverrides — fixes the contradicting 'without modifying the agent manifest' phrasing in both en and zh. - #3077 prefix_agent_name + Signal plain-text default - #3009 Slack reactions_enabled + #3005 Feishu @mention preservation consolidated into a 'Reactions and Processing State' section - #3008 Signal media attachments - #3012 WhatsApp send_voice + dm_policy / group_policy - #3011 Webhook deliver_only mode - #2972 channel file downloads (file_download_dir / max_bytes config) * docs(config-core): provider timeout / browser CDP / cron session caps (en + zh) Three new `config.toml` sections that landed Apr 22-23: - #3004 `[provider_request_timeout_secs]` map - #2993 / #2991 `[browser].cdp_endpoint` + `cdp_auth_token_env` - #2994 `[kernel].cron_session_max_tokens` / `cron_session_max_messages` * docs(api): GET /attach + Owner Notice envelope (en + zh) - #3078 SSE attach endpoint for multi-client co-watching — describes the per-session SessionStreamHub fan-out, attacher behaviour, and the curl example. - #2965 ReplyEnvelope.owner_notice + notify_owner tool — message body field, StreamEvent variant, WhatsApp OWNER_JIDS fan-out. * docs(observability + ops): Tempo stack, cache_hit_ratio metric, daemon log tee (en + zh) - #3064 + #3149 — bundled observability stack (otel-collector / Tempo / Grafana), bring-up commands, [telemetry] config, business spans, cache_hit_ratio metric formula and where it surfaces - #3022 — `librefang start --foreground` now tees logs to ~/.librefang/logs/daemon-YYYY-MM-DD.log; same-day append; 7-day rotation pruned on daemon start * docs: hooks transform / lazy tools / mcp taint / capability detection / providers extras Final batch of catch-up docs covering the remaining Apr 22-25 user-facing features: - agent/hooks: transform_tool_result hook contract (#3003) - agent/templates: tool_search / tool_load lazy loading + lazy_tools opt-out (#3047) - integrations/mcp-a2a: TaintRuleId enum + per-tool skip_rules policy (#2999) - configuration/features: compaction summaries in user's conversation language (#3007) - configuration/providers/local: embedding auto-detection priority list (#3099) + Ollama capability detection (Modality / metadata pipeline, #3074 / #3133 / #3134 / #3140) - configuration/providers/management: custom image-gen provider via generic OpenAI-compat driver (#2998) + Moonshot file-upload helper via /v1/files (#2966) All sections mirrored to docs/src/app/zh/. * docs: address houko fix-PR audit (8 items, en + zh) PR review caught 1 stale-doc contradiction + 7 undocumented behaviour changes from the fix-PR cohort. - #3114 — configuration/core: 'api_key empty = unauthenticated' was stale; now documents the loopback-only policy (non-loopback bypass was closed) - #3170 — features/observability: auto_start is opt-in; home_dir- scoped Docker labels; RAII cleanup - #2989 — api/agents POST /message: per-request session_id override - #3048 — Interrupting a Running Agent Turn: /stop cascades into agent_send subagents (recursive) - #3056 + #3000 — desktop: dedicated Connection screen via custom URI scheme with retry / open-in-browser / uninstall - #2952 — triggers: response routes to agent's home channel - #2955 — triggers: task_posted assignee_match = 'self' | 'any' - #3069 — providers/local: 60-second reprobe + manual 'Test connection' refresh All sections mirrored in docs/src/app/zh/.
1 participant
Summary
Modalityenum (text/image/audio) toModelCatalogEntrywithTextas the serde default — mirrors themodalityfield in the librefang-registry schema extension.image_input_cost_per_m/image_output_cost_per_mfields for image-priced models (OpenAIgpt-image-2: $5/$10 text, $8/$30 image).context_window/max_output_tokenswith#[serde(default)]so image/audio models that legitimately have no context window parse cleanly. Previously they were requiredu64, meaning a singlemodality = "image"entry inproviders/openai.tomlwould failtoml::from_str::<ProviderCatalogFile>()atcatalog_sync.rs:91and silently drop every OpenAI model on daemon start./api/models,/api/models/{id}, and/api/providers/{id}so the dashboard can render image pricing.ModelCatalogEntry::is_image_generation()access helper and two parse-level regression tests.Why now
registry PR librefang/librefang-registry#71 is landing this schema on the registry side. librefang's source must accept the new shape before the registry merges, or the next
latesttag wipes OpenAI from every running daemon.Test plan
cargo check -p librefang-types -p librefang-runtime -p librefang-kernel-metering -p librefang-api(done locally, clean)cargo test -p librefang-types model_catalog— two new tests:test_image_generation_model_parses_without_context_window,test_text_model_defaults_to_text_modalitylibrefang start→curl /api/models | jq '.[] | select(.id == "gpt-image-2")'returns the entry withmodality: "image"and non-null image costs, and OpenAI text models are still present.