Add rules engine types and merchant rule AST helpers

[Krishna2323]

Explanation of Change

Fixed Issues

$ #100300
PROPOSAL:

Tests

Setup

Control workspace with Categories, Tags, Taxes, and Rules enabled. Have a second Control workspace ready for §5, §9, and §10. Test with RULES_REVAMP on, then repeat §1–§3 with it off. Both use the same collection now.

1. Create a merchant rule

1. Rules → Expense defaults → Add merchant rule.
2. Set Merchant = Starbucks, Match type = Contains, Category = any category. Save.
3. Verify the rule and summary: If merchant contains "Starbucks" / Update category to "…".
4. Refresh and verify it persists without pending styling.

2. Edit a merchant rule

1. Open the rule and verify all fields are pre-filled.
2. Change Match type to Exactly matches and change the category. Save.
3. Verify the updated summary and category.
4. Refresh and verify it persisted without creating a second rule.

3. Delete a merchant rule

1. Open → Delete rule → confirm.
2. Verify it disappears and stays gone after refresh.

4. All default fields round-trip

1. Create a rule with merchant rename, category, tag, tax, description, vendor, reimbursable, and billable.

2. Refresh and reopen. Verify all values are preserved.

   • Description preserves markdown.
   • Tax shows rate name + percentage.
   • Don't change stays unset, not false.

3. Clear the description with spaces only. Verify it saves as empty.

5. Multi-workspace scoping ⚠️

1. Create a merchant rule on Workspace A.
2. Switch to B and verify A's rule isn't shown.
3. Create a rule on B, return to A, and verify only A's rule appears.
4. Repeat for Rules row, Copy settings, Duplicate workspace, and Getting started counts.

6. Fetch triggers ⚠️

1. Cold-load directly into a workspace overview and verify rules load.
2. Navigate to Rules and verify rules load.
3. Go offline/online from the overview and verify rules re-fetch correctly.
4. Verify GetRules does not fire on OpenApp.

7. Offline and errors

1. Offline: create a rule → verify pending/greyed out.
2. Online: verify pending clears and rule persists.
3. Offline: delete a rule → verify pending delete, then gone after reconnect.
4. Force SetRule failure → verify red error, Rules row red dot, and clean removal after dismissing.
5. Offline: edit a rule → verify pending state and correct reconnect behavior.

8. Unsupported rules stay read-only

Using a hand-made rule with a nested and tree or multiple merchant comparisons:

1. Verify it appears with a summary.
2. Verify it can't be opened or bulk-selected.
3. Verify saving another rule doesn't wipe or rewrite it.

9. CSV import

1. Import merchant rules and map the columns.
2. Verify imported rules appear and persist after refresh.
3. Re-import the same file → duplicates skipped and count reported.
4. Import an unknown category → rows reported invalid, not saved.
5. Verify an imported rule can be edited normally.

10. Duplicate workspace / Copy settings

1. Duplicate with Merchant rules selected → verify count and equivalent rules with new IDs.
2. Duplicate with Merchant rules unselected → verify no rules copied.
3. Copy merchant rules to two targets → verify both receive independent copies and source is unchanged.
4. Verify zero-rule workspaces don't error.

11. Disabling approvals ⚠️

1. With merchant rules + approval workflows, disable Add approvals.
2. Verify workflows are removed but all merchant rules remain after refresh.
3. Re-enable approvals and verify rules remain intact.

12. Approval workflows regression

Keep the existing five checks, plus:
6. Edit an approval workflow with merchant rules present → verify no merchant rule changes.
7. Remove an approver → verify reconciliation works and merchant rules remain untouched.

13. Category tax defaults

1. Add a default tax rate to a category and verify save/persist/edit/delete.
2. Verify it doesn't appear in Expense defaults; it remains in policy.rules.expenseRules.

14. Indicators and side effects

1. Getting started: Set up rules incomplete with no rules, complete after adding one.
2. Rules row shows a red dot only for failures in the current workspace.
3. Preview matches still shows matching unsubmitted expenses.
4. Search/filter still matches merchant, category, tag, and description.
5. Bulk-delete several rules → verify all are removed and stay gone after refresh.

Notes

• §8: Requires a hand-made unsupported rule; there's no UI to create one.
• §9: CSV import still uses the legacy ImportMerchantRulesSpreadsheet shape. Backend failure here is expected until that is resolved.

• Verify that no errors appear in the JS console

Offline tests

1. Offline: create a rule → verify pending/greyed out.
2. Online: verify pending clears and rule persists.
3. Offline: delete a rule → verify pending delete, then gone after reconnect.
4. Force SetRule failure → verify red error, Rules row red dot, and clean removal after dismissing.
5. Offline: edit a rule → verify pending state and correct reconnect behavior.

QA Steps

• Same as tests

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I verified there are no new alerts related to the canBeMissing param for useOnyx
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If new assets were added or existing ones were modified, I verified that:
  • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
  • The assets load correctly across all supported platforms.
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

android_hybrid.mp4

Android: mWeb Chrome

android_mWeb.mp4

iOS: Native

ios_hybrid.mp4

iOS: mWeb Safari

ios_mWeb.mp4

MacOS: Chrome / Safari

web_chrome.mp4

[codecov]

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.

Files with missing lines	Coverage Δ
src/CONST/index.ts	91.22% <ø> (ø)
src/ONYXKEYS.ts	100.00% <ø> (ø)
src/hooks/useRulesPrefetch.ts	100.00% <100.00%> (ø)
src/libs/API/types.ts	100.00% <ø> (ø)
src/libs/ExportOnyxState/common.ts	80.95% <ø> (ø)
src/libs/PolicyUtils.ts	80.24% <100.00%> (-0.10%)	⬇️
src/libs/WorkflowUtils.ts	92.65% <100.00%> (+0.04%)	⬆️
src/libs/actions/Policy/Policy.ts	71.99% <100.00%> (+0.46%)	⬆️
...tingStartedSection/hooks/useGettingStartedItems.ts	98.68% <100.00%> (+0.03%)	⬆️
src/pages/workspace/WorkspaceInitialPage.tsx	85.71% <100.00%> (+0.29%)	⬆️
... and 17 more
... and 12 files with indirect coverage changes

[Krishna2323]

@codex review

[chatgpt-codex-connector]

Review finding

• [P1] Preserve meaningful condition text for unsupported merchant rules. getMerchantRuleFormValues() intentionally returns undefined for nested or multi-merchant filters so those rules remain read-only. src/libs/ExpenseDefaultRuleUtils.tsL260-L275 However, MerchantRulesSection derives both the displayed merchant name and search tokens exclusively from that helper. src/pages/workspace/rules/MerchantRulesSection.tsxL114-L115 src/pages/workspace/rules/MerchantRulesSection.tsxL157-L164 Consequently, every unsupported rule is rendered as a disabled row with an empty condition (for example, “If merchant contains “””), and cannot be found by its merchant. This contradicts the intended behavior in the PR’s test plan that unsupported rules still appear with a useful summary. Use getRuleFilterLeaves() (as the revamp table already does) to derive display/search merchant values independently of editability, while continuing to disable navigation.

No code changes were made, so no commit or follow-up PR was created.

Checks

• ✅ git diff --check 0283d2bebad28796ca74b9506d358232988fe376...HEAD
• ⚠️ npm test -- --runInBand tests/unit/ExpenseDefaultRuleUtilsTest.ts tests/actions/PolicyRulesTest.ts tests/actions/CopyPolicySettingsTest.ts tests/actions/PolicyTest.ts tests/unit/WorkflowUtilsTest.ts tests/unit/PolicyUtilsTest.ts (could not run because dependencies are unavailable: jest: not found).

View task →

[melvin-bot]

@luacmartins @JS00001 One of you needs to 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]

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76a721a004

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Convert imported rows to rules-engine payloads

For every valid CSV row this still builds the legacy flat CodingRule shape, but the migrated list/editor only recognize rules-engine records with CreateTransaction triggers and Set actions in the rules_ collection. Thus imports either fail at the legacy endpoint or create rules that this UI cannot display or edit; construct rules-engine bodies (or switch the import endpoint) before reporting the import as successful.

Useful? React with 👍 / 👎.

[flaviadefaria]

Switching the review to @JmillsExpensify since this is his project.

[MelvinBot]

🤖 Code review of 76a721a

What it does: moves merchant rules off policy.rules.codingRules and into the standalone rules_ Onyx collection shared with approval workflows, adds the rules-engine filter/action types, and introduces ExpenseDefaultRuleUtils to convert between the engine's filter tree and the flat form the editor uses. All CI is green apart from the two checklist/approval checks.

The core design — getMerchantRuleFormValues() returning undefined so a rule the form can't represent stays read-only rather than being saved back lossily — is the right call, and the earlier codex P1 (merchant name and search tokens derived from that helper) is genuinely fixed via getRuleMerchantMatchSummary. One P1 remains, plus a display regression.

---

🔴 P1 — an unsupported rule is still editable by URL, and saving destroys it

MerchantRulePageBase.tsx:577 guards for a missing rule but not for one that exists and is not editable. The list and table only set disabled on the row, which blocks a tap — not the route.

Reach /workspaces/<policyID>/rules/merchant/<ruleID>/edit for a nested / multi-merchant rule any other way (bookmark, browser back or forward, deeplink) and the editor renders empty, because the seeding effect at MerchantRulePageBase.tsx:197-205 bails out. Type a merchant plus one field, press Save, and setMerchantRule Onyx.METHOD.SETs and SetRules that same ruleID with a freshly built single-comparison rule. The nested tree and every extra action are gone — the exact data loss the read-only design exists to prevent.

isEditableMerchantRule is already exported, so the guard is one block next to the existing two:

if (isEditing && existingRule && !isEditableMerchantRule(existingRule) && !isClosing) {
    return <NotFoundPage />;
}

Test plan §8 covers only the in-app tap path — please add the direct-URL case, and a unit test asserting the route is blocked.

🟠 P2 — tax defaults now show stale values, or vanish

MerchantTypeRulesUtils.ts:153 replaced getTaxRateDisplayName(policy, externalID, savedSnapshot) with the stored snapshot alone, gated on field_id_TAX.value being truthy. getTaxRateDisplayName prefers the live rate from policy.taxRates.taxes, falls back to the snapshot, then to the tax ID. So two regressions in the revamp table:

• Rename a tax rate or change its percentage → the row keeps showing the name and rate captured when the rule was saved.
• buildTaxActionValue omits name/value whenever the tax key isn't in policy.taxRates.taxes at save time — a rule saved before taxes loaded, or a rate deleted later. The tax default then renders nothing at all and silently disappears from the summary. The old code fell back to the tax ID.

getTaxRateDisplayName was even dropped from the imports. Worth restoring in both this table and MerchantRulesSection.tsx:73-74.

🟡 P3 — getRules() re-pulls every rule in the account on each workspace focus

getRules() takes no params, has no dedup, and its response SETs the whole collection — every rule the user can see, in every workspace. It now fires from WorkspaceInitialPage.fetchPolicyData on focus and reconnect, plus both rules pages. Tabbing between workspace overviews re-downloads the lot each time.

The waitForWrites inside API.read means this can't clobber an in-flight optimistic write, so it's performance rather than correctness — but it's the shape the High Traffic account checklist item is about. A last-fetched or loading guard would help.

Four smaller items and two nits

Inconsistent sort comparator — (second.rule.created ?? '') < (first.rule.created ?? '') ? -1 : 1 never returns 0, so compare(a,b) === compare(b,a) === 1 on ties and the order is implementation-defined. buildCopiedExpenseDefaultRules stamps every copy with one shared created, so §10 (duplicate / copy settings with several rules) hits the tie case directly. Two sites: MerchantRulesSection.tsx:111 and MerchantTypeRulesUtils.ts:134.

Clearing every update field silently does nothing — buildMerchantRule returns undefined with no actions, and setMerchantRule then only Log.warns. An admin who clears all the set-fields and presses Save gets no change and no error. Validate "at least one field" in the form, or surface an error.

emptyToUndefined keeps untrimmed values — ExpenseDefaultRuleUtils.ts:82 returns value, not value.trim(), so buildMerchantRuleFilters trims the matched merchant while the action values keep their padding. " Starbucks " saves as-is.

Classifiers aren't mutually exclusive — isExpenseDefaultRule (some trigger is CreateTransaction and some action is Set) and isApprovalWorkflowRule (some trigger is a report event) can both be true for one hand-made rule, which would list it as a merchant rule and delete it when approvals are turned off. Also note isExpenseDefaultRule asserts rule is Rule & ExpenseDefaultRule from that heuristic without checking every action's shape — contained, since getMerchantRuleFormValues re-validates field by field, but the predicate is stronger than the check behind it.

Nit — Rules.ts:89-105 has two stacked JSDoc blocks on setMerchantRule; the first is stale (says "coding rule", names the param form, omits existingRule).

Nit — selector memoization is inconsistent: WorkspaceInitialPage wraps createHasExpenseDefaultRuleErrorsSelector in useMemo, while useGettingStartedItems and CopyPolicySettingsSelectFeaturesPage build theirs inline on every render. Worth picking one.

❓ Do the duplicate and copy flows actually repopulate rules?

Both buildDuplicatePolicyData and buildCopyPolicySettingsData push successData that SETs each optimistic copy to null, on the premise that the server mints its own IDs. If DuplicatePolicy / CopyPolicySettings don't return the new rules in their onyxUpdates, the target workspace shows zero merchant rules until the next GetRules. That may self-heal now that GetRules fires on workspace focus, but §10's "verify count and equivalent rules with new IDs" should be checked against the real API — the unit test asserts it with the fetch paused, so it only covers the optimistic half.

What's good

• The read-only-rather-than-lossy contract is documented where it matters, including the explicit "callers MUST treat undefined as read-only" note.
• isApprovalWorkflowRule narrowing in setWorkspaceApprovalMode is the right fix for the shared collection — disabling approvals no longer takes merchant rules with it (§11).
• Extracting RuleFilters de-duplicates the approval-workflow filter types cleanly.
• 381 lines of new util tests covering real edge cases (single-value lists, nested trees, disagreeing operators, whitespace-only descriptions, missing tax rates), and the comments explain why rather than what.

Runtime check

I tried to verify create → persist → edit → delete on web, but the test session never rendered past a blank page after sign-in (0 accessibility nodes over 12 minutes; dev-server bundles all served fine). That's an environment failure, not a signal about this PR — so treat the review above as static analysis only, with the merchant-rule CRUD flow still unverified at runtime.

---

view run · no recording available

[MelvinBot]

🤖 Regression hunt on 76a721a

Six regressions, ranked. Two are silent data loss. All line references verified against the branch.

The common root cause for #2–#4: merchant rules used to arrive with the policy on OpenApp as policy.rules.codingRules. They now live in the rules_ collection, which is populated only by getRules() — and getRules() is called from exactly three places: WorkspaceInitialPage.tsx:155, PolicyRulesPage.tsx:67, PolicyRulesPageRevamp.tsx:106. Any screen not downstream of one of those reads an empty collection.

---

1. 🔴 Revamp table: an unsupported rule opens on a plain click, and saving destroys it

The read-only gate isn't wired up. MerchantTypeRulesUtils.ts:183 sets disabled: !isEditable on the item, but WorkspaceExpenseDefaultsTableRow.tsx:109 passes only disabled={isDeleting} to Table.Row. In TableRow.tsx:212-218 a wide-layout press falls straight through to onPress → item.action() → the edit route. item.disabled is read only at TableRow.tsx:221 (narrow-layout mobile selection) and :192 (the checkbox).

So with RULES_REVAMP on, on desktop, test plan §8 "verify it can't be opened" fails. The editor then renders blank — MerchantRulePageBase.tsx:577-589 has no non-editable guard — and Save SETs the same ruleID with a fresh single-comparison rule, wiping the nested tree.

This corrects my earlier review, which said the path needed a deeplink. It needs one click. Bulk select-all is correctly blocked (useRulesTableBulkActions.ts:185), and the non-revamp list gates correctly at MerchantRulesSection.tsx:189 — it's only the revamp table.

Fix: guard the editor itself (if (isEditing && existingRule && !isEditableMerchantRule(existingRule)) return <NotFoundPage />;) rather than relying on the row, since the row prop isn't read.

2. 🔴 Duplicate workspace and Copy settings silently drop merchant rules

The count doesn't just label the row — it decides whether the row exists. WorkspaceDuplicateSelectFeaturesForm.tsx:161 renders the "Merchant rules" option only when codingRulesCount > 0; CopyPolicySettingsUtils.ts:288-289 does the same.

Both flows launch from the workspaces-list three-dot menu, which never mounts any of the three fetching pages. So with an unpopulated collection the count is 0, the row is absent, parts.codingRules is false, and the server is told not to copy. No error, no warning — the duplicate just ships without its merchant rules. On main the count came from the policy blob that arrived with OpenApp.

Scoping caveat: rules_ is persisted to disk (it's in neither evictableKeys nor ramOnlyKeys in setup/index.ts), so this self-heals once the user has opened any workspace. The window is first load after deploy, fresh sign-in, new device or browser, and cache clear. Still worth fixing — calling getRules() on these two pages closes it.

3. 🟠 Home "Set up rules" checklist flips back to incomplete

Same root cause. useGettingStartedItems.ts:72 reads the collection from the Home page, which fetches nothing, and hasExpenseDefaultRules is now the first check in hasConfiguredRules. For a workspace whose only rules config is merchant rules, the item shows unchecked — and since the section only hides when every item is complete, the whole Getting started block reappears for a user who had finished onboarding.

4. 🟠 CSV import writes to a location nothing reads any more

The producing half is unchanged from main: ImportedMerchantRule = Omit<CodingRule, …>, the page still builds the flat legacy shape, and importMerchantRulesSpreadsheet does no optimistic Onyx write. The consuming half moved: policy.rules.codingRules (Policy.ts:3125) now has zero readers in src/.

The PR body predicts a backend rejection, which is one outcome. The other is worse: a legacy-shaped success merges into codingRules, the user sees "Import successful — N merchant rules have been added", lands on Expense defaults, and sees nothing. A refresh doesn't help.

Two knock-ons either way, since ImportedMerchantRulesPage.tsx:175-181 now seeds its duplicate set from the new collection: re-import dedupe never fires (§9.3 will fail), and the offline import short-circuit is permanently disabled.

5. 🟠 The rule lookup lost its policy and kind scoping

MerchantRulePageBase.tsx:182 resolves rules_<ruleID> against the whole collection — every workspace, and both rule kinds. main used policy?.rules?.codingRules?.[ruleID], scoped by construction, so a wrong ID was inert.

Combined with the missing guard from #1, a stale or hand-edited /rules/merchant/…/<approvalWorkflowRuleID> URL now resolves. Save fires SetRule on that ID with a merchant body and Delete fires DeleteRule — replacing or removing a live approval workflow. Worth checking scopeID === policyID and isExpenseDefaultRule here regardless of how #1 is fixed.

6. 🟡 A rule with both trigger kinds is deleted when approvals are turned off

isApprovalWorkflowRule uses .some(), so a rule carrying both CreateTransaction and ReportSubmit satisfies it and isExpenseDefaultRule. It is listed as a merchant rule but passes the guard in setWorkspaceApprovalMode and gets dropped. .every() would match the convention areTriggersEditable already uses. Not App-producible today, but it's the assumption the whole shared-collection design rests on.

Paths I checked and found genuinely safe

Worth stating, because these were the scariest surfaces:

• setWorkspaceApprovalMode — the isApprovalWorkflowRule guard at Policy.ts:1104 is correct and complete; it covers the optimistic SET-null and the failure restore, since both sit in the same loop body. Every App-built approval rule carries a report trigger, so nothing that should be deleted is now skipped. §11 holds, and the PR adds a regression test for it.
• The four approval-workflow reconcilers — every diff key is either a key of context.existingRules or a fresh rand64(), and all existingRules producers go through getApprovalWorkflowRulesForPolicy, which now narrows. They cannot touch a merchant rule.
• Workflow.ts rule diff — same constraint; a null there can't wipe a merchant rule.
• Preview matches — not regressed. The unseeded-draft path is unreachable because previewMatches already bails on an empty merchantToMatch at MerchantRulePageBase.tsx:568-575. The user gets the inline error instead.
• The approval-workflow UI (WorkflowsApprovalsTab, the approvals create/edit pages) — no merchant-rule leak into any count or list; each narrows at the point of use. One perf note: filterRulesForPolicy narrows by policy but not by kind, so saving a merchant rule now re-renders those pages.

Still open — needs a backend answer

1. Are existing codingRules migrated into rules_? Nothing in App reads policy.rules.codingRules any more. If the backend hasn't migrated them, every merchant rule that exists today vanishes from the UI on deploy. This is the single biggest risk and it can't be settled from this repo.
2. Does GetRules SET or MERGE? Rules.ts:35 says SET. If any command that populates rules_ ever returns a partial payload with onyxMethod: set, it wipes the other rule kind — the inherent hazard of two independently-fetched kinds sharing one SET-replaced collection.

Runtime check

Blocked again, for environment reasons rather than anything about this PR. A stale element ref during Control-plan setup landed a click on a help-site link, which navigated the session off the app and killed it before any merchant rule existed. Nothing above is runtime-verified — it is all static analysis, though every line reference was confirmed against the branch.

---

view run · no recording available

[luacmartins]

@Krishna2323 conflicts

[github-actions]

This PR adds a new Onyx.connectWithoutView call, so I've requested a review from the Onyx performance reviewers (@tgolen, @mountiny, @luacmartins) — a review from any one of them is enough. Please add a link in your PR description to the Slack discussion where the @frontend-performance team approved using connectWithoutView here.

[Krishna2323]

This PR adds a new Onyx.connectWithoutView call, so I've requested a review from the Onyx performance reviewer

Removing that, sorry for overlooking it.

[Krishna2323]

All code-level findings from both reviews are addressed, plus the earlier Codex one on the condition text.

Finding	Status
Unsupported rule editable (revamp row click + no editor guard)	Fixed - added canEditMerchantRule(rule, policyID) to gate the editor, and the row now passes item.disabled to Table.Row instead of dropping it
Rule lookup lost policy/kind scoping	Fixed - the same guard checks the scope, so an approval workflow or another policy's ruleID can't resolve
Tax defaults stale or disappearing	Fixed - restored getTaxRateDisplayName in both surfaces, so it falls back from live rate -> saved snapshot -> tax ID
Duplicate / Copy settings silently dropping merchant rules	Fixed - both flows now prefetch the collection
"Set up rules" checklist flipping to incomplete	Fixed - same prefetch
getRules() re-pulling the whole account	Fixed - getRules now sets an Onyx hasRulesDataBeenFetched flag, and consumers go through a useRulesPrefetch hook, following usePolicyConnectionsPrefetch
Rule with both trigger kinds deleted when approvals are disabled	Fixed - isApprovalWorkflowRule now requires every trigger to be a report event

---

@JS00001, two things for you, one blocker and one that checked out fine.

Blocker: GetRules returns no data. Verified end to end:

1. Created a merchant rule. SetRule returned 200 and the response had the new record, rules_6964069421310331044, correctly scoped to the policy.
2. Cleared the app cache and signed in fresh.
3. GetRules goes out and comes back {"jsonCode":200,"onyxData":[]}.

So the rule exists server-side, but nothing ever hydrates the rules_ collection on the client. Every merchant rule disappears from the UI after a cache clear.

We're calling it with no parameters, as the spec in the issue describes. Does it need a policyID, or is it just not returning data yet?

Checked out fine: ImportMerchantRulesSpreadsheet already dual-writes. A CSV import returns proper rules-engine records under rules_<ruleID> alongside the legacy policy.rules.codingRules, correctly scoped. So the import payload stays as it is and needs no change on my side.

[JS00001]

Blocker: GetRules returns no data. Verified end to end:

I'll look into this

[JS00001]

Looks like we weren't returning the response. I have a fix for that. We're also running into one more issue where the number indexed keys (triggers: { 0: ...}) are getting returned as an array, so I'll need to fix this as well

[luacmartins]

More conflicts

[Krishna2323]

Looks like we weren't returning the response. I have a fix for that. We're also running into one more issue where the number indexed keys (triggers: { 0: ...}) are getting returned as an array, so I'll need to fix this as well

Conflicts resolved. LMK when this is fixed.

[JS00001]

PR open to fix ^