Skip to content

Add Chronos display name constant to fix search showing Chat Report#84391

Draft
MelvinBot wants to merge 9 commits intomainfrom
claude-fixChronosDisplayNameInSearch
Draft

Add Chronos display name constant to fix search showing Chat Report#84391
MelvinBot wants to merge 9 commits intomainfrom
claude-fixChronosDisplayNameInSearch

Conversation

@MelvinBot
Copy link
Contributor

@MelvinBot MelvinBot commented Mar 6, 2026

Explanation of Change

When searching for reports like Chronos, the search results display "Chat Report" instead of the correct name. This happens because the display name for 1:1 DMs depends on participant personal details being loaded, but Chronos's personal details may not be available at initialization time — causing the fallback to the server default report.reportName ("Chat Report").

Concierge doesn't have this issue because it has a dedicated check that short-circuits to CONST.CONCIERGE_DISPLAY_NAME before attempting to resolve from personal details. This PR applies the same pattern for Chronos:

  1. Adds CHRONOS_DISPLAY_NAME: 'Chronos' constant in CONST/index.ts
  2. Adds a chatIncludesChronos check in computeReportName (ReportNameUtils.ts) — used by derived report attributes and search
  3. Adds the same check in the deprecated getReportName (ReportUtils.ts) — used by the chat header

This ensures Chronos always resolves to its correct display name regardless of personal details loading timing.

Fixed Issues

$ #83447
PROPOSAL: #83447 (comment)

Tests

  1. Open the app and search for "Chronos" (or any 1:1 DM report whose personal details may load after options are initialized)
  2. Verify the search result displays the correct name (e.g., "Chronos") instead of "Chat Report"
  3. Click on the result and verify the chat header also shows the correct name
  4. Verify that no errors appear in the JS console
  • Verify that no errors appear in the JS console

Offline tests

  1. Search for "Chronos" while online and verify the correct name appears
  2. Go offline and verify search results still display the cached correct name
  3. The Chronos name is resolved from a constant, so offline behavior is unaffected

QA Steps

  1. Log in to the app on staging
  2. Search for "Chronos" in the search bar
  3. Verify the search result shows "Chronos" and not "Chat Report"
  4. Click on the Chronos result and verify the chat header shows "Chronos"
  5. Navigate back and search again to confirm the name remains correct
  • 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
  • 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 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

N/A - Logic-only change, no UI modifications

Android: mWeb Chrome

N/A - Logic-only change, no UI modifications

iOS: Native

N/A - Logic-only change, no UI modifications

iOS: mWeb Safari

N/A - Logic-only change, no UI modifications

MacOS: Chrome / Safari

N/A - Logic-only change, no UI modifications

Screenshots/Videosundefined

Add a dedicated effect in OptionsListContextProvider that watches
reportAttributes.reports and updates option display names (text) when
the derived value provides a name that differs from what is currently
stored. This fixes the timing gap where options can be initialized
before derived report names are computed, causing reports like Chronos
to show "Chat Report" instead of their correct name in search results.

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot MelvinBot requested a review from a team March 6, 2026 11:15
@MelvinBot
Copy link
Contributor Author

I created this PR from an upstream branch since I don't have push access to your fork.

To take ownership of this branch and be able to push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-fixChronosDisplayNameInSearch
git checkout -b claude-fixChronosDisplayNameInSearch upstream/claude-fixChronosDisplayNameInSearch
git push -u origin claude-fixChronosDisplayNameInSearch

Then you can close this PR and open a new one from your fork.

@codecov
Copy link

codecov bot commented Mar 6, 2026

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 93.60% <ø> (ø)
src/libs/ReportNameUtils.ts 84.05% <100.00%> (-0.05%) ⬇️
src/libs/PersonalDetailsUtils.ts 80.72% <50.00%> (-0.38%) ⬇️
src/libs/ReportUtils.ts 78.02% <0.00%> (+0.14%) ⬆️
... and 141 files with indirect coverage changes

@rojiphil
Copy link
Contributor

rojiphil commented Mar 9, 2026

@MelvinBot Well! It is noticed that the effect used for updating the options only for changed reports is already recomputing the report name correctly. So, it does not make sense to add another effect for or updating option display names when reportAttributes provides a name different than what's currently stored. Looks like the alternative solutions are the best way to move forward here. What do you think?

@MelvinBot
Copy link
Contributor Author

You make a good point. Looking at the existing changedReportsEntries effect (lines 119-150), it already calls processReport with reportAttributes?.reports and has it in the dependency array. While it does return early when there are no changed report keys (line 127), the real issue is that the dedicated effect I added is treating the symptom (timing gap) rather than the root cause (Chronos lacking a fallback display name like Concierge has).

I agree the alternative solution — mirroring the Concierge pattern for Chronos — is the better approach. It's simpler, avoids an extra effect that iterates all options on every reportAttributes change, and directly addresses why Chronos resolves to "Chat Report" when personal details haven't loaded yet.

The changes would be:

  1. Add CHRONOS_DISPLAY_NAME: 'Chronos' in src/CONST/index.ts alongside CONCIERGE_DISPLAY_NAME
  2. Add a chatIncludesChronos check after the existing isConciergeChatReport check in computeReportName (src/libs/ReportNameUtils.ts)
  3. Add the same check in the deprecated getReportName (src/libs/ReportUtils.ts) for consistency

This matches the pattern already established for Concierge and fixes the issue regardless of personal details loading timing. I can update this PR with that approach if you'd like — just reply with @MelvinBot implement this.

@rojiphil
Copy link
Contributor

rojiphil commented Mar 9, 2026

@MelvinBot implement this

Mirror the existing Concierge pattern by adding CHRONOS_DISPLAY_NAME
and a chatIncludesChronos check in both computeReportName and
getReportName. This ensures Chronos always resolves to its correct
display name regardless of personal details loading timing, instead
of relying on a dedicated effect in OptionsListContextProvider.

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot
Copy link
Contributor Author

Updated the PR to use the alternative approach as discussed. Here's what changed:

Removed:

  • The dedicated useEffect in OptionListContextProvider that watched reportAttributes.reports for name mismatches (the original approach)

Added:

  • CHRONOS_DISPLAY_NAME: 'Chronos' constant in src/CONST/index.ts (mirrors CONCIERGE_DISPLAY_NAME)
  • chatIncludesChronos check in computeReportName (src/libs/ReportNameUtils.ts) — used by derived report attributes and search
  • Same check in the deprecated getReportName (src/libs/ReportUtils.ts) — used by the chat header

This mirrors the existing Concierge pattern exactly: when the report includes Chronos as a participant, the display name is set to "Chronos" immediately without depending on personal details being loaded first.

@MelvinBot MelvinBot changed the title Sync option display names with derived reportAttributes Add Chronos display name constant to fix search showing Chat Report Mar 9, 2026
Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot
Copy link
Contributor Author

Fixed the failing Prettier check: the chatIncludesChronos import in src/libs/ReportNameUtils.ts was not in alphabetical order. Moved it to the correct sorted position.

Use else-if so the Chronos check doesn't overwrite the Concierge name
when both account IDs resolve to the same value (e.g. in dev/test envs).

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot
Copy link
Contributor Author

Fixed the failing test (job 2) check. Two tests in ReportUtilsTest.ts were failing because the Chronos display name check was overwriting the Concierge display name.

Root cause: In the .env file used during testing, both EXPENSIFY_ACCOUNT_ID_CHRONOS and EXPENSIFY_ACCOUNT_ID_CONCIERGE are set to -1. Since the chatIncludesChronos check ran unconditionally after isConciergeChatReport, it matched the same participants and overwrote formattedName from "Concierge" to "Chronos".

Fix: Changed the independent if to else if in both ReportUtils.ts and ReportNameUtils.ts, so the Chronos check only runs when the report is not already identified as a Concierge chat.

@rojiphil
Copy link
Contributor

@MelvinBot Please resolve the conflicts

MelvinBot and others added 2 commits March 19, 2026 03:42
…layNameInSearch

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot
Copy link
Contributor Author

Resolved the merge conflicts by merging main into the branch and re-applying the Chronos display name changes on top of the updated code. The three modified files (src/CONST/index.ts, src/libs/ReportNameUtils.ts, src/libs/ReportUtils.ts) have been updated cleanly.

These patches (for sentry 10.24.0 and react-native 7.6.0) were
superseded by updated versions on main but remained as merge
artifacts. Removing them fixes the validate-patches CI check.

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot
Copy link
Contributor Author

Fixed the failing validate-patches check: removed stale old-version sentry patch files (@sentry/browser@10.24.0, @sentry/core@10.24.0, @sentry/react-native@7.6.0) that were superseded on main but remained as merge artifacts on this branch. The details.md files only reference the current versions (10.39.0, 8.2.0), so the old patches were causing the validation to fail.

The branch was based on an older version of main that had files since
removed. After merging with current main, these 106 stale files were
incorrectly retained as merge artifacts. Removing them to fix the
Changed files ESLint check, which was flagging ESLint errors in these
unrelated files.

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot
Copy link
Contributor Author

Fixed the failing Changed files ESLint check: The branch was carrying 106 stale files from the old branch base that no longer exist on main. These were incorrectly retained as merge artifacts when the branch was merged with main, causing ESLint to flag errors in unrelated files. Removed all stale files so the PR now only contains the 3 intended Chronos fix changes (src/CONST/index.ts, src/libs/ReportNameUtils.ts, src/libs/ReportUtils.ts).

@OSBotify
Copy link
Contributor

A preview of your ExpensifyHelp changes have been deployed to https://df3495e0.helpdot.pages.dev ⚡️

@MelvinBot
Copy link
Contributor Author

The previously failing checks (TypeScript, ESLint, Changed files ESLint) were caused by stale files from the old branch base that no longer exist on main. These were cleaned up in the latest commits (merge with main + removal of stale files). All checks are now passing:

  • TypeScript Checks: ✅
  • ESLint check: ✅
  • Changed files ESLint check: ✅
  • Jest Unit Tests: ✅
  • Reassure Performance Tests: ✅
  • Remote Build Android: ✅
  • Remote Build iOS: ✅
  • All other checks: ✅

@rojiphil
Copy link
Contributor

rojiphil commented Mar 19, 2026

@MelvinBot, The approach of mirroring the Concierge pattern for Chronos in computeReportName still does not help. Although the derived value is updated in Onyx via computeReportName, this does not trigger a rerender of the search auto-complete list. What is the best approach to trigger a rerender without affecting performance?

@MelvinBot
Copy link
Contributor Author

I have failed to help you. I let you down and for that I am sorry. Please try again later. This may be due to a Claude outage (check here).

Failure details: https://github.com/Expensify/Expensify/actions/runs/23304205060

@rojiphil
Copy link
Contributor

@MelvinBot Please re-attempt addressing the comment here

Mirror the Concierge pattern: resolve Chronos display name at the
personal details level so OptionListContextProvider picks up the
correct name via its personal details change effect, triggering a
rerender of the search auto-complete list.

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
@MelvinBot
Copy link
Contributor Author

I investigated the rerender issue and found the root cause.

Problem: OptionListContextProvider includes reportAttributes?.reports in its effect dependencies, but the "changed reports" effect only processes entries from changedReportsEntries — which is derived from actual report Onyx key changes, NOT from reportAttributes changes. So when computeReportName updates the derived value to "Chronos", there are no "changed report keys" to process, and the effect returns the previous (stale) options unchanged.

Why Concierge works: Concierge has an additional check in PersonalDetailsUtils.getDisplayNameOrDefault() (line 88-89) that overrides the display name based on account ID. This means the name resolves correctly at the personal details level, and OptionListContextProvider's personal details effect (which DOES trigger rerenders) picks it up.

Fix: Added the same pattern for Chronos in PersonalDetailsUtils.getDisplayNameOrDefault():

if (passedPersonalDetails?.accountID === CONST.ACCOUNT_ID.CHRONOS) {
    displayName = CONST.CHRONOS_DISPLAY_NAME;
}

This ensures "Chronos" resolves at the personal details level — just like Concierge — so OptionListContextProvider picks up the correct name via its personal details change effect, triggering the search auto-complete list to rerender with the correct name.

@MelvinBot
Copy link
Contributor Author

The failing check (perf-tests) is unrelated to this PR's changes.

Analysis: The perf test useAdvancedSearchFilters hook initial render with policies failed with a render count difference of 0.8 (allowed deviation: 0). This hook depends only on policies, categories, tags, cards, and search filter form data — it does not use any report name functions (computeReportName, getReportName) or PersonalDetailsUtils.getDisplayNameOrDefault, which are the only code paths this PR modifies.

Evidence:

  • Files changed in PR: src/CONST/index.ts, src/libs/PersonalDetailsUtils.ts, src/libs/ReportNameUtils.ts, src/libs/ReportUtils.ts
  • Failing test's dependencies: useOnyx for POLICY, POLICY_CATEGORIES, POLICY_TAGS, SEARCH_ADVANCED_FILTERS_FORM, PERSONAL_AND_WORKSPACE_CARD_LIST, SESSION — none overlap with this PR's changes
  • Render count difference is fractional (0.8), indicating non-deterministic behavior across test iterations — this is statistical noise, not a regression caused by the PR

This appears to be a flaky perf test. Re-running the checks should resolve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants