feat: Add Suggestions tab to the agent rule creation flow#95875
feat: Add Suggestions tab to the agent rule creation flow#95875samranahm wants to merge 28 commits into
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@mananjadhav Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
Pull request overview
Adds a “Suggestions” tab to the workspace agent-rule creation flow, allowing admins to pick a backend-provided suggested rule, prefill the prompt, edit it, and then save via the existing addPolicyAgentRule path.
Changes:
- Introduces a tabbed add-agent-rule page (Suggestions + Write/Edit), with selection → draft prefill → save.
- Adds Onyx plumbing (key, type, hook) for backend-served suggested rules plus icon-derivation utility.
- Adds translations and unit/UI tests for the suggestions experience.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/getSuggestedAgentRuleIconTest.ts | Unit tests for suggestion → icon mapping helper. |
| tests/ui/AddAgentRuleSuggestionsTabTest.tsx | UI tests covering rendering, selection, filtering, and empty state. |
| src/types/onyx/SuggestedAgentRule.ts | Defines SuggestedAgentRule Onyx type shape {id,title,prompt}. |
| src/types/onyx/index.ts | Exposes SuggestedAgentRule from the central Onyx types barrel. |
| src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx | New “Write/Edit” tab owning the prompt form + submit. |
| src/pages/workspace/rules/AgentRules/AddAgentRuleSuggestionsTab.tsx | New “Suggestions” tab UI: search, select card, concierge/help, next. |
| src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx | Converts the add rule page to a tabbed flow + draft prefill wiring. |
| src/ONYXKEYS.ts | Adds SUGGESTED_AGENT_RULES Onyx key + type mapping. |
| src/libs/PolicyRulesUtils.ts | Adds getSuggestedAgentRuleIcon() and icon rule constants. |
| src/languages/zh-hans.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/pt-BR.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/pl.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/nl.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/ja.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/it.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/fr.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/es.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/en.ts | Adds/updates copy for the new tabbed flow. |
| src/languages/de.ts | Adds/updates copy for the new tabbed flow. |
| src/hooks/useSuggestedAgentRules.ts | Hook to read suggested rules + loading state from Onyx. |
| src/CONST/index.ts | Adds agent-rule tab constants + Sentry label for suggested-rule cards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a845696ee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
f2a57f0 to
a233648
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb4cd9b0bb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
mananjadhav
left a comment
mananjadhav
left a comment
There was a problem hiding this comment.
@samranahm I've reviewed the PR thoroughly. I've marked some important comments that should be addressed for sure.
| */ | ||
| function getAgentRuleSuggestions(policyID: string | undefined) { | ||
| if (!policyID) { | ||
| Log.warn('Invalid params for getAgentRuleSuggestions', {policyID}); |
There was a problem hiding this comment.
nab: Do we need to clear the key on this invalid case?
There was a problem hiding this comment.
Nope, we only set IS_LOADING_AGENT_RULE_SUGGESTIONS after the policyID check, so the invalid case never turns loading spinner on. So nothing to clear there.
| { | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: ONYXKEYS.IS_LOADING_AGENT_RULE_SUGGESTIONS, | ||
| value: true, |
There was a problem hiding this comment.
important: Will this cause any infinite spinner when the user goes offline?
There was a problem hiding this comment.
IS_LOADING_AGENT_RULE_SUGGESTIONS flag is cleared via successData/failureData when the request completes, but I think we shouldn’t fire the fetch while offline in the first place. So we can follow the same pattern as PreviewMatchesPage skip getAgentRuleSuggestions() when isOffline. wyt
There was a problem hiding this comment.
Yes I think that would be better. @yuwenmemon wdyt?
There was a problem hiding this comment.
@yuwenmemon We have two options here
-
Always use
FullPageOfflineBlockingView(“You appear to be offline.” / “This feature requires an active internet connection.”), even if suggestions are already cached in Onyx, same behaviour asPreviewMatchesPage. -
Only show the offline empty state when there’s no cached data and we need to call
getAgentRuleSuggestions. If suggestions are already loaded, keep the list usable offline. (our current approach) Here's how it looks like
Which one do you prefer?
|
UI Testing:
web-list-scroll-back.mov
web-button-save.mov
web-no-filter.mov
web-offline-to-online.mov |
|
@samranahm you have some tests failing |
|
Looking into this. |
|
@yuwenmemon @mananjadhav PR ready for review. |
|
🚧 yuwenmemon has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Reviewing this now. |
|
Yup, agree with Danny's suggestion to use the super simple text under the input for no search results. Thanks for moving the Concierge icon up to the header as well. |
|
@samranahm Is this handled? and can we please resolve the conflicts? |
|
@mananjadhav Yes it has been resolved, resolving merge conflicts. |
|
Adding offline test to match the current offline behaviour. |
|
@mananjadhav PR ready for review, please take a look. |
@mananjadhav We actually need to fix this because even if we click the next button and switch back to Suggested tab via tab selector we lost the selected item anyway, fixing it asap. |
… to preserve selection and scroll state during tab switches.
|
@mananjadhav All yours. |
|
@samranahm I think we have 2 bugs. One of them I am able to reproducible every time and second unreliably but I think it exists.
web-discard-modal.mov |
| if (!isOffline) { | ||
| getAgentRuleSuggestions(policyID); | ||
| } | ||
| return () => clearDraftValues(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM); |
There was a problem hiding this comment.
Yes, cleanup is tied to isOffline, so network connectivity flips clears the draft mid-edit. Let me impliment draft cleanup on unmount only.
There was a problem hiding this comment.
I can reliably produce this bug,
- Select a suggestion from list and press Next
- On edit tab flip the network state (turn on/off "Force offline")
- Switch to suggested tab via tab selector
- Switch back to Edit tab > Draft gone
Fixed this
| }; | ||
|
|
||
| const leavePage = () => { | ||
| clearDraftValues(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM); |
There was a problem hiding this comment.
important: Raised a bug here. Discard shows twice.
There was a problem hiding this comment.
Agree, the manual leave modal conflict with useDiscardChangesConfirmation because clearDraftValues is async and goBack() ran immediately. Back now just calls Navigation.goBack() so the hook owns a single discard prompt.
| const handleKeyPress = (e: TextInputKeyPressEvent | KeyboardEvent) => { | ||
| if (!('key' in e)) { | ||
| useEffect(() => { | ||
| Tab.setSelectedTab(CONST.TAB.AGENT_RULE_TAB_TYPE, CONST.TAB.AGENT_RULE.SUGGESTIONS); |
There was a problem hiding this comment.
question: activeTab is local state now, and nothing reads SELECTED_TAB + AGENT_RULE_TAB_TYPE anymore, right?. These Tab.setSelectedTab writes, are these needed or can be removed?
There was a problem hiding this comment.
Yes, activeTab is local now and nothing reads SELECTED_TAB + AGENT_RULE_TAB_TYPE, so those writes are not required, removing them.
…ggestions # Conflicts: # src/languages/de.ts # src/languages/en.ts # src/languages/es.ts # src/languages/fr.ts # src/languages/it.ts # src/languages/ja.ts # src/languages/nl.ts # src/languages/pl.ts # src/languages/pt-BR.ts # src/languages/zh-hans.ts # src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx
|
@mananjadhav All yours. |
| agentCreatedTitle: 'RuleBot がワークスペースに追加されました!', | ||
| agentCreatedDescription: (agentsRoute: string) => | ||
| `<muted-text>エージェント ルールを適用するために、エージェントを作成し、ワークスペースの管理者として追加しました。<br><br>エージェントの詳細は <a href="${agentsRoute}">「アカウント」>「エージェント」</a> で編集できます。</muted-text>`, | ||
| suggestionsTab: 'おすすめ', |
There was a problem hiding this comment.
question: Are we sure the translation is correct?
| /** Higher priority wins when multiple rules match the same suggestion. */ | ||
| priority: number; | ||
|
|
||
| /** keywords matched against suggestion id + title */ |
There was a problem hiding this comment.
| /** keywords matched against suggestion id + title */ | |
| /** Keywords matched against suggestion id + title */ |
Explanation of Change
Fixed Issues
$ #95742
PROPOSAL: #95742 (comment)
Tests
Offline tests
QA Steps
Same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android.Native.mp4
Android: mWeb Chrome
Android.mWeb.Chrome.mp4
iOS: Native
IOS.Native.mp4
iOS: mWeb Safari
IOS.mWeb.safari.mp4
MacOS: Chrome / Safari
macOS.Chrome.mp4