Skip to content

Improve go/rpc API#905

Open
qmuntal wants to merge 3 commits intogithub:mainfrom
qmuntal:dev/qmuntal/imprpc
Open

Improve go/rpc API#905
qmuntal wants to merge 3 commits intogithub:mainfrom
qmuntal:dev/qmuntal/imprpc

Conversation

@qmuntal
Copy link
Contributor

@qmuntal qmuntal commented Mar 23, 2026

  • Replace interface{} -> any.
  • Remove Rpc from rpc struct names. The package is already named rpc, there is no need to duplicate the "context" in the struct name. E.g. ModelRpcApi -> ModelApi.
  • NewSessionRpc and NewServerRpc now allocate less.

While here, fix TestMultiClient.

@qmuntal qmuntal requested a review from a team as a code owner March 23, 2026 10:48
Copilot AI review requested due to automatic review settings March 23, 2026 10:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Go RPC surface and generator to modernize types (any), simplify API names, and reduce allocations in the generated RPC wrappers by sharing a common service struct.

Changes:

  • Replace generated interface{} usages with any in Go RPC types.
  • Rename generated RPC API structs to drop the redundant Rpc/RpcApi suffix (e.g., ModelRpcApiModelApi).
  • Reduce per-wrapper allocations by sharing a single underlying service struct across API groups; adjust JSON-RPC client to treat nil params as {}.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
scripts/codegen/go.ts Updates Go RPC codegen: replaces interface{}any, renames API types, and emits shared-service wrapper structure.
go/rpc/generated_rpc.go Regenerated Go RPC types/wrappers reflecting any, renamed APIs, and shared-service construction.
go/internal/jsonrpc2/jsonrpc2.go Changes request/notify param encoding so nil params serialize as {} to support no-params calls without allocating empty maps.
Comments suppressed due to low confidence (1)

go/internal/jsonrpc2/jsonrpc2.go:226

  • This change introduces special handling for params == nil (sending {}), which is important behavior to lock down because it affects the wire protocol for no-params calls. There are already unit tests for this package; please add coverage that asserts Request/Notify serialize nil params as an empty object (and not null) so future refactors don’t regress this.
	var paramsData json.RawMessage
	if params == nil {
		paramsData = json.RawMessage("{}")
	} else {
		var err error
		paramsData, err = json.Marshal(params)
		if err != nil {
			return nil, fmt.Errorf("failed to marshal params: %w", err)
		}
	}

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