Skip to content

Make CI actually run, and cover L2/L3 nightly - #161

Merged
aldro61 merged 2 commits into
mainfrom
ci/compositional-coverage
Sep 23, 2026
Merged

aldro61 merged 2 commits into
mainfrom
ci/compositional-coverage

Conversation

@aldro61

@aldro61 aldro61 commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #160 (it needs that fix for the L2 nightly tests to pass). Retarget to main once #160 merges.

Summary

Per-PR CI has been dead for months. Every unit_tests.yml run on main since at least March 2026 failed, for three independent reasons:

  • It expects SNOW_INSTANCE_URL/UNAME/PWD secrets that no longer exist. Undefined secrets resolve to '', so tests failed with Invalid URL ''.
  • It has no HUGGING_FACE_HUB_TOKEN, so the tests that load the instance pool errored at collection with a 401.
  • black flagged src/wa_action_traces.py.

In practice the "fast" job ran about 16 tests in 8 seconds, and L2/L3 compositional tasks never ran in CI at all.

This PR:

  • Per-PR (unit_tests.yml): uses the instance pool through the existing HF token and runs only the fast suite: 42 tests, about a minute. The slow (833 selected, about 265 not permanently skipped) and weekly end-to-end (682) jobs are removed. Revived as-is, they'd take roughly 45 minutes per PR and about 4 hours per week, so they move to the nightly shard below. Nothing is lost relative to today, since neither job has run in months.

  • Nightly shard (instance_pool_ci.yml): the slow and compositional tests, one seventh per weekday (about 105 runnable tests at -n 20), so everything runs once a week. The seed rotates daily instead of always being 0. The shard and seed are written to the job summary so a failure can be reproduced exactly. The job runs after the L1 jobs rather than alongside them, to spread load on the shared pool. L1 (test_task_general.py) is excluded because it already runs in full nightly. Supported by WORKARENA_TEST_SHARD (new tests/conftest.py) and WORKARENA_TEST_SEED (test_compositional.py).

  • Pool health check (tests/test_instance_preconditions.py, nightly, API-only, about 8s): checks on every instance the catalog preconditions the task code relies on, instead of trying to model the whole instance. Each rule is as strict as the code it guards:

    • exactly one active Hardware item contains each name the order tasks click via h2:has-text (a case-insensitive substring match);
    • each name looked up with a sys_name= Table API query (case-insensitive) resolves to exactly one item;
    • each name that request-item validation compares with != (case-sensitive) exists with that exact sys_name.

    Names are read from the task classes themselves, so the check follows the code. To make that possible, CATALOG_ITEM_SYS_NAMES in dash_do_base.py replaces two identical local copies of the same dict. This is behavior-preserving.

    It also compares each instance's catalog against the rest of the pool: items missing or extra relative to the majority, and content fields (name, sys_name, active, category, price, description, catalogs, order) that disagree with the majority. Each problem is reported with the user who last updated the record. sys_updated_on and similar fields churn legitimately and aren't compared. Drift shared by most instances goes unnoticed by design.

  • Retries: test_cheat_compositional now retries only on TimeoutError, as its log message always claimed, and emits a warning on each retry so flakiness shows up even when a test passes. Retrying on any exception is what hid Bind L2 compositional subtasks to the parent's instance #160's bug.

  • Two tests fixed: they silently used several random pool instances at once. SNowInstance replaces credentials passed without a URL. See below.

Findings from this work

  • The health check found live contaminations on its first runs, all traced to agent-generated users:

    • workarenapublic22: Apple iPhone 13 pro was renamed to Apple iPhone 13 Pro on 2026-09-10 by Jessica.Rodriguez.8270, along with its short_description. The …RequestAppleIphone13proTask validation compares with !=, so those tasks failed whenever they landed there. Repaired in place; renaming it back also resynced sys_name.
    • workarenapublic22 and 23: Windows Surface Pro 4 is active while 4/6 instances have it inactive, set by Nicole.Shaw.6191 and Alexander.James.6191. The same …6191 suffix as Anna.Weaver.6191, who activated Apple Watch Series 2 on instance 20: both items are inactive items that the request tasks ask agents to order. Instance since removed from the pool.
    • workarenapublic20: two blank-named catalog items that exist on no other instance, created on 2026-07-28 by Marc.Nash.7331. Instance since removed from the pool.

    Screening also found 40–50K orphaned task users on each of 20/22/23, plus the data they created (89% of task records on instance 20). So 20/22/23 were removed from the pool to be reprovisioned rather than cleaned. On the current pool (25/26/27) the health check passes 87/87.

  • Replaying the two earlier contaminations (Apple Watch Series 2 active on instance 20; Loaner Laptop sys_name resynced on 19) through the same checks flags each with one precise failure, and a healthy catalog passes 28/28.

  • SNowInstance footgun: passing snow_credentials without snow_url silently replaces the credentials with an env or pool instance's admin credentials. Library code is safe (the installer's wrapper always passes both), but it's easy to misuse. Consider raising instead; that's left for a separate PR.

  • The 72 infeasible dashboard-catalog tasks are neither registered in the benchmark nor tested. FYI in case that's unintended.

Test plan

  • The exact per-PR command, run locally unpinned: 42/42 pass (68s). Before the two test fixes it was 39/42. The fixed tests passed 3 more repeats.
  • Sharding: 1317 selected tests split into 7 disjoint shards of 188–189 that together cover everything; a malformed spec fails with a clear error.
  • The seed shows up in test ids; TimeoutError is retried with a warning, while IndexError fails on the first attempt.
  • Pool health check live: 171 pass, 3 fail. The failures are the three unrepaired cases above, with no other differences across 190 shared catalog items.
  • Cross-instance comparison on synthetic pools: identical pool passes, a deleted item is reported as missing, and a 2-vs-2 split is flagged.
  • Refactor: the 8 L2 dashboard-catalog tasks that exercise CATALOG_ITEM_SYS_NAMES (including the Loaner Laptop mapping) pass.
  • black . --check with CI's pinned black==24.2.0 is clean.
  • The nightly jobs can't be exercised locally. The first scheduled run, or a manual workflow_dispatch, will confirm the workflow wiring. This PR's own unit_tests.yml run exercises the new per-PR job.

🤖 Generated with Claude Code

aldro61 and others added 2 commits September 23, 2026 14:25
unit_tests.yml had failed on main for months: it expected SNOW_INSTANCE_*
secrets that no longer exist, lacked the HF token the instance pool needs, and
black flagged src/wa_action_traces.py. PRs now run only the fast suite against
the pool (42 tests, about a minute); the slow and weekly end-to-end jobs, which
would otherwise run for hours, move to a nightly shard.

instance_pool_ci.yml gains:
- a nightly shard of the slow and compositional tests: one seventh per weekday
  so the full suite runs weekly, with the seed rotated daily and reported in the
  job summary (tests/conftest.py adds WORKARENA_TEST_SHARD, and
  test_compositional.py reads WORKARENA_TEST_SEED)
- a pool health check (tests/test_instance_preconditions.py) that verifies, on
  every instance, the catalog preconditions the task code relies on. It reads
  the item names from the task classes; CATALOG_ITEM_SYS_NAMES in dash_do_base.py
  replaces two identical local copies so it can be imported.

test_cheat_compositional now retries only on TimeoutError, as its log message
always claimed, and warns on each retry so flakiness is visible. Retrying on any
exception hid the L2 instance-binding bug.

Also fix two tests that silently used several random pool instances at once:
SNowInstance replaces credentials passed without a URL.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds test_catalog_matches_other_instances to the nightly pool health check:
for every instance, flag catalog items that are missing or extra relative to
the majority of the pool, and content fields (name, sys_name, active,
category, price, short_description, sc_catalogs, order) that disagree with the
majority, reporting who last updated each record. sys_updated_on and similar
fields churn legitimately and are not compared. Drift shared by most instances
goes unnoticed by design.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Base automatically changed from fix/l2-subtask-instance-binding to main September 23, 2026 20:18
@aldro61
aldro61 merged commit 87a9e61 into main Sep 23, 2026
6 checks passed
@aldro61
aldro61 deleted the ci/compositional-coverage branch September 23, 2026 20:20
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.

1 participant