Merge main

Author: mountiny

.claude/agents/code-inline-reviewer.md

@@ -66,58 +66,48 @@ Bad:
 
 ---
 
-### [PERF-2] Use early returns in array iteration methods
+### [PERF-2] Return early before expensive work
 
-- **Search patterns**: `.every(`, `.some(`, `.find(`, `.filter(`
+- **Search patterns**: Function bodies where `if (!param)` or `if (param === undefined)` appears AFTER function calls that use `param`
 
 - **Condition**: Flag ONLY when ALL of these are true:
 
-  - Using .every(), .some(), .find(), .filter() or similar function
-  - Function contains an "expensive operation" (defined below)
-  - There exists a simple property check that could eliminate items earlier
-  - The simple check is performed AFTER the expensive operation
-
-  **Expensive operations are**:
-
-  - Function calls (except simple getters/property access)
-  - Regular expressions
-  - Object/array iterations
-  - Math calculations beyond basic arithmetic
-
-  **Simple checks are**:
-
-  - Property existence (!obj.prop, obj.prop === undefined)
-  - Boolean checks (obj.isActive)
-  - Primitive comparisons (obj.id === 5)
-  - Type checks (typeof, Array.isArray)
+  - Code performs expensive work (function calls, iterations, API/Onyx reads)
+  - A simple check could short-circuit earlier
+  - The simple check happens AFTER the expensive work
 
   **DO NOT flag if**:
 
-  - No expensive operations exist
-  - Simple checks are already done first
-  - The expensive operation MUST run for all items (e.g., for side effects)
+  - Simple checks already come first
+  - Validation requires the computed result
+  - Expensive work must run for side effects
 
-- **Reasoning**: Expensive operations can be any long-running synchronous tasks (like complex calculations) and should be avoided when simple property checks can eliminate items early. This reduces unnecessary computation and improves iteration performance, especially on large datasets.
+- **Reasoning**: Early returns prevent wasted computation. Validate inputs before passing them to expensive operations.
 
 Good:
 
 ```ts
-const areAllTransactionsValid = transactions.every((transaction) => {
-    if (!transaction.rawData || transaction.amount <= 0) {
-        return false;
+function clearReportActionErrors(reportID: string, reportAction: ReportAction) {
+    if (!reportAction?.reportActionID) {
+        return;
     }
-    const validation = validateTransaction(transaction);
-    return validation.isValid;
-});
+
+    const originalReportID = getOriginalReportID(reportID, reportAction);
+    // ...
+}
 ```
 
 Bad:
 
 ```ts
-const areAllTransactionsValid = transactions.every((transaction) => {
-    const validation = validateTransaction(transaction);
-    return validation.isValid;
-});
+function clearReportActionErrors(reportID: string, reportAction: ReportAction) {
+    const originalReportID = getOriginalReportID(reportID, reportAction);
+
+    if (!reportAction?.reportActionID) {
+        return;
+    }
+    // ...
+}
 ```
 
 ---
@@ -996,6 +986,8 @@ function ReportScreen({ params: { reportID }}) {
 
 - **Reasoning**: When parent components compute and pass behavioral state to children, if a child's requirements change, then parent components must change as well, increasing coupling and causing behavior to leak across concerns. Letting components own their behavior keeps logic local, allows independent evolution, and follows the principle: "If removing a child breaks parent behavior, coupling exists."
 
+**Distinction from CLEAN-REACT-PATTERNS-3**: This rule is about data flow DOWN (parent → child) — "Don't pass data the child can get itself."
+
 Good (component owns its behavior):
 
 - Component receives only IDs and handlers
@@ -1079,6 +1071,124 @@ In this example:
 
 ---
 
+### [CLEAN-REACT-PATTERNS-3] Design context-free component contracts
+
+- **Search patterns**: Callback props with consumer-specific signatures like `(index: number) => void`, props used only to extract values for callbacks, refs used to access external component state, useImperativeHandle
+
+- **Condition**: Flag ONLY when BOTH of these are true:
+
+  1. A component's interface is shaped around a specific consumer's implementation rather than abstract capabilities
+  2. AND at least ONE of the following manifestations is present:
+     - The component receives data only to extract values for callbacks (doesn't use it for rendering)
+     - Callback signatures encode consumer-specific assumptions (e.g., `(index: number) => void` for navigation)
+     - The component accesses external state through refs or imperative handles
+
+  **Signs of violation:**
+  - Callback signatures that encode consumer assumptions: `navigateToWaypoint(index: number)` instead of `onAddWaypoint()`
+  - Props passed only to extract values for callbacks, not for rendering (e.g., `transaction` passed just to compute `waypoints.length`)
+  - Imperative access to external state via refs or `useImperativeHandle`
+  - Component requires modification to work in a different context
+
+  **DO NOT flag if:**
+  - Component signals events with data it naturally owns (e.g., `onChange(value)` for an input, `onSelectItem(item)` for a list)
+  - Callbacks are abstract actions the component can trigger (e.g., `onAddStop()`, `onSubmit()`)
+  - State coordination happens at a higher level with clear data flow
+
+  **What makes a contract "abstract":**
+  - Callback describes *what happened* in component terms: `onAddStop`, `onSelect`, `onChange`
+  - Callback does NOT describe *what consumer should do*: `navigateToWaypoint(index)`, `updateParentState(value)`
+  - Props are used for rendering or internal logic, not just to compute callback arguments
+  - Component works without modification in a different context
+
+- **Reasoning**: A component's contract should expose its capabilities abstractly, not encode assumptions about how it will be used. When interfaces leak consumer-specific details, the component becomes coupled to that context and requires modification for reuse. Good contracts signal *what the component can do*, not *what the consumer needs*.
+
+**Distinction from CLEAN-REACT-PATTERNS-2**: PATTERNS-2 ensures components fetch their own data. This rule ensures components expose abstract capabilities, not consumer-specific interfaces.
+
+Good (abstract contract):
+
+- Interface exposes capability: "user can add a stop"
+- Implementation details (index computation, navigation) stay with consumer
+- Component is reusable in any context needing an "add stop" action
+
+```tsx
+<DistanceRequestFooter
+    onAddStop={() => {
+        const nextIndex = Object.keys(transaction?.comment?.waypoints ?? {}).length;
+        Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_WAYPOINT.getRoute(..., nextIndex.toString(), ...));
+    }}
+/>
+
+// in DistanceRequestFooter
+<Button onPress={onAddStop}>{translate('distance.addStop')}</Button>
+```
+
+Bad (contract leaks consumer assumptions):
+
+- Callback `navigateToWaypointEditPage(index: number)` encodes routing assumption
+- `transaction` prop exists only to compute index for callback
+- Requires modification if consumer navigates differently
+
+```tsx
+type DistanceRequestFooterProps = {
+    waypoints?: WaypointCollection;
+    navigateToWaypointEditPage: (index: number) => void;  // Encodes routing assumption
+    transaction: OnyxEntry<Transaction>;
+    policy: OnyxEntry<Policy>;
+};
+
+// in IOURequestStepDistance
+<DistanceRequestFooter
+    waypoints={waypoints}
+    navigateToWaypointEditPage={navigateToWaypointEditPage}
+    transaction={transaction}
+    policy={policy}
+/>
+
+// in DistanceRequestFooter - computes value for consumer's callback
+<Button
+    onPress={() => navigateToWaypointEditPage(Object.keys(transaction?.comment?.waypoints ?? {}).length)}
+    text={translate('distance.addStop')}
+/>
+```
+
+Good (independent contracts):
+
+- Each component has a self-contained interface
+- State coordination happens at composition level
+
+```tsx
+function EditProfile() {
+    const [formData, setFormData] = useState<FormData>();
+    return (
+        <>
+            <Form onChangeFormData={setFormData} />
+            <SaveButton onSave={() => API.save(formData)} />
+        </>
+    );
+}
+```
+
+Bad (coupled contracts):
+
+- `SaveButton` interface requires knowledge of `Form`'s internals
+- Neither component works independently
+
+```tsx
+function SaveButton({ getSiblingFormData }: { getSiblingFormData: () => FormData }) {
+    const handleSave = () => {
+        const formData = getSiblingFormData(); // Reaches into sibling
+        API.save(formData);
+    };
+    return <Button onPress={handleSave}>Save</Button>;
+}
+
+// Parent wires siblings together
+<Form ref={formRef} />
+<SaveButton getSiblingFormData={() => formRef.current?.getData()} />
+```
+
+---
+
 ## Instructions
 
 1. **First, get the list of changed files and their diffs:**

.github/workflows/testBuildOnPush.yml

@@ -5,6 +5,10 @@ on:
     branches: [main]
     paths-ignore: ['docs/**', 'contributingGuides/**', 'help/**', '.github/**', 'scripts/**', 'tests/**', 'jest/**', '.claude/**']
 
+concurrency:
+  group: 'testBuildOnPush'
+  cancel-in-progress: false
+
 jobs:
   prep:
     runs-on: ubuntu-latest
@@ -432,7 +436,6 @@ jobs:
               summary.addRaw(`\n**Mobile-Expensify Submodule SHA:** [${submoduleSHA}](${submoduleUrl})`);
             }
             
-
             const prUrl = '${{ needs.prep.outputs.PR_URL }}';
 
             if (prUrl) {
@@ -441,5 +444,4 @@ jobs:
               summary.addRaw(`\n**PR Link:** No PR associated with this commit.`);
             }
             
-            
             summary.write();

Mobile-Expensify

@@ -114,8 +114,8 @@ android {
         minSdkVersion rootProject.ext.minSdkVersion
         targetSdkVersion rootProject.ext.targetSdkVersion
         multiDexEnabled rootProject.ext.multiDexEnabled
-        versionCode 1009031149
-        versionName "9.3.11-49"
+        versionCode 1009031153
+        versionName "9.3.11-53"
         // Supported language variants must be declared here to avoid from being removed during the compilation.
         // This also helps us to not include unnecessary language variants in the APK.
         resConfigs "en", "es"

android/app/build.gradle

@@ -193,15 +193,16 @@ This helps future investigators understand the history and current status of err
 16. Please pay attention to the pull request template, especially to how we link PRs with issues they fix. Make sure you don't use GitHub keywords such as `fixes` in your PR description, as this can break our current automated steps for issue management. Follow the PR template format carefully.
 17. Upon submission of a PR, please include a numbered list of explicit testing steps for each platform (Web, iOS, Android, and Mobile Web) to confirm the fix works as expected and there are no regressions.
 18. Please add a screenshot of the app running on each platform (Web, iOS, Android, Mobile Web).
+19. Please review the [PR Authoring & Reviewing Best Practices](./PR_AUTHOR_REVIEWER_BEST_PRACTICES.md) for standards on PR titles, testing responsibilities, and the review workflow.
 
 ### Completing the final checklist
-19. Once your PR has been deployed to production, a checklist will automatically be commented in the GH issue. You're required to complete the steps that have your name mentioned before payment will be issued.
-20. The items requiring your completion consist of: 
+20. Once your PR has been deployed to production, a checklist will automatically be commented in the GH issue. You're required to complete the steps that have your name mentioned before payment will be issued.
+21. The items requiring your completion consist of: 
     1. Proposing steps to take for a regression test to ensure the bug doesn't occur again (For information on how to successfully complete this, head [here](https://github.com/Expensify/App/blob/main/contributingGuides/REGRESSION_TEST_BEST_PRACTICES.md)).
     2. Identifying and noting the offending PR that caused the bug (if any).
     3. Commenting on the offending PR to note the bug it caused and why (if applicable).
     4. Starting a conversation on if any additional steps should be taken to prevent further bugs similar to the one fixed from occurring again. 
-21. Once the above items have been successfully completed, then payments will begin to be issued. 
+22. Once the above items have been successfully completed, then payments will begin to be issued. 
 
 ### Timeline expectations and asking for help along the way
 - If you have made a change to your pull request and are ready for another review, leave a comment that says "Updated" on the pull request  itself.

assets/images/home-testdrive-image.png

@@ -19,6 +19,7 @@ C+ are contributors who are experienced at working with Expensify and have gaine
 - Follow our Code of Conduct, Contributing.md and README.md docs and processes.
 - Comment and fix bugs in a timely manner.
 - Clear communicator.
+- Familiar with [PR Authoring & Reviewing Best Practices](./PR_AUTHOR_REVIEWER_BEST_PRACTICES.md).
 - Bonus points:
   - Help other contributors by commenting on their issues. 
   - Actively involved in the #expensify-open-source slack channel.