Skip to content

[No QA] Add IncludesProductChange labeling - #102560

Closed
tgolen wants to merge 6 commits into
mainfrom
ci/includes-product-change
Closed

tgolen wants to merge 6 commits into
mainfrom
ci/includes-product-change

Conversation

@tgolen

@tgolen tgolen commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

Automatically adds IncludesProductChange to non-draft App PRs when an AI classifier finds product/UI changes or cannot confidently rule them out. Includes add-only behavior, dry-run/backfill support, and classification summaries for the trial.

Fixed Issues

For https://github.com/Expensify/Expensify/issues/686684
PROPOSAL: N/A — internal implementation.

Tests

Reviewer checks — no local setup or API key required

  1. Open this PR's Checks tab.
  2. Open Bun tests and confirm it passes. This runs the classifier/action tests with fake GitHub and model responses, covering product changes, refactors, drafts, existing labels, invalid responses, and failures.
  3. Open Validate Github Actions and confirm its verify job passes. This checks that the committed action bundles match their source.
  4. Confirm Validate Github Actions and Workflows, ESLint check, Oxfmt check, and Spell check pass.
  5. Confirm TypeScript Checks passes.

Optional local automated tests — no API key required

  1. Open a terminal in an App checkout of ci/includes-product-change with the repository's Node, npm, Bun, and dependencies installed.
  2. Enter npm run test:bun and press Return. Confirm all tests pass.
  3. Enter npm run lint-changed, npm run typecheck, and npm run spell-changed, one command at a time.
  4. From a clean checkout, enter .github/scripts/verifyActions.sh and confirm the generated actions match their sources.

Real AI accuracy evaluation — after merge

The automated tests above use mocked model responses. They do not establish how accurately the real model classifies PRs.

After merge, run the existing workflow from GitHub's Actions tab using the steps below. GitHub supplies OPENAI_APP_REVIEWER_API_KEY automatically; no local API key setup is needed.

Automatic add-only labeling starts when this workflow is merged. Evaluate the prompt with historical PRs in dry-run mode after merge, and complete that evaluation before backfilling existing open PRs. Real-model testing is not a pre-merge requirement.

  • Verify that no errors appear in the JS console — N/A: GitHub automation only.

AI Tests

  • Passed: all 636 Bun tests; 58 tests across the classifier/action and OpenAI utility suites.
  • Passed locally: full TypeScript check, changed-file lint, spelling, formatting, and generated-action verification.
  • Passed: TypeScript CI after synchronizing with the upstream report-lookup fix; successful CI run.
  • Passed after the main merge: 375 tests across SearchAutocompleteList, DeferredSearchAutocompleteList, and OptionsListUtils suites, plus React Compiler checks.
  • Passed: compiled-action, read-only dry runs against [No QA] Update PERFORMANCE.md #102450 and [No QA] Add files via upload #102499, exercising missing-key and binary-patch fallbacks.
  • Planned after merge: real-model accuracy evaluation through the workflow's dry-run mode using the existing repository secret. The earlier local dry runs only validated missing-key and binary-patch fallbacks; reviewers do not need to reproduce those commands.
  • Pending: live GitHub event delivery and label-write checks after deployment, including fork and bot PRs. Rollout and weekly-observation steps are listed below.

Offline tests

  1. Run npm run test:bun -- -t 'product-change|OpenAIUtils'.
  2. Confirm mocked API/model errors, missing credentials, and invalid output take the conservative fallback.
  3. Confirm mocked label-write failures fail the action rather than report successful labeling.

Application offline behavior: N/A.

The action uses the existing OpenAI helper's request/retry behavior and a 10-minute workflow limit. A job stopped by that limit cannot apply the fallback and must be retried.

QA Steps

[No QA] — tooling-only change. Engineer rollout verification:

  1. After this PR is merged, open the Actions tab of Expensify/App.
  2. Select Label product changes and click Run workflow.
  3. Select main, enter 102450 as the PR number, and keep dry_run enabled.
  4. Click Run workflow to start the test.
  5. Open the completed run and find Product-change classification in its summary. Confirm it reports a verdict and would_add_label or would_skip_label, with no fallback field. GitHub supplies the model key automatically; dry runs do not change labels.
  6. Run each PR in the QA step 6 evaluation table below with dry_run=true. Compare the summary's head/base SHA prefixes with the recorded revision, then fill in the actual verdict/action, workflow-run link, and notes. Investigate missed product changes and record fallbacks separately. Closed and already-labeled PRs can be evaluated; drafts are skipped.
  7. Open an eligible non-draft PR and inspect the automatic classification summary and label.
  8. Open a draft PR, mark it ready, and confirm classification begins when it becomes ready.
  9. Push a product-changing commit to an unlabeled PR and confirm classification uses the full updated PR.
  10. Confirm an already-labeled PR preserves the label and skips inference.
  11. Verify fork and bot PR event delivery. Inspect failed or missing runs; a workflow that never starts cannot apply the conservative fallback.
  12. After the sample evaluation, list existing open non-draft PRs without the label, then dispatch each selected PR with dry_run=false to backfill:
gh api --paginate 'repos/Expensify/App/pulls?state=open&per_page=100' \
    --jq '.[] | select(.draft == false) | select(all(.labels[]; .name != "IncludesProductChange")) | .number'

gh workflow run labelProductChanges.yml --repo Expensify/App \
    -f pull_request_number=PR_NUMBER -f dry_run=false
  1. After the separate checklist-grouping PR ships, inspect the first generated checklist for correct grouping and preserved checkmarks.
  2. For 2–3 weeks, post a short weekly update in the tracking issue. Sample unlabeled PRs for missed changes, sample labeled refactors/tooling for excessive labeling, compare against current product-review assignments, and report workflow failures and fallbacks separately. Include examples and recommend prompt changes, continued observation, or the reviewer-assignment follow-up.

Treat current reviewer assignments as a comparison point, not ground truth. The later reviewer-assignment change must distinguish pending classification from confirmed non-product results.

QA step 6 evaluation table

Expected results were prepared from diff review on 2026-09-29, before running the classifier. All 24 samples are merged PRs. These are review expectations, not observed model results; the final three columns are intentionally blank.

The sample contains 13 product changes, 9 clearly non-product changes, 1 uncertain dependency change, and 1 expected binary-file fallback. The classifier's input checks accepted the 23 text-based samples. The binary sample failed the readable-patch check as expected. No OpenAI calls were made for this preparation.

How to fill this in: copy verdict and action from the workflow summary, link the run, and record its explanation or fallback in Notes. The revision column lists 12-character head / base SHA prefixes; if either differs from the run summary, re-review that sample before scoring it. Record the workflow revision/model when comparing repeated evaluation rounds.

How to judge results: product_change and unsure both mean add (would_add_label); clearly_non_product means skip (would_skip_label). An unsure result on a product-change row still catches the change, but record the uncertainty. Adding the label to a clearly non-product row is a conservative false positive. Treat model/API failures as operational fallbacks, not accurate classifications. The final binary row tests fallback behavior and should be excluded from model-accuracy counts.

PR / case Reviewed head / base Expected verdict / label Reason from diff review Actual verdict / action Workflow run Notes / fallback
102450 — Performance bounty documentation 0b0ac65b09a3 / 53e6747c08be clearly_non_product / skip Changes the contributor bounty amount in contributingGuides/PERFORMANCE.md only.
102342 — Contributor+ compensation documentation a454b38fb03f / a9709e2c8906 clearly_non_product / skip Updates a payment example in contributor guidance; no application code changes.
102085 — Contributor payment documentation 3c82e5cd3c2e / ea01dd159fdf clearly_non_product / skip Adds bank-account guidance to contributor documentation only.
102445 — Delete disabled Codex review workflow e1ec2cfc647d / 9c680bceb940 clearly_non_product / skip Removes GitHub code-review automation, not product behavior.
102401 — Report-details performance test 04da0cf741dd / e72bd3189130 clearly_non_product / skip Adds one performance-test file; no production implementation changes.
102334 — Fix test fixtures after a refactor 65f22b9a5344 / d2911302606d clearly_non_product / skip Changes test arguments and provider wrappers in three test files only.
102136 — Fix test timestamp race c777990b8bd1 / 87618c248648 clearly_non_product / skip Makes timestamps deterministic inside ReportTest.ts; no runtime behavior change.
102361 — Pass translation functions explicitly 243a6609df47 / af4ed367c1bb clearly_non_product / skip Replaces implicit translateLocal calls with supplied translation functions, preserving translation keys and naming logic.
102398 — Extract transaction helper functions ff3bbaf3526f / e72bd3189130 clearly_non_product / skip Moves helpers into separate modules and re-exports them. All five moved function declarations have identical syntax trees after ignoring comments/locations.
102364 — Onyx refactor with a translation behavior change f84dd6b71532 / af4ed367c1bb product_change / add Also removes the Expensify-account-specific MISSING TRANSLATION output, returning the key string instead. The refactor title and NoQA marker must not hide this behavior change.
102501 — Tag GL-code lock icon 74ddd09efb75 / 68ad1ed0e2bf product_change / add Changes whether the tag settings field shows its right-hand icon with an accounting connection.
102486 — Accounting tiles after rotation 47f83ef02111 / 1ec666a7cb7a product_change / add Changes responsive tile sizing and landscape handling in onboarding.
102426 — Report-row border overlap f32dc5ba2d67 / 5badaa68111d product_change / add Keeps a transparent last-row border to preserve row height and prevent visible overlap.
102375 — Search footer loading flicker f8da012d973b / c6f374078238 product_change / add Changes loading-state updates for repeated searches; regression tests accompany a runtime fix.
102374 — Export-failed reports in Ready To Pay 93856becd417 / c6f374078238 product_change / add Changes which reports appear in the pay to-do list, with a regression test.
102344 — Complete onboarding while offline 1754bf462c0a / a9709e2c8906 product_change / add Queues guided-setup writes when offline instead of using a non-retried side-effect request.
102322 — Card-feed OAuth reconnect 64f1d2008ac8 / 7041ea0f69f5 product_change / add Passes the selected feed to the reconnect callback. Changes product behavior despite the NON-UI title.
102317 — Move an undeleted expense ca06bd260544 / bd73f8fbb300 product_change / add Prefers the live IOU action so restored expenses and their threads move correctly.
102314 — Remove spend-rule supporting text db5d38a0d90c / bd73f8fbb300 product_change / add Removes rendered helper text and its translations from card-spend rule forms.
102198 — Stop Save spinning after validation failure 97ef95d49b86 / 1bb7b7e7d337 product_change / add Changes Save-button loading behavior in two rule forms.
102195 — Preserve recent-search history 78ac577362dd / bfa20a3e0d31 product_change / add Explicitly sends shouldSaveRecentSearch=false so background searches do not overwrite user history.
102134 — Revert workspace-selector changes 82bd30717c69 / d23683be1245 product_change / add Changes report-creation workspace selection and preferred-workspace handling. Reverts still change the product.
102417 — websocket-driver dependency update 1a323e8d3b1b / 5badaa68111d unsure / add The lockfile only changes a dependency version; the diff does not establish that dependency behavior is unchanged.
102499 — Binary help-site image 516ae6a8ccd3 / 68ad1ed0e2bf unsure / add via fallback Adds docs/assets/images/wallet-test-transactions.png without a readable patch. Expect fallback: A changed file has no readable patch. before model inference; this is not a product-change accuracy claim.
  • Verify that no errors appear in the JS console — N/A.

PR Author Checklist

Application/platform-specific items below are N/A for this tooling-only change. Live model and event verification are planned after merge.

  • 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 turned off my network connection and tested it while offline to ensure it matches the expected behavior — N/A
    • I tested this PR with a High Traffic account against the staging or production API — N/A
  • I included screenshots or videos for tests on all platforms — N/A
  • I ran the tests on all platforms and verified they passed on:
    • Android: Native — N/A
    • Android: mWeb Chrome — N/A
    • iOS: Native — N/A
    • iOS: mWeb Safari — N/A
    • MacOS: Chrome / Safari — N/A
  • I verified there are no console errors — N/A
  • I followed proper code patterns
    • I verified that comments were added to code that is not self explanatory
    • I verified that new or modified comments are clear and explain the relevant reasoning
    • I verified any copy / text added to the app is grammatically correct and approved — N/A
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers — N/A
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes — the full tooling suite passes
  • If a new CSS style is added I verified that:
    • A similar style does not already exist — N/A
    • The style cannot be created with an existing StyleUtils — N/A
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed — N/A
    • The assets load correctly across all supported platforms — N/A
  • If the PR modifies code that runs when editing or sending messages, I tested all supported markdown — N/A
  • If the PR modifies a generic component, I tested other usages — N/A
  • If the PR modifies a component related to Storybook stories, I tested those stories — N/A
  • If the PR modifies a component or page accessible by a direct deeplink, I tested logged-in and logged-out states — N/A
  • If the PR modifies the UI or form input styles:
    • I verified form inputs are aligned — N/A
    • I added the Design label or tagged the design team — N/A
  • I added unit tests for the new behavior
  • If main was merged after a review, I retested the changes — N/A

Screenshots/Videos

Android: Native

N/A — GitHub automation only.

Android: mWeb Chrome

N/A — GitHub automation only.

iOS: Native

N/A — GitHub automation only.

iOS: mWeb Safari

N/A — GitHub automation only.

MacOS: Chrome / Safari

N/A — GitHub automation only.

- Tim's A.I. Agent 🤖

@tgolen
tgolen requested a review from AndrewGable September 29, 2026 17:12
@tgolen
tgolen marked this pull request as ready for review September 29, 2026 17:12
@tgolen
tgolen requested a review from a team as a code owner September 29, 2026 17:12
@melvin-bot
melvin-bot Bot requested review from grgia and removed request for a team September 29, 2026 17:12
@melvin-bot

melvin-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown

@grgia 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]

@tgolen

tgolen commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

@AndrewGable I know the tests look... verbose... but it basically just amounts to merging this PR, testing it in dry run mode on a couple of dozen PRs, and verifying the results.

@tgolen tgolen added the InternalQA This pull request required internal QA label Sep 29, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3ced15e71

ℹ️ 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".

Comment on lines +41 to +47
input,
instructions: PRODUCT_CHANGE_INSTRUCTIONS,
model: PRODUCT_CHANGE_MODEL,
textFormat: PRODUCT_CHANGE_RESPONSE_FORMAT,
promptCacheKey: 'includes-product-change-v1',
});
const result = openAI.parseJSONResponse(response.text, isProductChangeClassification);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound the model request below the job timeout

When the Responses API stalls, this request can consume the entire workflow budget: the bundled OpenAI client defaults to a 600,000 ms request timeout, exactly matching labelProductChanges.yml's timeout-minutes: 10. GitHub then terminates the job before classify() can catch the failure and return unsure, so the promised conservative fallback never adds IncludesProductChange for that PR. Pass a shorter abort signal/timeout to the model call so the fallback can complete before the job deadline.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really think this is necessary as that is a pretty long edge case.

@tgolen
tgolen requested a review from a team as a code owner September 29, 2026 17:19
@melvin-bot
melvin-bot Bot requested review from JmillsExpensify and removed request for a team September 29, 2026 17:19
@tgolen
tgolen marked this pull request as draft September 29, 2026 19:58
@tgolen

tgolen commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

After talking with Andrew, we might go a different direction with this, so I'm going to close this for now.

@tgolen tgolen closed this Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

InternalQA This pull request required internal QA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant