proxy: support user defined metadata in model configuration #333
Conversation
First initial plan (pre-expansion) to implement metadata in models configurations.
- add Metadata key to ModelConfig - include metadata in /v1/models under llamaswap_meta key - add recursive macro substitution into Metadata - change macros to be any scalar type
WalkthroughAdds typed macro support and recursive metadata macro substitution during config load, exposes per-model metadata in the /v1/models API as Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant YAML as Config (YAML)
participant Loader as LoadConfigFromReader
participant Macro as Macro Merger
participant Subst as substituteMetadataMacros
participant Store as ModelConfig
YAML->>Loader: Read global + per-model config
Loader->>Macro: Merge macros (MacroList map[string]any, add MODEL_ID/PORT)
Loader->>Subst: Substitute macros in string fields (cmd, cmdStop, etc.)
Loader->>Subst: Recursively substitute macros in Metadata (maps, lists, scalars)
Subst-->>Loader: Metadata with preserved scalar types / interpolated strings
Loader->>Store: Save ModelConfig with substituted Metadata
sequenceDiagram
autonumber
participant Client as Client
participant API as listModelsHandler
participant Cfg as Loaded ModelConfigs
Client->>API: GET /v1/models
API->>Cfg: Iterate model configs
alt model.Metadata non-empty
API->>API: Add `meta.llamaswap` to model record
else metadata empty
API->>API: Omit `meta.llamaswap`
end
API-->>Client: JSON list (ids + optional meta.llamaswap)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)proxy/config/config_test.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (14)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Makefile (1)
26-29: Consider preserving staticcheck exit status for visibility.The
|| truesuppresses staticcheck failures, which could hide code quality issues during development. Consider one of these alternatives:
- Remove
|| trueand let staticcheck failures be visible (developers can still iterate)- Add a warning message when staticcheck fails:
staticcheck ./proxy/... || echo "⚠️ staticcheck found issues"- Use a separate target for strict checking vs. fast iteration
Apply this diff if you want to preserve visibility of staticcheck issues:
test-dev: proxy/ui_dist/placeholder.txt go test -short ./proxy/... - staticcheck ./proxy/... || true + staticcheck ./proxy/... || echo "⚠️ Warning: staticcheck found issues"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CLAUDE.md(1 hunks)Makefile(2 hunks)ai-plans/issue-264-add-metadata.md(1 hunks)config.example.yaml(3 hunks)proxy/config/config.go(6 hunks)proxy/config/config_posix_test.go(1 hunks)proxy/config/config_test.go(1 hunks)proxy/config/model_config.go(1 hunks)proxy/proxymanager.go(1 hunks)proxy/proxymanager_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
proxy/config/config_posix_test.go (1)
proxy/config/config.go (1)
MacroList(19-19)
proxy/config/config_test.go (1)
proxy/config/config.go (1)
LoadConfigFromReader(103-288)
proxy/proxymanager_test.go (1)
proxy/config/config.go (1)
LoadConfigFromReader(103-288)
🔇 Additional comments (6)
config.example.yaml (1)
149-172: LGTM! Comprehensive metadata examples.The metadata examples effectively demonstrate the new functionality:
- Type preservation with scalar values (port, temperature)
- String interpolation in the note field
- Complex types (arrays, nested objects) with macro substitution
proxy/proxymanager.go (1)
373-376: LGTM! Clean conditional metadata inclusion.The implementation correctly adds
llamaswap_metaonly when metadata is present, preserving backward compatibility for models without metadata.proxy/config/model_config.go (1)
34-37: LGTM! Appropriate field addition.The
Metadatafield addition is well-documented with an issue reference and uses the appropriatemap[string]anytype to support flexible metadata structures.proxy/config/config_posix_test.go (1)
166-168: LGTM! Test correctly updated for MacroList type.The test properly reflects the MacroList type change from
map[string]stringtomap[string]any.proxy/config/config_test.go (1)
521-761: LGTM! Excellent test coverage for metadata and typed macros.The test suite comprehensively covers:
- Type preservation for all scalar types (string, int, float, bool)
- String interpolation behavior
- Nested data structures (maps and arrays)
- Macro precedence (model-level overriding global)
- Error handling for unknown macros and invalid types
- Validation distinguishing scalar vs. composite macro types
This thorough coverage will help prevent regressions as the metadata feature evolves.
proxy/proxymanager_test.go (1)
285-357: LGTM! Comprehensive metadata testing.
- Type preservation (int, float, bool) in llamaswap_meta
- String interpolation for composed values
- Nested metadata structures
- Conditional metadata presence
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
proxy/config/config_windows_test.go (1)
158-160: Type update looks correct.The update to
MacroListproperly reflects the type change frommap[string]stringtomap[string]any. The test continues to validate string macro substitution, which remains a valid use case.Consider adding test cases for non-string scalar macro types (int, float, bool) to verify they work correctly on Windows, if not already covered elsewhere.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
proxy/config/config_windows_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
proxy/config/config_windows_test.go (1)
proxy/config/config.go (1)
MacroList(19-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-tests
Create a meta top level key for a model's information will make it easier to find all metadata related to the model.
mostlygeek
changed the title
|
Thank you for the feature, and thank you for taking the time to document your process so thoroughly , it's inspiring and instructive to read! |
Changes:
Note about this PR:
The code for this PR was primarily written by Claude Code + Sonnet 4.5. I've very been against agentic code generation because the code was usually shit, very verbose and hard to review. CC with Sonnet 4.5 is the first time where those things haven't been entirely true for me.
This PR includes a workflow that I've been prototyping and found to be reliable using Sonnet 4.5:
ai-plans/issue-<num>-short-description.mdCLAUDE.mdto follow the guidelines for a plan.CLAUDE.mdto start automated assembly. I prefer to choose to accept all changes and review when Claude is done.I've kept the stages I went through as individual commits to make it easier to see the process.
From sitting down to write out the first plan to this PR was about 2.5 hours.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores