feat(preference): add InputHintOption for structured input options #1378
Conversation
Add InputHintOption struct with name and title fields for user-friendly option display. The InputOptions field in Trait allows specifying both machine-readable names and display titles. - Add InputHintOption struct - Add InputOptions field to Trait (takes precedence over InputHints) - Add NewSelectValidatorFromOptions function Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds InputHintOption and InputOptions to Trait, Preference.ValueTitle, Trait.GetValueTitle, and NewSelectValidatorFromOptions; GetValidator now prefers InputOptions for select/combobox; service Create and LoadUserPreferences populate Preference.ValueTitle; API/schema and validation updated to include InputHintOption and value_title. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@core/preference/preference.go`:
- Around line 108-112: The TraitInputMultiselect branch incorrectly routes to
NewSelectValidatorFromOptions (used by TraitInputSelect/Combobox) causing
comma-separated values like "1,3" to fail SelectValidator.Validate; fix by
changing handling of TraitInputMultiselect in the switch so it does not use
SelectValidator: either return NewTextValidator() for TraitInputMultiselect
(simple fallback) or implement and return a new NewMultiselectValidator that
splits on commas, trims each token and validates each against the options
(mirroring SelectValidator logic for individual tokens). Update the branch that
currently checks len(t.InputOptions) > 0 (and the case label
TraitInputMultiselect) to use the chosen approach and ensure tests referencing
multiselect behavior pass.
In `@core/preference/validator.go`:
- Around line 59-70: NewSelectValidatorFromOptions currently appends raw
opt.Name values which can contain surrounding whitespace and cause mismatches;
update the function (NewSelectValidatorFromOptions) to trim whitespace (e.g.,
strings.TrimSpace) from each InputHintOption.Name before adding to allowedValues
and skip entries that are empty after trimming so the
SelectValidator.allowedValues list is normalized and matches NewSelectValidator
behavior.
core/preference/preference.go
Outdated
| case TraitInputSelect, TraitInputCombobox, TraitInputMultiselect: | ||
| // Use InputOptions if available, otherwise fall back to InputHints | ||
| if len(t.InputOptions) > 0 { | ||
| return NewSelectValidatorFromOptions(t.InputOptions) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find how multiselect values are serialized and validated
rg -n -C3 '\.GetValidator\(' --type goRepository: raystack/frontier
Length of output: 1063
🏁 Script executed:
# Search for TraitInputMultiselect usage and multiselect patterns
rg -n -C3 'TraitInputMultiselect|multiselect' --type goRepository: raystack/frontier
Length of output: 3953
🏁 Script executed:
# Search for value splitting/processing patterns
rg -n -C3 'strings\.Split|json\.Unmarshal' --type goRepository: raystack/frontier
Length of output: 50373
🏁 Script executed:
# Look at the SelectValidator implementation
rg -n -C10 'type SelectValidator|func.*SelectValidator' --type goRepository: raystack/frontier
Length of output: 4323
🏁 Script executed:
# Check the preference.go file around the lines in question
cat -n core/preference/preference.go | head -130 | tail -40Repository: raystack/frontier
Length of output: 1495
Multiselect is incorrectly routed through SelectValidator and will reject comma-separated values.
The code at lines 108–113 treats TraitInputMultiselect the same as TraitInputSelect and TraitInputCombobox, routing all three to SelectValidator. However, SelectValidator.Validate() performs exact-match validation against individual options using slices.Contains(). When a multiselect preference value like "1,3" is passed directly to this validator against options ["1", "2", "3"], the validation fails because "1,3" is not an exact match.
The test at line 102 of validator_test.go confirms this issue—its name explicitly states "multiselect falls back to text" and expects the comma-separated value "1,3" to validate as true against hints "1,2,3". Under the current SelectValidator implementation, this test would fail.
To fix this, either:
- Revert
TraitInputMultiselectto useNewTextValidator()(fall back to text validation), or - Create a dedicated multiselect validator that splits comma-separated values, trims each, and validates each one against the allowed options.
🤖 Prompt for AI Agents
In `@core/preference/preference.go` around lines 108 - 112, The
TraitInputMultiselect branch incorrectly routes to NewSelectValidatorFromOptions
(used by TraitInputSelect/Combobox) causing comma-separated values like "1,3" to
fail SelectValidator.Validate; fix by changing handling of TraitInputMultiselect
in the switch so it does not use SelectValidator: either return
NewTextValidator() for TraitInputMultiselect (simple fallback) or implement and
return a new NewMultiselectValidator that splits on commas, trims each token and
validates each against the options (mirroring SelectValidator logic for
individual tokens). Update the branch that currently checks len(t.InputOptions)
> 0 (and the case label TraitInputMultiselect) to use the chosen approach and
ensure tests referencing multiselect behavior pass.
| // NewSelectValidatorFromOptions creates a SelectValidator from InputHintOptions | ||
| // It extracts the Name field from each option as the allowed value | ||
| func NewSelectValidatorFromOptions(options []InputHintOption) *SelectValidator { | ||
| var allowed []string | ||
| for _, opt := range options { | ||
| if opt.Name != "" { | ||
| allowed = append(allowed, opt.Name) | ||
| } | ||
| } | ||
| return &SelectValidator{ | ||
| allowedValues: allowed, | ||
| } |
There was a problem hiding this comment.
Trim option names to avoid mismatches.
NewSelectValidator trims whitespace but NewSelectValidatorFromOptions doesn’t, so a config like " sq_km " would never validate against "sq_km". Consider normalizing names for consistency.
Proposed fix
func NewSelectValidatorFromOptions(options []InputHintOption) *SelectValidator {
var allowed []string
for _, opt := range options {
- if opt.Name != "" {
- allowed = append(allowed, opt.Name)
+ if trimmed := strings.TrimSpace(opt.Name); trimmed != "" {
+ allowed = append(allowed, trimmed)
}
}
return &SelectValidator{
allowedValues: allowed,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // NewSelectValidatorFromOptions creates a SelectValidator from InputHintOptions | |
| // It extracts the Name field from each option as the allowed value | |
| func NewSelectValidatorFromOptions(options []InputHintOption) *SelectValidator { | |
| var allowed []string | |
| for _, opt := range options { | |
| if opt.Name != "" { | |
| allowed = append(allowed, opt.Name) | |
| } | |
| } | |
| return &SelectValidator{ | |
| allowedValues: allowed, | |
| } | |
| // NewSelectValidatorFromOptions creates a SelectValidator from InputHintOptions | |
| // It extracts the Name field from each option as the allowed value | |
| func NewSelectValidatorFromOptions(options []InputHintOption) *SelectValidator { | |
| var allowed []string | |
| for _, opt := range options { | |
| if trimmed := strings.TrimSpace(opt.Name); trimmed != "" { | |
| allowed = append(allowed, trimmed) | |
| } | |
| } | |
| return &SelectValidator{ | |
| allowedValues: allowed, | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@core/preference/validator.go` around lines 59 - 70,
NewSelectValidatorFromOptions currently appends raw opt.Name values which can
contain surrounding whitespace and cause mismatches; update the function
(NewSelectValidatorFromOptions) to trim whitespace (e.g., strings.TrimSpace)
from each InputHintOption.Name before adding to allowedValues and skip entries
that are empty after trimming so the SelectValidator.allowedValues list is
normalized and matches NewSelectValidator behavior.
- Add ValueTitle field to Preference struct - Add GetValueTitle method to Trait for looking up title from InputOptions - Populate ValueTitle in LoadUserPreferences from trait's InputOptions - Only applies to select/combobox/multiselect input types Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@core/preference/preference.go`:
- Around line 132-140: The GetValueTitle method on Trait currently only matches
opt.Name exactly so comma-separated multiselect inputs like "opt1,opt2" return
empty; update Trait.GetValueTitle to detect if value contains commas, split by
"," (use strings.Split and strings.TrimSpace), look up each token against
t.InputOptions matching opt.Name to collect opt.Title values, then return the
joined titles (e.g., strings.Join with ", " or similar); ensure you add import
"strings" if not present and preserve existing exact-match behavior when value
has no commas.
| // GetValueTitle returns the human-readable title for a value from InputOptions | ||
| func (t Trait) GetValueTitle(value string) string { | ||
| for _, opt := range t.InputOptions { | ||
| if opt.Name == value { | ||
| return opt.Title | ||
| } | ||
| } | ||
| return "" | ||
| } |
There was a problem hiding this comment.
GetValueTitle won't resolve titles for multiselect values.
This method performs exact matching against opt.Name, so comma-separated multiselect values like "opt1,opt2" will always return "". This is related to the validation issue flagged earlier.
If multiselect support is intended, consider splitting the value and returning joined titles:
💡 Possible approach for multiselect title resolution
// GetValueTitle returns the human-readable title for a value from InputOptions
func (t Trait) GetValueTitle(value string) string {
+ // Handle comma-separated multiselect values
+ if strings.Contains(value, ",") {
+ var titles []string
+ for _, v := range strings.Split(value, ",") {
+ v = strings.TrimSpace(v)
+ for _, opt := range t.InputOptions {
+ if opt.Name == v {
+ titles = append(titles, opt.Title)
+ break
+ }
+ }
+ }
+ return strings.Join(titles, ", ")
+ }
for _, opt := range t.InputOptions {
if opt.Name == value {
return opt.Title
}
}
return ""
}Note: This requires import "strings" at the top of the file.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // GetValueTitle returns the human-readable title for a value from InputOptions | |
| func (t Trait) GetValueTitle(value string) string { | |
| for _, opt := range t.InputOptions { | |
| if opt.Name == value { | |
| return opt.Title | |
| } | |
| } | |
| return "" | |
| } | |
| // GetValueTitle returns the human-readable title for a value from InputOptions | |
| func (t Trait) GetValueTitle(value string) string { | |
| // Handle comma-separated multiselect values | |
| if strings.Contains(value, ",") { | |
| var titles []string | |
| for _, v := range strings.Split(value, ",") { | |
| v = strings.TrimSpace(v) | |
| for _, opt := range t.InputOptions { | |
| if opt.Name == v { | |
| titles = append(titles, opt.Title) | |
| break | |
| } | |
| } | |
| } | |
| return strings.Join(titles, ", ") | |
| } | |
| for _, opt := range t.InputOptions { | |
| if opt.Name == value { | |
| return opt.Title | |
| } | |
| } | |
| return "" | |
| } |
🤖 Prompt for AI Agents
In `@core/preference/preference.go` around lines 132 - 140, The GetValueTitle
method on Trait currently only matches opt.Name exactly so comma-separated
multiselect inputs like "opt1,opt2" return empty; update Trait.GetValueTitle to
detect if value contains commas, split by "," (use strings.Split and
strings.TrimSpace), look up each token against t.InputOptions matching opt.Name
to collect opt.Title values, then return the joined titles (e.g., strings.Join
with ", " or similar); ensure you add import "strings" if not present and
preserve existing exact-match behavior when value has no commas.
- Update PROTON_COMMIT to include InputHintOption and value_title changes - Regenerate proto stubs - Fix multiselect to use text validator (not select validator) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pull Request Test Coverage Report for Build 21899742500Details
💛 - Coveralls |
- Add ValueTitle to Preference proto response for human-readable display - Add InputOptions to PreferenceTrait proto response for select/combobox Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The Create method now populates ValueTitle from the matched trait's InputOptions before returning the created preference. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/preference/service.go (1)
136-151:⚠️ Potential issue | 🟡 MinorValueTitle is inconsistently exposed across API implementations.
While
CreateandLoadUserPreferencescorrectly populateValueTitle, the service methodsGetandListdo not enrich the response. More critically, the two API implementations handle this differently:
internal/api/v1beta1connect/preferences.go: includesValueTitlein the transformed responseinternal/api/v1beta1/preferences.go: omitsValueTitlefrom the response entirelyThis creates inconsistent data shapes depending on which API endpoint (Connect vs REST) clients use. Align the implementations by either:
- Populating
ValueTitleinGetandListservice methods (consistent withCreate)- Or explicitly document this field only applies to
Createresponses- Or update
v1beta1to includeValueTitlelikev1beta1connectdoes
2 participants
Summary
Add
InputHintOptionstruct andInputOptionsfield to support user-friendly option display in preference traits.Changes
core/preference/preference.go
core/preference/validator.go
NewSelectValidatorFromOptions()to validate against InputOptions namesUsage
Dependencies
Next Steps
Once proton PR is merged:
🤖 Generated with Claude Code