internal/config,server: add setParamsByMatch request filter - #934
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
| Filename | Overview |
|---|---|
| internal/server/filters.go | Core runtime implementation of setParamsByMatch; applySetParamsByMatch and setMergedParam are well-structured, but escapePathSegment omits ` |
| internal/config/filters.go | Adds MatchRule struct, Validate(), and SanitizedSet(); key regex, protected-param checks, and sorted output all look correct. |
| internal/config/macros.go | Extends macros to support map/list values, adds isScalarMacroValue guard, and propagates errors from substituteMacroList; self-reference detection via yaml.Marshal is a reasonable heuristic. |
| internal/config/load.go | Adds validateSetParamsByMatch hook that validates both model and peer rules at config load time; ordering and error wrapping look correct. |
| internal/config/config_test.go | Updates two pre-existing tests that assumed array/map macros were invalid, and adds comprehensive TestConfig_SetParamsByMatchValidation and TestConfig_StructuredMacros suites. |
| internal/server/filters_test.go | Adds TestServer_ApplySetParamsByMatch with 12 sub-tests covering the documented use case, merge behaviour, ordering, protected params, and ordering relative to stripParams/setParams. |
| config-schema.json | Adds setParamsByMatch array schema for both model and peer filters; also extends the macro value schema to allow object and array types. |
| internal/config/filters_test.go | Adds TestFilters_MatchRuleValidate and TestFilters_MatchRuleSanitizedSet covering edge cases for key validation and protected-param removal. |
Reviews (2): Last reviewed commit: "internal/config,server: add setParamsByM..." | Re-trigger Greptile
Set request parameters when a request field matches a value. Rules match on the request body rather than the model ID, so clients that send a fixed model and vary a separate field can change behaviour between requests without triggering a model swap. - rules run in order after stripParams and before setParams/setParamsByID - object values merge into an existing request object so keys the client already sent are preserved - protected params like model cannot be matched on or set - macros can now hold maps and lists, letting several models share one rule set; structured macros are only valid as a whole value fixes mostlygeek#958
a9b9538 to
600cc8b
Compare
Add a reusable macroReference definition and let model and peer
setParamsByMatch validate as either the rule array or a whole-value
macro string, so editors no longer flag ${rules} as a type error.
Set request parameters when a request field matches a value. Rules match on the request body rather than the model ID, so clients that send a fixed
modeland vary a separate field can change behaviour between requests without triggering a model swap.stripParamsand beforesetParams/setParamsByID, which keep the final saychat_template_kwargsthe client already sent are preserved rather than replacedmatchcompares against the field's string form, so JSON booleans and numbers must be quotedmodelcannot be matched on or setMacros can now hold maps and lists so several models can share one rule set. Without this a shared rule list needs a stray top-level YAML anchor key. Structured macros are only valid as a whole value; interpolating one into a string is a load-time error. This changes two existing tests that asserted macros were scalar-only.
Replaces the earlier reasoning-specific filter in this PR per the discussion in #958.
Fixes #958