Skip to content

feat: Add default provider and engine params#234

Merged
ravisoundar merged 1 commit into
mainfrom
rs-params
Mar 10, 2026
Merged

feat: Add default provider and engine params#234
ravisoundar merged 1 commit into
mainfrom
rs-params

Conversation

@ravisoundar
Copy link
Copy Markdown
Collaborator

No description provided.

@ravisoundar ravisoundar requested a review from dmitsh as a code owner March 10, 2026 02:02
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.99%. Comparing base (e564c3c) to head (5c92d66).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
+ Coverage   66.52%   66.99%   +0.46%     
==========================================
  Files          82       82              
  Lines        4562     4572      +10     
==========================================
+ Hits         3035     3063      +28     
+ Misses       1411     1398      -13     
+ Partials      116      111       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 10, 2026

Greptile Summary

This PR extends the server Config struct with two new optional fields — ProviderParams and EngineParams — allowing operators to declare default provider/engine parameters in the server configuration file (YAML). The readRequest handler is updated to merge these config-level defaults into every incoming request, with request-supplied values taking precedence for overlapping keys. Three focused test functions are added to validate the empty-payload, no-overlap, and overlapping-key scenarios.

Key changes:

  • pkg/config/config.go: Adds ProviderParams map[string]any and EngineParams map[string]any fields with yaml:"...,omitempty" tags.
  • pkg/server/http_server.go: Implements nil-safe map initialization and merge logic — config defaults are only applied if the key is absent from the request payload. The implementation uses lazy allocation: maps are only created when there are actually values to merge from the config.
  • pkg/server/http_server_test.go: Adds TestReadRequest and TestReadInvalidRequest with httptest.NewRecorder to exercise the new merging behaviour without spinning up a real server.

The implementation is correct and the tests cover the key behavioural scenarios (empty payload, no overlap, and overlapping keys with proper precedence).

Confidence Score: 5/5

  • Safe to merge — implementation is correct, tests cover key scenarios, and all file changes follow intended patterns.
  • The PR implements a well-designed feature with correct nil-safe lazy initialization of config parameter maps. The merge logic properly preserves request-supplied values over config defaults. Test coverage is comprehensive, covering empty payloads, non-overlapping keys, and overlapping-key precedence scenarios. The YAML field definitions use proper omitempty tags for optional config parameters. No safety concerns identified.
  • No files require special attention

Last reviewed commit: 5c92d66

Comment thread pkg/server/http_server.go Outdated
Comment thread pkg/server/http_server_test.go
Signed-off-by: Ravi Shankar <ravish@nvidia.com>
@ravisoundar ravisoundar merged commit a9b6076 into main Mar 10, 2026
6 checks passed
@ravisoundar ravisoundar deleted the rs-params branch March 10, 2026 19:31
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.

2 participants