Skip to content

feat(group): add unified Group type deriving toolset and promptset views#3488

Open
twishabansal wants to merge 4 commits into
mainfrom
feat/groups-pr1-group-package
Open

feat(group): add unified Group type deriving toolset and promptset views#3488
twishabansal wants to merge 4 commits into
mainfrom
feat/groups-pr1-group-package

Conversation

@twishabansal

@twishabansal twishabansal commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Description

First slice of the toolsets→groups evolution. Introduces internal/group as the unified source of truth for a named collection of tools and prompts, without changing any existing behavior yet.

  • GroupConfig / Group types; Initialize reuses tools.ToolsetConfig.Initialize and prompts.PromptsetConfig.Initialize so validation and manifest-building stay consistent.
  • ToToolset() / ToPromptset() project the legacy views keyed by the group name.

Wiring (config parsing, ResourceManager, server init, validation rules) and the prompt-scoping fix follow in subsequent PRs.

Introduce internal/group as the source of truth for a named collection of
tools and prompts. Group.Initialize reuses the toolset/promptset Initialize
logic; ToToolset/ToPromptset project the legacy views keyed by the group name.
No existing behavior changes yet.
@twishabansal twishabansal requested a review from a team as a code owner June 22, 2026 10:18

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new group package to manage a named collection of both tools and prompts, acting as a single source of truth while providing projection methods (ToToolset and ToPromptset) for backward compatibility. The review feedback highlights an efficiency issue where manual reconstruction of tools.Toolset and prompts.Promptset in these projection methods discards the unexported toolNameSet field, causing ContainsTool to fall back to an O(N) linear scan. The reviewer suggests storing the fully initialized Toolset and Promptset structs directly within the Group struct to preserve O(1) lookups, simplify the code, and update the corresponding unit tests.

Comment thread internal/group/group.go
Comment thread internal/group/group_test.go Outdated
@twishabansal twishabansal marked this pull request as draft June 22, 2026 11:01
Embed the initialized Toolset and Promptset in Group instead of
flattening their fields, so ToToolset/ToPromptset return the views
with their lookup sets intact rather than rebuilding them per call.
@twishabansal

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new group package under internal/group to support grouping tools and prompts together. It defines GroupConfig for configuration and Group as the initialized source of truth, along with methods to project them into legacy Toolset and Promptset views. Comprehensive unit tests are also added to validate initialization and projections. There are no review comments, and I have no feedback to provide.

@twishabansal twishabansal marked this pull request as ready for review June 29, 2026 07:41
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.

3 participants