-
Notifications
You must be signed in to change notification settings - Fork 24
feat(frontier): add InputHintOption for structured preference options #442
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?
Conversation
Add InputHintOption message with name and title fields for user-friendly option display in preference traits. The input_options field in PreferenceTrait allows specifying both machine-readable names and display titles. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded a new protobuf message Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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
🤖 Fix all issues with AI agents
In `@raystack/frontier/v1beta1/models.proto`:
- Around line 521-527: InputHintOption allows empty/invalid identifiers; add
validation rules to ensure name is non-empty (and matches a stable
machine-readable pattern) and title is non-empty. Update the InputHintOption
message to include validation annotations (e.g., protoc-gen-validate /
validate.rules) on fields name and title: require name to be non-empty and
enforce a conservative pattern such as lowercase alphanumerics/underscores (or
your project's chosen identifier regex) and require title to be non-empty;
reference the message name InputHintOption and the fields name and title when
applying these annotations so generated code/validators will reject
empty/invalid values.
- Around line 557-560: Clarify the precedence rule for InputHintOption vs
input_hints: update the comment for the repeated field input_options to state
explicitly that input_options only takes precedence when it is present and
non-empty; if clients send input_options: [] (an empty list), the service should
fall back to using input_hints to populate options. Mention that implementations
may validate/require non-empty input_options if they want to enforce Option A,
and reference the fields InputHintOption, input_options, and input_hints in the
comment so the behavior is unambiguous.
| // InputHintOption represents a selectable option with a machine-readable name and user-friendly title | ||
| message InputHintOption { | ||
| // Machine-readable identifier (e.g., "sq_km", "megagram") | ||
| string name = 1; | ||
| // User-friendly display title (e.g., "Square Kilometers", "Megagram (Mg)") | ||
| string title = 2; | ||
| } |
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.
Add validation for InputHintOption fields to avoid empty/invalid identifiers.
Empty name values undermine the “machine‑readable identifier” contract and can collide in storage/validation. Consider enforcing non‑empty (and optionally a stable pattern) for name and title.
🛡️ Suggested validation rules
message InputHintOption {
// Machine-readable identifier (e.g., "sq_km", "megagram")
- string name = 1;
+ string name = 1 [(validate.rules).string = {min_len: 1, pattern: "^[A-Za-z0-9-_]+$"}];
// User-friendly display title (e.g., "Square Kilometers", "Megagram (Mg)")
- string title = 2;
+ string title = 2 [(validate.rules).string = {min_len: 1}];
}🤖 Prompt for AI Agents
In `@raystack/frontier/v1beta1/models.proto` around lines 521 - 527,
InputHintOption allows empty/invalid identifiers; add validation rules to ensure
name is non-empty (and matches a stable machine-readable pattern) and title is
non-empty. Update the InputHintOption message to include validation annotations
(e.g., protoc-gen-validate / validate.rules) on fields name and title: require
name to be non-empty and enforce a conservative pattern such as lowercase
alphanumerics/underscores (or your project's chosen identifier regex) and
require title to be non-empty; reference the message name InputHintOption and
the fields name and title when applying these annotations so generated
code/validators will reject empty/invalid values.
| InputType input_type = 27; | ||
| // Structured input options with name and title for select/combobox/multiselect inputs | ||
| // Takes precedence over input_hints when both are provided | ||
| repeated InputHintOption input_options = 28; |
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.
Make the precedence rule unambiguous when input_options is empty.
If clients send input_options: [], the current wording implies input_hints is ignored, which could yield no options. Consider enforcing non‑empty lists or explicitly stating fallback behavior.
🧭 Option A: enforce non-empty lists
- // Takes precedence over input_hints when both are provided
- repeated InputHintOption input_options = 28;
+ // Takes precedence over input_hints when non-empty
+ repeated InputHintOption input_options = 28 [
+ (validate.rules).repeated = {min_items: 1}
+ ];🧭 Option B: clarify fallback in comment (if empty lists are allowed)
- // Takes precedence over input_hints when both are provided
+ // Takes precedence over input_hints when non-empty; otherwise input_hints applies
repeated InputHintOption input_options = 28;📝 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.
| InputType input_type = 27; | |
| // Structured input options with name and title for select/combobox/multiselect inputs | |
| // Takes precedence over input_hints when both are provided | |
| repeated InputHintOption input_options = 28; | |
| InputType input_type = 27; | |
| // Structured input options with name and title for select/combobox/multiselect inputs | |
| // Takes precedence over input_hints when non-empty | |
| repeated InputHintOption input_options = 28 [ | |
| (validate.rules).repeated = {min_items: 1} | |
| ]; |
| InputType input_type = 27; | |
| // Structured input options with name and title for select/combobox/multiselect inputs | |
| // Takes precedence over input_hints when both are provided | |
| repeated InputHintOption input_options = 28; | |
| InputType input_type = 27; | |
| // Structured input options with name and title for select/combobox/multiselect inputs | |
| // Takes precedence over input_hints when non-empty; otherwise input_hints applies | |
| repeated InputHintOption input_options = 28; |
🤖 Prompt for AI Agents
In `@raystack/frontier/v1beta1/models.proto` around lines 557 - 560, Clarify the
precedence rule for InputHintOption vs input_hints: update the comment for the
repeated field input_options to state explicitly that input_options only takes
precedence when it is present and non-empty; if clients send input_options: []
(an empty list), the service should fall back to using input_hints to populate
options. Mention that implementations may validate/require non-empty
input_options if they want to enforce Option A, and reference the fields
InputHintOption, input_options, and input_hints in the comment so the behavior
is unambiguous.
Add value_title field to show human-readable display title alongside the machine-readable value in preference responses. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Add
InputHintOptionmessage andinput_optionsfield toPreferenceTraitfor user-friendly option display.Changes
Usage
When
input_optionsis provided, it takes precedence overinput_hints. This allows:Example:
Summary by CodeRabbit