-
Notifications
You must be signed in to change notification settings - Fork 120
Api version endpoint #395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Api version endpoint #395
Conversation
…use version package This commit refactors how version information is handled in the application by moving from main package variables to a dedicated version package. The changes include: - Removed version variables from llama-swap.go and imported the new version package - Modified version output in main to use the version package variables - Added a new /version API endpoint in proxy manager that returns version information as JSON - Imported version package in proxymanager_api.go for the new endpoint - Updated Makefile build commands to inject version info into github.com/mostlygeek/llama-swap/version package instead of main package These changes make version handling more maintainable and provide an API endpoint for external tools to query the application version.
This commit adds a new test case for the ProxyManager's /api/version endpoint. The test verifies that: - The endpoint returns a 200 OK status code - The response has the correct Content-Type header (application/json) - The response body is valid JSON containing a "version" field - The version field exists and is a string type The test follows the same pattern as other ProxyManager tests, using a test configuration with a simple model responder and validating the HTTP response through the proxy's ServeHTTP method. A defer statement ensures proper cleanup of proxy processes after the test completes.
…reconnection This commit enhances the ConnectionStatus component to fetch and display version information when the connection is established. It also fixes a bug where connectionStatus is only updates once upon disconnection. The changes include: - Added useEffect hook to automatically fetch version info when connection status becomes "connected" - Introduced new VersionInfo interface and APIEventEnvelope interface in APIProvider - Added versionInfo state and getVersionInfo async function to APIProvider context - Updated ConnectionStatusIcon to display detailed tooltip with version, commit hash, and build date - Reset versionInfo to null when connection is disconnected - Extended APIProviderType interface to include versionInfo and getVersionInfo - Updated useMemo dependencies in APIProvider to include connectionStatus (fix reconnect bug) and versionInfo - Added proper error handling in getVersionInfo fetch call The version information is fetched from the /api/version endpoint and displayed in the connection status tooltip, providing users with build details when the system is connected.
This commit introduces a new version package that provides structured access to build metadata including version number, git commit hash, and build date. The package exports a VersionStruct type with JSON tags for easy serialization and includes default values for development builds. Key changes: - Create new version/version.go file - Define VersionStruct with version, commit, and date fields - Add package-level variables for easy access to build info - Include JSON struct tags for API compatibility - Set default placeholder values for local development This lays the foundation for build-time version injection and programmatic access to release information.
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Build
participant Server as ProxyManager/API
participant APIProvider
participant UI as ConnectionStatus
rect rgba(100,150,240,0.08)
Build->>Build: compile with ldflags\nset version.Version/Commit/Date
end
UI->>APIProvider: connectionStatus -> "connected"
APIProvider->>Server: GET /api/version
alt success
Server-->>APIProvider: 200 {version,commit,date}
APIProvider->>APIProvider: set versionInfo
APIProvider-->>UI: provide versionInfo
UI->>UI: render title with versionInfo
else error
Server-->>APIProvider: error/500
APIProvider->>UI: keep versionInfo null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
proxy/proxymanager_test.go (1)
1087-1119: Consider asserting all version fields.The test verifies the presence of the
versionfield but doesn't check forcommitanddate. Consider asserting all three fields to ensure the completeVersionJSONstructure is returned.Apply this diff to enhance the test:
// Check for version _, versionExists := responseData["version"] assert.True(t, versionExists, "version should exist in the response") _, ok := responseData["version"].(string) assert.True(t, ok, "version should be a string") + + // Check for commit + _, commitExists := responseData["commit"] + assert.True(t, commitExists, "commit should exist in the response") + _, ok = responseData["commit"].(string) + assert.True(t, ok, "commit should be a string") + + // Check for date + _, dateExists := responseData["date"] + assert.True(t, dateExists, "date should exist in the response") + _, ok = responseData["date"].(string) + assert.True(t, ok, "date should be a string")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.goreleaser.yaml(2 hunks)Makefile(1 hunks)llama-swap.go(2 hunks)proxy/proxymanager_api.go(3 hunks)proxy/proxymanager_test.go(1 hunks)ui/src/components/ConnectionStatus.tsx(2 hunks)ui/src/contexts/APIProvider.tsx(6 hunks)version/version.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-29T05:26:34.964Z
Learnt from: mostlygeek
Repo: mostlygeek/llama-swap PR: 371
File: proxy/process.go:0-0
Timestamp: 2025-10-29T05:26:34.964Z
Learning: In proxy/process.go, the loading message "llama-swap loading model: {name}" intentionally uses p.ID (Process.ID) rather than the realModelName from the request context. This is the correct design choice.
Applied to files:
proxy/proxymanager_api.go
📚 Learning: 2025-11-08T16:56:35.618Z
Learnt from: ryan-steed-usa
Repo: mostlygeek/llama-swap PR: 391
File: docker/build-container.sh:29-32
Timestamp: 2025-11-08T16:56:35.618Z
Learning: In the mostlygeek/llama-swap repository, llama.cpp container tags from ghcr.io/ggml-org/llama.cpp use build identifiers (e.g., `b6981`) rather than semantic versions. The tag format is `server-<something>-<build-id>` for cpu and `server-<arch>-<something>-<build-id>` for other architectures, where extracting the third field with `awk -F '-' '{print $3}'` correctly retrieves the build identifier.
Applied to files:
Makefile.goreleaser.yaml
📚 Learning: 2025-10-07T05:41:52.728Z
Learnt from: mostlygeek
Repo: mostlygeek/llama-swap PR: 337
File: proxy/config/config.go:268-291
Timestamp: 2025-10-07T05:41:52.728Z
Learning: In the mostlygeek/llama-swap repository's proxy/config/config.go, macro-in-macro substitution requires that referenced macros be declared earlier in the YAML file (declaration order matters). A macro can only reference macros that appear before it in the configuration. The YAML order is preserved by the UnmarshalYAML implementation for MacroList, and the LIFO substitution approach is intentional.
Applied to files:
llama-swap.go
🧬 Code graph analysis (4)
proxy/proxymanager_api.go (1)
version/version.go (1)
VersionJSON(13-17)
ui/src/components/ConnectionStatus.tsx (1)
ui/src/contexts/APIProvider.tsx (1)
useAPI(266-272)
proxy/proxymanager_test.go (2)
proxy/config/config.go (2)
AddDefaultGroupToConfig(406-448)Config(112-135)proxy/proxymanager.go (1)
New(50-140)
llama-swap.go (1)
version/version.go (3)
Version(10-10)Commit(11-11)Date(12-12)
🔇 Additional comments (13)
Makefile (1)
48-48: LGTM!The ldflags are correctly updated to reference the new version package paths across all build targets (macOS, Linux, Windows). The injection of version metadata is consistent and properly formatted.
Also applies to: 53-54, 59-59
proxy/proxymanager_api.go (3)
13-13: LGTM!The import of the version package is appropriate for the new endpoint.
32-32: LGTM!The new
/api/versionendpoint is properly registered in the API group.
233-240: LGTM!The handler correctly marshals
version.VersionJSONand returns it asapplication/json. Error handling is appropriate with a 500 status on marshaling failure..goreleaser.yaml (2)
19-20: LGTM!The ldflags correctly reference the new version package paths and use GoReleaser template variables (
{{.Version}},{{.Commit}},{{.CommitDate}}) to inject build-time metadata.
34-34: LGTM!Minor formatting adjustment for consistency.
llama-swap.go (2)
20-20: LGTM!The import of the version package is appropriate for the version display functionality.
35-35: LGTM!The version display now correctly uses the centralized version package variables.
ui/src/contexts/APIProvider.tsx (4)
52-56: LGTM!The
VersionInfointerface correctly matches the backendVersionStructstructure withversion,commit, anddatefields.
71-71: LGTM!The
versionInfostate is properly initialized tonull.
158-158: LGTM!Resetting
versionInfotonullon disconnect is appropriate to clear stale data.
232-243: LGTM!The
getVersionInfofunction correctly fetches from/api/versionand updates state. Silent error handling (early return) is acceptable for this non-critical feature.version/version.go (1)
13-17: No issues found—initialization order concern is correctly addressed by Go's linker behavior.The code is technically sound. Go's linker applies
-Xstring variable replacements at link time (before the program runs), so whenVersionJSONinitializes at package load time, theVersion,Commit, andDatevariables already contain the injected values, not the defaults. The explanation in the review comment is accurate.Verification confirms:
- Makefile ldflags correctly target package variables (e.g.,
-X github.com/mostlygeek/llama-swap/version.Commit=...)- .goreleaser.yaml ldflags configured properly with build metadata
apiGetVersion()endpoint inproxy/proxymanager_api.go:234marshalsVersionJSONto return version infoThe binary will correctly return injected version metadata through the version endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ui/src/components/ConnectionStatus.tsx (1)
35-35: Simplify the template literal.The template literal wrapping is unnecessary since
titleis already a string.Apply this diff:
- <div className="flex items-center" title={`${title}`}> + <div className="flex items-center" title={title}>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/src/components/ConnectionStatus.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ui/src/components/ConnectionStatus.tsx (1)
ui/src/contexts/APIProvider.tsx (1)
useAPI(266-272)
⏰ 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
🔇 Additional comments (4)
ui/src/components/ConnectionStatus.tsx (4)
2-2: LGTM!The
useEffectimport is correctly added to support the new side effect functionality.
5-5: LGTM!The destructuring correctly includes the new
versionInfoandgetVersionInfofrom the API context, aligning with the version tracking feature.
19-24: Past review comment addressed.The
useEffectdependency array now correctly includesgetVersionInfo, satisfying React's exhaustive-deps rule. The logic appropriately fetches version info when the connection is established.
26-32: LGTM!The
useMemocorrectly computes the multi-line title with proper dependencies. The fallback handling for nullversionInfois clean and the newline-separated format will display correctly in tooltips.
c2517c8 to
b70f79c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ui/src/contexts/APIProvider.tsx (1)
232-243: Consider adding fetch cancellation and error logging.The implementation is functional and follows React patterns. For better resilience, consider these optional improvements:
- Add an
AbortControllerto cancel in-flight fetches if called again or on unmount- Log errors to the console for debugging (currently silent)
- Validate the response shape at runtime before casting to
VersionInfoOptional refactor with AbortController
const getVersionInfo = useCallback(async () => { const controller = new AbortController(); try { const response = await fetch("/api/version", { signal: controller.signal }); if (!response.ok) { throw new Error(`HTTP error! status: ${response.status}`); } const data: VersionInfo = await response.json(); setVersionInfo(data); } catch (error) { if (error instanceof Error && error.name !== 'AbortError') { console.error('Failed to fetch version info:', error); } return; } }, []);Note: Full cancellation on unmount would require storing the controller in a ref and cleaning up in useEffect.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ui/src/components/ConnectionStatus.tsx(2 hunks)ui/src/contexts/APIProvider.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/src/components/ConnectionStatus.tsx
⏰ 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
🔇 Additional comments (5)
ui/src/contexts/APIProvider.tsx (5)
26-27: LGTM!The interface additions are well-typed and follow existing patterns. The nullable
VersionInfoappropriately handles cases where version data is unavailable.
52-56: LGTM!The
VersionInfotype definition is clean and appropriately models the version metadata from the/api/versionendpoint.
71-71: LGTM!The state initialization correctly starts with
nullto represent the absence of version info before it's fetched.
158-158: LGTM!Resetting
versionInfotonullon connection error maintains consistency between connection state and version availability.
257-260: LGTM! Dependency array is now complete.The new
versionInfoandgetVersionInfoare correctly included in both the context value and theuseMemodependency array. This addresses the previous review comment about missing dependencies.All values returned by the context are now present in the dependency array, ensuring proper memoization behavior.
This pull request adds comprehensive version tracking and display functionality to the application. These features can be particularly useful when managing multiple instances. Here's a summary of the key changes:
Main Features Added:
versionpackage that provides structured build metadata (version number, git commit hash, build date) with JSON serialization support/api/versionendpoint that returns version information as JSONKey Technical Changes:
Development Workflow:
This lays the foundation for better release management and provides users with easy access to build information through both API calls and UI tooltips.
The UI tooltip:

CLI:
API:
Summary by CodeRabbit
New Features
Tests