Add bindings-backed Go SDK implementation#260
Conversation
anirudh-makuluri
requested review from
CoderOMaster,
HarshaNalluru,
r4ghu and
yatharthk2
as code owners
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@CoderOMaster can you do a deep review? |
There was a problem hiding this comment.
Pull request overview
This PR introduces a bindings-backed Go SDK for Moss by adding a low-level CGO wrapper over libmoss and wiring the public Go SDK to use that bindings layer for index management, document mutation, local load/unload, and local query.
Changes:
- Added
sdks/go/bindingsmodule (CGO-backed, with-tags libmossstub fallback). - Added
sdks/go/sdkmodule implementing a typed Go client that uses the bindings for manage + local runtime operations. - Added examples, unit tests, and env-gated integration tests for the new Go SDK.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sdks/go/README.md | Top-level overview of the two-layer Go SDK/bindings structure and requirements. |
| sdks/go/sdk/go.mod | Defines the Go SDK module and replaces the local bindings module. |
| sdks/go/sdk/README.md | Documents installation, quick start, examples, and integration testing workflow. |
| sdks/go/sdk/moss/client.go | Implements the public Client with lazy-initialized manage + index runtimes. |
| sdks/go/sdk/moss/options.go | Compatibility options retained from earlier scaffolding. |
| sdks/go/sdk/moss/models.go | Defines public SDK models/options/results. |
| sdks/go/sdk/moss/errors.go | Defines public SDK error values and compatibility HTTPError. |
| sdks/go/sdk/moss/conversion.go | Converts between bindings-layer types and public SDK types. |
| sdks/go/sdk/moss/read.go | Adds bindings-backed index/document read operations. |
| sdks/go/sdk/moss/mutation.go | Adds bindings-backed mutations plus job polling support. |
| sdks/go/sdk/moss/query.go | Adds local query API and filter serialization support. |
| sdks/go/sdk/moss/local.go | Adds local load/unload/refresh operations via bindings. |
| sdks/go/sdk/moss/client_test.go | Unit tests for client behavior against fake runtimes. |
| sdks/go/sdk/moss/mutation_test.go | Unit tests for mutation behaviors and polling logic. |
| sdks/go/sdk/moss/integration_test.go | Env-gated end-to-end integration test using real bindings when available. |
| sdks/go/sdk/examples/basic/main.go | Runnable example demonstrating create/load/query/delete. |
| sdks/go/sdk/examples/custom-embeddings/main.go | Runnable example demonstrating caller-provided embeddings workflow. |
| sdks/go/bindings/go.mod | Defines the bindings module. |
| sdks/go/bindings/README.md | Documents the -tags libmoss workflow and required env flags. |
| sdks/go/bindings/errors.go | Defines ErrBindingsUnavailable for non-libmoss builds. |
| sdks/go/bindings/models.go | Defines bindings-layer data structures used by the CGO wrapper. |
| sdks/go/bindings/stub.go | Provides the non-libmoss stub implementation returning ErrBindingsUnavailable. |
| sdks/go/bindings/libmoss.go | Implements the CGO wrapper around libmoss manage + local runtime APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| timeTaken := value.TimeTakenMs | ||
| return SearchResult{ | ||
| Docs: docs, | ||
| Query: value.Query, | ||
| IndexName: value.IndexName, | ||
| TimeTakenMs: &timeTaken, | ||
| } |
| func (m *IndexManager) LoadQueryModel(indexName string) error { | ||
| return nil | ||
| } |
| func (c *ManageClient) CreateIndex(name string, docs []DocumentInfo, modelID string) (MutationResult, error) { | ||
| input, err := newCDocumentInput(docs) | ||
| if err != nil { | ||
| return MutationResult{}, err | ||
| } | ||
| defer input.free() |
| func (m *IndexManager) query(indexName, query string, queryEmbedding []float32, topK int, alpha float32, filterJSON *string) (SearchResult, error) { | ||
| cName := C.CString(indexName) | ||
| defer C.free(unsafe.Pointer(cName)) | ||
| cQuery := C.CString(query) | ||
| defer C.free(unsafe.Pointer(cQuery)) | ||
|
|
| From this repository, use the module at: | ||
|
|
||
| ```go | ||
| github.com/usemoss/moss/sdks/go/sdk/moss | ||
| ``` |
| func (m *IndexManager) LoadQueryModel(indexName string) error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🚩 LoadQueryModel is a no-op in the bindings layer
LoadQueryModel at sdks/go/bindings/libmoss.go:290-292 always returns nil. The SDK calls it after loading non-custom indexes (sdks/go/sdk/moss/local.go:39-42). This means for standard model indexes (e.g., moss-minilm), no explicit query model loading occurs. This is presumably because the C SDK's moss_client_load_index handles model loading internally, but it's worth confirming — if the C API does require a separate model-load step, text queries on non-custom indexes would silently fail or produce incorrect results.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if err != nil { | ||
| return SearchResult{}, err | ||
| } | ||
| if !manager.HasIndex(indexName) { |
There was a problem hiding this comment.
Add cloud query fallback for unloaded indexes
Go returns ErrIndexNotLoaded when Query is called before LoadIndex, at query.go (line 23). Python and JS both fall back to cloud query instead.
| func (m *IndexManager) withClient(call func(*C.MossClient) C.MossResult) error { | ||
| if m == nil { | ||
| return ErrClientClosed | ||
| } | ||
| m.mu.RLock() | ||
| defer m.mu.RUnlock() | ||
| if m.ptr == nil { | ||
| return ErrClientClosed | ||
| } | ||
| return withErrorThread(func() C.MossResult { | ||
| return call(m.ptr) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🚩 IndexManager.withClient uses RLock — concurrent RefreshIndex + Query depends on libmoss thread safety
IndexManager.withClient at sdks/go/bindings/libmoss.go:465 uses m.mu.RLock(), allowing RefreshIndex (line 338) and query (line 373) to run concurrently since both call withClient. This is correct from the Go SDK's perspective — the RWMutex protects m.ptr from being freed during use — but it means the C library must handle concurrent moss_client_refresh_index + moss_client_query calls on the same MossClient pointer safely. This matches the pattern where LoadIndex/UnloadIndex take exclusive Lock() (they mutate m.loaded), while read-path operations share RLock(). Worth confirming that libmoss supports this concurrency model.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Just a heads up: this is not the final user-ready distribution story yet. Right now the Go SDK works, but native/local runtime usage requires the go run -tags libmoss ./examples/basicTo make this fully user-ready, we still need to decide on the distribution path: cloud-first default with optional native acceleration, bundled native libs, a setup helper, separate cloud/native packages, etc. Happy to discuss the best direction whenever you guys are free. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@anirudh-makuluri i think using libmoss is fine for now, rest you can update and address all the review comments |
|
@CoderOMaster yes, agreed. We'll keep using My earlier message was more about how A few possible options:
I'd lean toward option 1. With the current structure, go get github.com/usemoss/moss/sdks/go/sdk |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@anirudh-makuluri okay lets go with option 1. also can you also address copliot comments? |
| if err := ctx.Err(); err != nil { | ||
| return SearchResult{}, err | ||
| } | ||
| if err := c.validateQueryRequest(indexName); err != nil { | ||
| return SearchResult{}, err | ||
| } |
| func (c *Client) queryCloud(ctx context.Context, indexName, query string, options *QueryOptions) (SearchResult, error) { | ||
| topK := defaultTopK | ||
| var embedding []float32 | ||
| if options != nil { |
| if options.Filter != nil { | ||
| bytes, err := json.Marshal(options.Filter) | ||
| if err != nil { | ||
| return SearchResult{}, err | ||
| } | ||
| value := string(bytes) | ||
| filterJSON = &value | ||
| } |
| if err := ctx.Err(); err != nil { | ||
| return RefreshResult{}, err | ||
| } | ||
| if strings.TrimSpace(indexName) == "" { | ||
| return RefreshResult{}, ErrEmptyIndexName | ||
| } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Sounds good, thanks. The review comments should be addressed now. For option 1, I think it’s better to handle the prebuilt |
Pull Request Checklist
Please ensure that your PR meets the following requirements:
Description
This PR adds the first bindings-backed Go SDK implementation for Moss.
What’s included:
sdks/go/bindingsas a low-level CGO wrapper overlibmosssdks/go/sdkto use the bindings layer for index management, document operations, local load/unload, and local queryScope of this PR:
libmossC SDK plus-tags libmossValidation:
cd sdks/go/bindings && go test ./...cd sdks/go/sdk && go test ./...Fixes #227
Type of Change