Conversation
add exclusion to gosec for false hits on keys
WalkthroughA comprehensive set of HTTP content type and header constants was introduced and existing code was refactored to use these centralised constants instead of hardcoded string literals. Updates span application, middleware, proxy, handler, and test files. Minor test script adjustments and a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant ProxyCore
participant Constants
Client->>Handler: HTTP Request
Handler->>Constants: Use Header/Content-Type constant
Handler->>ProxyCore: Pass request with constants
ProxyCore->>Constants: Use header/content type constants
ProxyCore-->>Handler: Response
Handler-->>Client: HTTP Response (headers set via constants)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
add exclusion to gosec for false hits on keys # Conflicts: # internal/adapter/proxy/core/streaming_test.go # internal/adapter/proxy/proxy_streaming_profiles_test.go
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
internal/adapter/proxy/core/streaming.go (1)
32-36: Context key should use a custom type to avoid collisions
constants.ContextKeyStreamis a plain string. Best practice is:type ctxKey int const keyStream ctxKey = iotato prevent accidental collisions with other packages storing
"stream".Refactor if feasible.
♻️ Duplicate comments (2)
internal/app/handlers/handler_provider_ollama.go (1)
72-74: Same duplication comment applies hereSee previous note – factoring out the common JSON writer would simplify maintenance.
internal/app/handlers/handler_provider_lmstudio.go (1)
53-55: See duplication noteRepeats the same pattern—worth centralising.
🧹 Nitpick comments (8)
internal/core/constants/context.go (1)
8-8: Name order is inconsistent with the other context keysAll existing entries follow the
Context<Something>Keypattern (e.g.ContextRoutePrefixKey).
The new constant flips the order (ContextKeyStream). For consistency and grep-ability across the codebase, consider renaming to:- ContextKeyStream = "stream" + ContextStreamKey = "stream"Otherwise future searches and linters relying on a common prefix/suffix may miss this key.
test/scripts/streaming/run-all-tests.sh (2)
36-37: Update comment to reflect the removal of quick mode.The comment on line 36 mentions "quick mode" but the
--quickflag has been removed from the command on line 37. Please update the comment to reflect that the test now runs in full mode.-# Test 1: Streaming Detection (quick mode) -echo "1. Running streaming detection test (quick mode)..." +# Test 1: Streaming Detection +echo "1. Running streaming detection test..."
51-53: Update comment to reflect the removal of sample mode.The comment on line 52 mentions "sample mode" but the
--sampleflag has been removed from the command on line 53. Please update the comment to reflect that the test now runs in full mode.-# Test 3: Streaming Responses -echo "3. Running streaming responses test (sample mode)..." +# Test 3: Streaming Responses +echo "3. Running streaming responses test..."internal/app/handlers/handler_provider_ollama.go (1)
27-28: Duplicate header-setting code across handlersEvery provider handler now repeats:
w.Header().Set(constants.HeaderContentType, constants.ContentTypeJSON)Consider a tiny helper, e.g.:
func writeJSON(w http.ResponseWriter, status int, v any) { w.Header().Set(constants.HeaderContentType, constants.ContentTypeJSON) w.WriteHeader(status) _ = json.NewEncoder(w).Encode(v) }to cut duplication and guarantee consistent behaviour.
internal/app/handlers/handler_provider_lmstudio.go (1)
28-30: JSON helper abstraction would reduce repetitionSame suggestion as for the Ollama handler: extract a common
writeJSONhelper to avoid copy-pasted three-liner blocks.internal/adapter/proxy/core/streaming.go (1)
68-79: Mixed prefixes vs full MIME types
binaryPrefixescontains full types likeapplication/pdfand prefix markers likeimage/. Mixing them while relying onHasPrefixmay miss matches (e.g.,application/pdfwill only match if the header is exactly that string with no charset suffix). Consider:
- Keep only real prefixes here (
image/,video/, etc.)- Move exact matches (
application/pdf,application/zip, …) intobinaryTypesThis makes the intent clearer and avoids false negatives.
internal/adapter/proxy/core/common.go (1)
55-63: Header filter is case-sensitive despite canonicalisation – usestrings.EqualFold.
normalisedHeaderis canonicalised, but you compare it with mixed-case constants using==. While canonical forms should match, a future constant added in lower-case (e.g.x-auth-token) would silently bypass the filter.
Switching tostrings.EqualFoldremoves this edge case at negligible cost.- if normalisedHeader == constants.HeaderAuthorization || - normalisedHeader == constants.HeaderCookie || + if strings.EqualFold(normalisedHeader, constants.HeaderAuthorization) || + strings.EqualFold(normalisedHeader, constants.HeaderCookie) || ...internal/adapter/proxy/proxy_streaming_profiles_test.go (1)
395-419: Hard-coded MIME types reduce consistency with the new constants.Most tests now rely on
constants.ContentType…, but the comprehensive matrix still hard-codes"text/event-stream"and"image/png". Using the same constants keeps the suite self-documenting and shields it from typos or later changes.- contentType: "text/event-stream", + contentType: constants.ContentTypeEventStream, ... - contentType: "image/png", + contentType: constants.ContentTypeImagePNG,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.gosec.json(1 hunks)internal/adapter/discovery/http_client.go(2 hunks)internal/adapter/inspector/body_inspector.go(2 hunks)internal/adapter/proxy/core/common.go(7 hunks)internal/adapter/proxy/core/streaming.go(3 hunks)internal/adapter/proxy/core/streaming_test.go(5 hunks)internal/adapter/proxy/olla/service.go(2 hunks)internal/adapter/proxy/proxy_benchmark_test.go(5 hunks)internal/adapter/proxy/proxy_headers_test.go(5 hunks)internal/adapter/proxy/proxy_streaming_profiles_test.go(10 hunks)internal/adapter/proxy/proxy_test.go(10 hunks)internal/adapter/proxy/sherpa/service.go(2 hunks)internal/adapter/proxy/sherpa/service_streaming.go(2 hunks)internal/app/handlers/handler_health.go(1 hunks)internal/app/handlers/handler_process.go(2 hunks)internal/app/handlers/handler_provider_generic.go(3 hunks)internal/app/handlers/handler_provider_lmstudio.go(3 hunks)internal/app/handlers/handler_provider_models_test.go(2 hunks)internal/app/handlers/handler_provider_ollama.go(3 hunks)internal/app/handlers/handler_provider_openai.go(2 hunks)internal/app/handlers/handler_proxy.go(2 hunks)internal/app/handlers/handler_proxy_model_test.go(2 hunks)internal/app/handlers/handler_stats_models.go(2 hunks)internal/app/handlers/handler_status.go(2 hunks)internal/app/handlers/handler_status_endpoints.go(2 hunks)internal/app/handlers/handler_status_models.go(2 hunks)internal/app/handlers/handler_unified_models.go(3 hunks)internal/app/handlers/handler_version.go(2 hunks)internal/app/handlers/server.go(2 hunks)internal/app/middleware/logging.go(4 hunks)internal/core/constants/content.go(1 hunks)internal/core/constants/context.go(1 hunks)test/scripts/streaming/run-all-tests.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
internal/{app,adapter}/**/*.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
Endpoints should be exposed at
/internal/healthand/internal/status.
Files:
internal/adapter/discovery/http_client.gointernal/app/handlers/handler_provider_lmstudio.gointernal/adapter/proxy/olla/service.gointernal/app/handlers/handler_status_models.gointernal/app/handlers/handler_provider_ollama.gointernal/app/handlers/handler_health.gointernal/app/handlers/handler_unified_models.gointernal/adapter/proxy/proxy_headers_test.gointernal/app/handlers/handler_process.gointernal/app/handlers/handler_provider_generic.gointernal/app/handlers/handler_version.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/app/handlers/handler_status_endpoints.gointernal/adapter/proxy/sherpa/service.gointernal/app/handlers/handler_status.gointernal/app/handlers/handler_provider_openai.gointernal/app/handlers/handler_stats_models.gointernal/app/handlers/handler_proxy_model_test.gointernal/adapter/proxy/core/streaming.gointernal/app/middleware/logging.gointernal/app/handlers/handler_proxy.gointernal/app/handlers/handler_provider_models_test.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/proxy/core/streaming_test.gointernal/adapter/inspector/body_inspector.gointernal/app/handlers/server.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/core/common.gointernal/adapter/proxy/proxy_test.go
**/*_test.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unit tests should test individual components in isolation.
Files:
internal/adapter/proxy/proxy_headers_test.gointernal/app/handlers/handler_proxy_model_test.gointernal/app/handlers/handler_provider_models_test.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/proxy/core/streaming_test.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/proxy_test.go
internal/adapter/proxy/*_test.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
internal/adapter/proxy/*_test.go: Integration tests should test the full request flow through the proxy.
Benchmark tests should measure performance of critical paths, proxy engine comparisons, connection pooling efficiency, and circuit breaker behavior.
Shared proxy tests should ensure compatibility between both proxy engines.
Files:
internal/adapter/proxy/proxy_headers_test.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/proxy_test.go
internal/adapter/proxy/*.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
Expose the following response headers:
X-Olla-Endpoint,X-Olla-Model,X-Olla-Backend-Type,X-Olla-Request-ID,X-Olla-Response-Time.
Files:
internal/adapter/proxy/proxy_headers_test.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/proxy_test.go
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.788Z
Learning: Applies to internal/adapter/proxy/*.go : Expose the following response headers: `X-Olla-Endpoint`, `X-Olla-Model`, `X-Olla-Backend-Type`, `X-Olla-Request-ID`, `X-Olla-Response-Time`.
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to internal/adapter/proxy/*_test.go : Shared proxy tests should ensure compatibility between both proxy engines.
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.788Z
Learning: Applies to internal/{app,adapter}/**/*.go : Endpoints should be exposed at `/internal/health` and `/internal/status`.
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to internal/adapter/proxy/*_test.go : Integration tests should test the full request flow through the proxy.
📚 Learning: applies to internal/adapter/proxy/*.go : expose the following response headers: `x-olla-endpoint`, `...
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.788Z
Learning: Applies to internal/adapter/proxy/*.go : Expose the following response headers: `X-Olla-Endpoint`, `X-Olla-Model`, `X-Olla-Backend-Type`, `X-Olla-Request-ID`, `X-Olla-Response-Time`.
Applied to files:
internal/adapter/discovery/http_client.gointernal/app/handlers/handler_provider_lmstudio.gointernal/core/constants/context.gointernal/adapter/proxy/olla/service.gointernal/app/handlers/handler_status_models.gointernal/app/handlers/handler_provider_ollama.gointernal/app/handlers/handler_health.gointernal/app/handlers/handler_unified_models.gointernal/adapter/proxy/proxy_headers_test.gointernal/app/handlers/handler_process.gointernal/app/handlers/handler_provider_generic.gointernal/app/handlers/handler_version.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/app/handlers/handler_status_endpoints.gointernal/adapter/proxy/sherpa/service.gointernal/app/handlers/handler_status.gointernal/app/handlers/handler_provider_openai.gointernal/app/handlers/handler_stats_models.gointernal/app/handlers/handler_proxy_model_test.gointernal/app/middleware/logging.gointernal/app/handlers/handler_proxy.gointernal/app/handlers/handler_provider_models_test.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/inspector/body_inspector.gointernal/app/handlers/server.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/core/common.gointernal/adapter/proxy/proxy_test.gointernal/core/constants/content.go
📚 Learning: applies to handler_proxy.go : request routing logic should be implemented in `handler_proxy.go`....
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to handler_proxy.go : Request routing logic should be implemented in `handler_proxy.go`.
Applied to files:
internal/core/constants/context.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/adapter/proxy/sherpa/service.gointernal/app/handlers/handler_proxy.gointernal/adapter/proxy/core/common.gointernal/adapter/proxy/proxy_test.go
📚 Learning: applies to internal/{app,adapter}/**/*.go : endpoints should be exposed at `/internal/health` and `/...
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.788Z
Learning: Applies to internal/{app,adapter}/**/*.go : Endpoints should be exposed at `/internal/health` and `/internal/status`.
Applied to files:
internal/adapter/proxy/olla/service.gointernal/app/handlers/handler_status_models.gointernal/app/handlers/handler_health.gointernal/app/handlers/handler_unified_models.gointernal/adapter/proxy/proxy_headers_test.gointernal/app/handlers/handler_process.gointernal/app/handlers/handler_version.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/app/handlers/handler_status_endpoints.gointernal/adapter/proxy/sherpa/service.gointernal/app/handlers/handler_status.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/app/handlers/server.gointernal/adapter/proxy/core/common.gointernal/adapter/proxy/proxy_test.go
📚 Learning: applies to {proxy_sherpa.go,proxy_olla.go} : proxy implementations should be in `proxy_sherpa.go` an...
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to {proxy_sherpa.go,proxy_olla.go} : Proxy implementations should be in `proxy_sherpa.go` and `proxy_olla.go`.
Applied to files:
internal/adapter/proxy/olla/service.gointernal/adapter/proxy/proxy_headers_test.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/adapter/proxy/sherpa/service.gointernal/app/middleware/logging.gointernal/app/handlers/handler_proxy.gointernal/adapter/inspector/body_inspector.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/core/common.gointernal/adapter/proxy/proxy_test.go
📚 Learning: applies to internal/adapter/proxy/*_test.go : shared proxy tests should ensure compatibility between...
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to internal/adapter/proxy/*_test.go : Shared proxy tests should ensure compatibility between both proxy engines.
Applied to files:
internal/adapter/proxy/olla/service.gointernal/adapter/proxy/proxy_headers_test.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/adapter/proxy/sherpa/service.gointernal/app/handlers/handler_proxy_model_test.gointernal/app/handlers/handler_proxy.gointernal/app/handlers/handler_provider_models_test.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/proxy/core/streaming_test.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/core/common.gointernal/adapter/proxy/proxy_test.go
📚 Learning: applies to internal/adapter/proxy/*_test.go : benchmark tests should measure performance of critical...
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to internal/adapter/proxy/*_test.go : Benchmark tests should measure performance of critical paths, proxy engine comparisons, connection pooling efficiency, and circuit breaker behavior.
Applied to files:
internal/adapter/proxy/olla/service.gointernal/adapter/proxy/proxy_headers_test.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/app/handlers/handler_proxy_model_test.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/proxy/core/streaming_test.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/proxy_test.go
📚 Learning: applies to internal/adapter/proxy/*_test.go : integration tests should test the full request flow th...
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to internal/adapter/proxy/*_test.go : Integration tests should test the full request flow through the proxy.
Applied to files:
internal/adapter/proxy/proxy_headers_test.gointernal/adapter/proxy/sherpa/service_streaming.gointernal/adapter/proxy/sherpa/service.gointernal/app/handlers/handler_proxy_model_test.gointernal/app/handlers/handler_proxy.gointernal/adapter/proxy/proxy_benchmark_test.gointernal/adapter/proxy/core/streaming_test.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/core/common.gointernal/adapter/proxy/proxy_test.go
📚 Learning: applies to **/*_test.go : unit tests should test individual components in isolation....
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Applies to **/*_test.go : Unit tests should test individual components in isolation.
Applied to files:
internal/adapter/proxy/proxy_headers_test.gointernal/app/handlers/handler_proxy_model_test.gointernal/adapter/proxy/core/streaming_test.gointernal/adapter/proxy/proxy_streaming_profiles_test.gointernal/adapter/proxy/proxy_test.go
📚 Learning: go version 1.24 or higher must be used....
Learnt from: CR
PR: thushan/olla#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T12:59:29.787Z
Learning: Go version 1.24 or higher must be used.
Applied to files:
internal/app/handlers/handler_version.go
🧬 Code Graph Analysis (28)
internal/adapter/discovery/http_client.go (1)
internal/core/constants/content.go (1)
DefaultContentTypeJSON(4-4)
internal/app/handlers/handler_provider_lmstudio.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/adapter/proxy/olla/service.go (1)
internal/core/constants/content.go (1)
HeaderContentType(57-57)
internal/app/handlers/handler_status_models.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_provider_ollama.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_health.go (2)
internal/app/handlers/application.go (1)
Application(69-85)internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_unified_models.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/adapter/proxy/proxy_headers_test.go (1)
internal/core/constants/content.go (7)
HeaderContentType(57-57)ContentTypeJSON(7-7)HeaderXOllaEndpoint(102-102)HeaderXServedBy(91-91)HeaderXOllaModel(104-104)HeaderXOllaRequestID(101-101)HeaderXOllaBackendType(103-103)
internal/app/handlers/handler_process.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_provider_generic.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_version.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/adapter/proxy/sherpa/service_streaming.go (1)
internal/core/constants/content.go (1)
HeaderContentType(57-57)
internal/app/handlers/handler_status_endpoints.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/adapter/proxy/sherpa/service.go (1)
internal/core/constants/content.go (1)
HeaderContentType(57-57)
internal/app/handlers/handler_status.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_provider_openai.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_stats_models.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/handler_proxy_model_test.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/adapter/proxy/core/streaming.go (2)
internal/core/constants/content.go (22)
HeaderContentType(57-57)ContentTypeEventStream(16-16)ContentTypeNDJSON(17-17)ContentTypeStreamJSON(18-18)ContentTypeJSONSeq(19-19)ContentTypeTextUTF8(20-20)ContentTypePrefixImage(47-47)ContentTypePrefixVideo(48-48)ContentTypePrefixAudio(49-49)ContentTypePDF(23-23)ContentTypeZIP(24-24)ContentTypeGZIP(25-25)ContentTypeTAR(26-26)ContentTypeRAR(27-27)ContentType7Z(28-28)ContentTypePrefixFont(50-50)ContentTypePrefixModel(51-51)ContentTypeOctetStream(29-29)ContentTypeExcel(30-30)ContentTypeOfficeDocument(32-32)ContentTypeWordDOC(33-33)ContentTypePowerPoint(34-34)internal/core/constants/context.go (1)
ContextKeyStream(8-8)
internal/app/middleware/logging.go (1)
internal/core/constants/content.go (3)
HeaderXRequestID(87-87)HeaderContentType(57-57)HeaderAccept(58-58)
internal/app/handlers/handler_proxy.go (1)
internal/core/constants/content.go (1)
HeaderContentType(57-57)
internal/app/handlers/handler_provider_models_test.go (1)
internal/core/constants/content.go (2)
ContentTypeJSON(7-7)HeaderContentType(57-57)
internal/adapter/proxy/proxy_benchmark_test.go (1)
internal/core/constants/content.go (4)
HeaderContentType(57-57)ContentTypeJSON(7-7)ContentTypeText(8-8)HeaderAccept(58-58)
internal/adapter/proxy/core/streaming_test.go (2)
internal/core/constants/content.go (29)
ContentTypeImagePNG(37-37)ContentTypeEventStream(16-16)ContentTypeNDJSON(17-17)ContentTypeStreamJSON(18-18)ContentTypeJSONSeq(19-19)ContentTypeTextUTF8(20-20)ContentTypeImageJPEG(38-38)ContentTypeImageWebP(39-39)ContentTypeVideoMP4(43-43)ContentTypePDF(23-23)ContentTypeZIP(24-24)ContentTypeOctetStream(29-29)ContentTypeExcel(30-30)ContentTypeWordDOCX(31-31)ContentTypeJSON(7-7)ContentTypeHTML(9-9)ContentTypeXML(10-10)ContentTypeText(8-8)HeaderContentType(57-57)ContentTypeImageSVG(40-40)ContentTypeVideoWebM(44-44)ContentTypeWordDOC(33-33)ContentTypePowerPoint(34-34)ContentTypeGZIP(25-25)ContentTypeTAR(26-26)ContentTypeRAR(27-27)ContentType7Z(28-28)ContentTypeJavaScript(11-11)ContentTypeCSS(12-12)internal/core/constants/config.go (2)
ConfigurationProxyProfileStandard(6-6)ConfigurationProxyProfileAuto(4-4)
internal/adapter/inspector/body_inspector.go (1)
internal/core/constants/content.go (2)
HeaderContentType(57-57)ContentTypeJSON(7-7)
internal/app/handlers/server.go (1)
internal/core/constants/content.go (1)
HeaderContentType(57-57)
internal/adapter/proxy/proxy_streaming_profiles_test.go (3)
internal/core/constants/config.go (3)
ConfigurationProxyProfileStandard(6-6)ConfigurationProxyProfileStreaming(5-5)ConfigurationProxyProfileAuto(4-4)internal/core/constants/content.go (9)
ContentTypeEventStream(16-16)ContentTypeStreamJSON(18-18)ContentTypeText(8-8)ContentTypeImagePNG(37-37)ContentTypePDF(23-23)ContentTypeNDJSON(17-17)ContentTypeImageJPEG(38-38)ContentTypeTextUTF8(20-20)HeaderContentType(57-57)internal/core/constants/context.go (1)
ContextKeyStream(8-8)
internal/adapter/proxy/proxy_test.go (1)
internal/core/constants/content.go (3)
HeaderContentType(57-57)ContentTypeJSON(7-7)ContentTypeText(8-8)
🔇 Additional comments (63)
.gosec.json (1)
6-9: Verify that skipping G101 only suppresses false positivesExempting
internal/core/constants/content.gofrom hard-coded credential checks is reasonable for MIME/content-type literals, but double-check that:
- No secrets (API keys, tokens) have been added alongside those literals.
- The exemption remains narrowly scoped to that single file.
A quick manual scan or targeted secret-scanning job in CI would mitigate accidental secret leakage.
internal/adapter/proxy/olla/service.go (1)
432-434: LGTM – constant used correctlyReplacing the magic string with
constants.HeaderContentTypekeeps header keys centralised and typo-safe. No functional impact.internal/adapter/proxy/sherpa/service_streaming.go (1)
41-42: LGTM – header lookup now uses canonical constantThe refactor improves maintainability while leaving behaviour unchanged.
internal/adapter/proxy/sherpa/service.go (1)
158-160: LGTM – panic handler now relies on centralised constantGood move towards eliminating hard-coded header strings.
internal/app/handlers/handler_provider_openai.go (2)
6-8: LGTM - Clean import addition!The import of the constants package is properly structured and follows Go conventions.
29-29: LGTM - Proper constant usage!The replacement of hardcoded strings with centralized constants improves maintainability and consistency.
internal/app/handlers/handler_proxy_model_test.go (2)
16-16: LGTM - Appropriate test import!The constants import is correctly added to support the test's use of centralized header constants.
158-158: LGTM - Consistent test constant usage!Excellent application of the centralized constants pattern to test code, ensuring consistency across both production and test code.
internal/app/handlers/handler_status_models.go (2)
11-12: LGTM - Well-formatted import addition!The constants import is properly added with appropriate formatting.
107-107: LGTM - Consistent constant implementation!The header constant usage follows the established pattern and improves code maintainability.
internal/app/handlers/handler_version.go (2)
8-9: LGTM - Consistent import structure!The constants import follows the established pattern with proper formatting.
68-68: LGTM - Proper constant usage in version handler!The centralized constants are correctly applied, maintaining consistency across all handlers.
internal/app/handlers/handler_stats_models.go (2)
9-10: LGTM - Standard import addition!The constants import is correctly added with consistent formatting.
106-106: LGTM - Completes the consistent refactoring!The final handler now uses centralized constants, completing the systematic refactoring across all reviewed files.
internal/app/handlers/handler_process.go (2)
8-9: LGTM!The constants import is properly added and positioned correctly within the import block.
94-94: Excellent refactoring to centralized constants!The replacement of hardcoded strings with
constants.HeaderContentTypeandconstants.ContentTypeJSONimproves maintainability and consistency across the codebase.internal/app/handlers/handler_status.go (2)
12-13: LGTM!The constants import is correctly positioned and follows Go import conventions.
122-122: Perfect constant usage!The refactoring to use centralized constants for HTTP header management is well-executed and improves code maintainability.
internal/adapter/inspector/body_inspector.go (3)
12-13: LGTM!The constants import is properly positioned and follows Go import conventions.
59-59: Good refactoring!Using
constants.HeaderContentTypeinstead of the hardcoded string improves consistency and maintainability.
60-60: Excellent constant usage!The replacement of the hardcoded "application/json" string with
constants.ContentTypeJSONaligns perfectly with the refactoring objectives and improves code consistency.internal/app/handlers/handler_unified_models.go (3)
12-13: LGTM!The constants import is correctly positioned and follows Go import conventions.
137-137: Perfect refactoring!The use of centralized constants for HTTP header management is well-implemented and improves code consistency.
303-303: Consistent refactoring implementation!The use of centralized constants across both handler functions maintains excellent consistency and follows the established refactoring pattern.
internal/app/handlers/handler_status_endpoints.go (2)
9-10: LGTM!The constants import is correctly positioned and follows Go import conventions.
94-94: Excellent completion of the refactoring pattern!The use of centralized constants for HTTP header management is consistently implemented across all handler functions, demonstrating a well-coordinated refactoring effort that improves maintainability and consistency.
internal/app/handlers/handler_provider_models_test.go (2)
16-16: LGTM! Import properly added for constants package.The import is necessary for using the centralised constants and follows proper Go conventions.
211-211: Excellent refactoring to use centralised constants.Replacing hardcoded strings with
constants.HeaderContentTypeandconstants.ContentTypeJSONimproves maintainability and reduces the risk of typos. This aligns perfectly with the broader codebase refactoring effort.test/scripts/streaming/run-all-tests.sh (1)
43-45: Good improvement to test thoroughness.Increasing the question count from 3 to 5 enhances the comprehensiveness of the streaming latency test. The comment and parameter are consistently updated.
internal/adapter/discovery/http_client.go (2)
13-14: LGTM! Import properly added for constants package.The import is necessary for using the centralised constants and is correctly placed in the import block.
26-26: Excellent refactoring to centralise content type constant.Replacing the hardcoded string with
constants.DefaultContentTypeJSONimproves maintainability and ensures consistency across the codebase. This aligns perfectly with the broader refactoring effort.internal/app/handlers/handler_provider_generic.go (3)
6-7: LGTM! Import properly added for constants package.The import is correctly placed and necessary for using the centralised header and content type constants.
32-32: Good refactoring to use centralised constants.Replacing hardcoded strings with
constants.HeaderContentTypeandconstants.ContentTypeJSONimproves maintainability and follows the established refactoring pattern.
70-70: Consistent refactoring with centralised constants.Using the same centralised constants as line 32 maintains consistency throughout the file and improves maintainability.
internal/app/handlers/handler_proxy.go (2)
63-63: Good refactoring to use centralised header constant.Replacing the hardcoded
"Content-Type"withconstants.HeaderContentTypeimproves maintainability and reduces the risk of typos in header names.
188-188: Consistent use of centralised header constant.Using
constants.HeaderContentTypemaintains consistency with line 63 and follows the established refactoring pattern while preserving the important logic that prevents double-writing response headers.internal/app/handlers/handler_health.go (2)
7-8: Centralised constants import looks goodReplacing string literals with
constants.*keeps header names consistent across the codebase and avoids typos.
14-14: Header constant usage is correct
constants.HeaderContentType/constants.ContentTypeJSONare set before the body is written – all good.internal/app/handlers/handler_provider_ollama.go (1)
7-8: Good move to centralised constantsImporting
constantskeeps header names uniform.internal/app/handlers/handler_provider_lmstudio.go (1)
7-8: Consistent constant usageImporting the shared constants module keeps things tidy.
internal/adapter/proxy/core/streaming.go (3)
24-24: Constant for header key – ✅Switching to
constants.HeaderContentTyperemoves magic strings.
48-53: Streaming MIME type list reads wellCentralising the literals makes maintenance easier.
81-86: Binary types list – looks goodExplicit matches are now constants; implementation is sound.
internal/adapter/proxy/proxy_headers_test.go (7)
12-13: Import alias update is correctUsing the constants package in tests keeps expectations in lock-step with production code.
20-23: Upstream stub now uses constants – goodMinor but keeps tests resilient to header-name refactors.
71-77: Header assertions aligned with constantsAssertions correctly reference the new constant names; test intent unchanged.
79-80: Content-Type assertion uses constant – ✅No further comments.
97-106: Model-specific header checks accurateTest covers both proxy engines and validates all required headers; looks solid.
116-119: Attempted upstream override scenario well coveredEnsures proxy defends its own headers against upstream overwrites.
141-145: Final assertions correctProtected headers affirmed; test passes the guideline for shared proxy behaviour.
internal/app/middleware/logging.go (4)
10-11: LGTM: Constants import added correctly.The import of the constants package aligns with the project-wide effort to centralise HTTP header and content type definitions.
77-77: LGTM: Header constant replacement.Replacing the hardcoded
"X-Request-ID"string withconstants.HeaderXRequestIDimproves maintainability and consistency.
172-173: LGTM: Content type and accept header constants.The replacement of hardcoded header strings with
constants.HeaderContentTypeandconstants.HeaderAcceptfollows the established pattern for centralising HTTP header definitions.
181-196: LGTM: formatBytes function improvements.The improvements to the
formatBytesfunction are well-implemented:
- Defining suffixes as a constant improves readability
- Adding boundary check prevents potential out-of-range access
- The logic remains functionally equivalent whilst being more robust
internal/adapter/proxy/core/streaming_test.go (3)
29-29: LGTM: Comprehensive content type constant replacements.All hardcoded content type strings have been systematically replaced with their corresponding constants from the
constantspackage. This greatly improves maintainability and reduces the risk of typos in content type strings.Also applies to: 35-35, 43-43, 49-49, 55-55, 61-61, 67-67, 75-75, 81-81, 87-87, 93-93, 105-105, 111-111, 117-117, 123-123, 129-129, 149-149, 155-155, 161-161, 167-167, 175-175, 182-182, 189-189, 232-232
71-97: LGTM: Improved test case naming.The test case names have been appropriately updated from "uses standard" to "buffers" for binary content types, which better describes the actual behaviour being tested.
Also applies to: 121-143
249-257: LGTM: Test assertion constant usage.All test assertions now consistently use constants for content types, ensuring the tests remain aligned with the centralised constant definitions.
Also applies to: 276-282, 288-313
internal/adapter/proxy/proxy_benchmark_test.go (2)
15-15: LGTM: Constants import added.The import of the constants package enables the use of centralised HTTP header and content type definitions throughout the benchmark tests.
111-111: LGTM: Header and content type constants in benchmarks.All hardcoded HTTP header names and content type strings have been systematically replaced with their corresponding constants. This ensures benchmark tests use identical string values as the production code, maintaining consistency and reducing maintenance overhead.
Also applies to: 147-147, 178-178, 196-196, 408-408, 411-411
internal/app/handlers/server.go (2)
8-11: LGTM: Import reorganisation and constants addition.The addition of the constants import and reorganisation of imports aligns with the project-wide refactoring to centralise HTTP header definitions.
71-71: LGTM: Content-Type header constant usage.The replacement of the local
ContentTypeHeaderconstant withconstants.HeaderContentTypefollows the established pattern for using centralised header definitions.internal/adapter/proxy/proxy_test.go (2)
19-19: LGTM: Constants import added.The import enables consistent use of centralised HTTP header and content type definitions across all proxy tests.
196-196: LGTM: Comprehensive header and content type constant usage.All hardcoded HTTP header names and content type strings have been systematically replaced with their corresponding constants from the
constantspackage. This ensures consistent string values across both Sherpa and Olla proxy implementations and their tests, supporting the requirement for shared proxy tests to ensure compatibility between both engines.Also applies to: 356-356, 460-460, 492-492, 648-648, 687-687, 998-998, 1000-1000, 1011-1011, 1024-1024
This PR ensures we constant-ise things properly and reorganises things.
You could say it's Constant Craving.
Summary by CodeRabbit
Refactor
Chores
New Features