# Codex prompt — review both authors' work, then fix the open list

Paste everything below the line into Codex. Unlike the walkthrough prompt, this
one **may change code**. It reviews work by two authors — Claude and a previous
Codex pass — and neither is presumed right.

---

You are reviewing a feature branch written by two agents and then fixing an
agreed list. Read the diff AND use the product; a claim that only survives on
paper has not survived.

## Environment

- Repo: `~/Documents/GitHub/petav3-dev-chen-integration`, branch `dev-chen`
- Baseline for the whole feature: `69f0a123`. Everything after it is under review.
- Site: `https://petav3.test` (Herd, self-signed cert — `curl -k`)
- PHP: **always** `herd php ...`. Terminal default is 7.4 and will fail.
- Dev login: `POST /auth/dev-login` with `_token` (CSRF meta tag), `email`,
  `portal=manage`. Accepts any ACTIVE user.
  - `demo.sales@propertylab.test` — sales agent
  - `demo.admin@propertylab.test` — super admin
- Re-seed demo data:
  `herd php artisan tinker --execute="require '/private/tmp/claude-501/-Users-dadadineiyou/7e06f09f-d625-4736-8030-a7c1ea8456c9/scratchpad/seed_acceptance.php';"`

### Hard constraints — each has already cost this branch real time

- **Never** `migrate:fresh` / `db:wipe` / `migrate --force` against the dev
  database. `.env` points at `petav3_preview_20260722` and there is **no
  `.env.testing`**, so `--env=testing` silently falls back to it. It has already
  been wiped once this way. Name the database explicitly instead:
  `DB_DATABASE=petav3_scratch herd php artisan ...`
- **Use your own test database.** Another agent works in this repo at the same
  time, and two `artisan test` runs against one schema both `migrate:fresh` it
  and corrupt each other — that has already produced two full-suite results
  worth nothing. Do not coordinate by timing; isolate instead:

  ```bash
  herd php artisan tinker --execute="DB::statement('CREATE DATABASE IF NOT EXISTS petav3_codex_testing CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci');"
  sed 's|value="petav3_testing"|value="petav3_codex_testing"|g' phpunit.xml > phpunit.codex.xml
  herd php vendor/bin/phpunit --configuration=phpunit.codex.xml
  ```

  The name **must end in `_testing`** — `tests/Support/TestDatabaseGuard.php`
  refuses to boot otherwise. Use `vendor/bin/phpunit` directly, not
  `artisan test --configuration`, which rejects a second `--configuration`.
  Delete `phpunit.codex.xml` when you are done and never commit it.
- **Never `pkill -f phpunit`.** Another agent's suite is very likely running and
  that command kills it, silently destroying a 12-minute measurement. Kill only
  processes you started, by PID.
- `./vendor/bin/pint` with no path reformats ~200 unrelated files. Always pass
  explicit paths.
- Do not touch **Lead Discussion → Action Items**
  (`app/Http/Controllers/Manage/Leads/`). Deliberately out of scope.
- `ai_credentials` is empty in this environment, so anything calling a provider
  fails. That is configuration, not a defect. Do not add keys.

### A decision that is settled — do not re-open it

`/manage/zoom/actions` and `/manage/action-items` both being called "Action
Items" is **known and accepted**. A previous pass renamed the Zoom one to
"Meeting Follow-ups"; it was reverted (`8752b288`), because Calls and F2f each
have their own "Action Items" tab too, so renaming one channel makes the product
less consistent, not more. The grain is carried by where each number LINKS, not
by the page names. If you think this is wrong, say so in the report — do not
change it.

## What to fix (agreed, do these)

### 1. Provenance is dropped when a reviewer CLEARS a date

`BuildsActionPlanChecklist::toMyTaskArray()` and
`DashboardController::toTeamItemArray()` both compute:

```php
'scheduled_source_label' => $item->scheduled_for ? $this->scheduleSourceLabel(...) : null,
```

and the template guards on `item.scheduled_for && item.scheduled_source_label`.

A reviewer deleting the AI's suggested day is a **decision**: the repository
stores `scheduled_for = null` with `scheduled_source = SCHEDULE_SOURCE_HUMAN`,
and keeps "key absent" and "key null" apart specifically so that decision
survives. Both guards throw it away, so on the surface a salesperson reads,
"the AI never proposed a day" and "someone decided this needs no day" look
identical again.

Two failing tests are already written in `tests/Feature/Manage/DashboardChecklistTest.php`:

- `test_a_reviewer_clearing_the_date_is_still_recorded_as_their_decision`
- `test_a_step_nobody_ever_dated_claims_no_decision`

They are the specification. Make them pass on **both** My Tasks and Team Tasks,
and check how the label reads next to an "Unscheduled" badge — the wording
matters as much as the field.

### 2. Verify or refute: the fleet tile may double-count multi-assignee steps

`SalesWorkQueue::openStepsByAssignee()` JOINs the assignee pivot and groups by
`user_id`. A step assigned to two people therefore contributes 1 to each. AI
Employee's tile is `$rows->sum('open_steps')`, so that one step would be counted
twice — while the Zoom agent's `followThrough.open` counts distinct task rows.
The test asserting the two agree
(`CopilotAgentsPanelTest::test_it_agrees_with_the_zoom_agent_about_the_same_team`)
only uses single-assignee steps, so it would not catch this.

This is **an unverified suspicion, reasoned from the code, not observed**. Today
the plan-approval path assigns exactly one person per step (`assignee_ids` is a
one-element array) and the dev database has zero multi-assignee items — so it
may be unreachable in practice. Establish which it is: if reachable, add the
failing test first and then fix it; if genuinely unreachable, say what prevents
it and leave a test that would fail the day it becomes reachable.

### 3. Re-check the previous pass's own fixes under load

Commit `8dc8b2ff` fixed four things. Confirm each actually holds, rather than
trusting the commit message:

- `completeItem()` now adjusts the schedule counts locally. Does it stay correct
  when a step is completed from the **sign-in agenda** and from **Team Tasks**,
  not just from the workbench? Complete several in a row without reloading and
  compare the chips against a fresh page load.
- `offset` now validates and 422s. Confirm the real "Load more" button still
  works — a legitimate offset must not be rejected — and that `next_offset`
  round-trips.
- Team Tasks now renders `priority_source_label`. Check the compact/detail
  toggle: the rule is that a value and its provenance are shown together or
  hidden together, never one without the other.

## What to review (find what neither author caught)

Both authors' work is in scope and neither is presumed right. `git log --oneline
69f0a123..HEAD` shows who wrote what.

Attack these claims. They are the ones the feature stands on:

1. **One number per audience scope.** For a given viewer AND scope, the Hub, the
   Action Items workbench, the Zoom AI Agent and AI Employee report the same
   count of open follow-up steps, and completing one moves all of them. Note the
   scopes genuinely differ: a Super Admin's Hub is their OWN checklist while the
   Zoom agent shows the TEAM. Disagreement within one scope is a bug;
   disagreement between scopes is the design.
2. **Ordering.** Overdue → Today → Unscheduled → Upcoming, then priority, then a
   stable tie-break. Urgency outranks priority.
3. **Dates are confirmed, not guessed**, and who confirmed is always legible.
4. **Team Tasks is supervision, not execution** — no phone, no `wa.me`, no
   `tel:`, no call payload. `action_type_label: "WhatsApp"` is a step TYPE and is
   legitimate.
5. **Nothing claims more than it knows.** No conversion rates or uplift.
   Commission is Super-Admin-only and appears only after a human confirmed the
   pipeline match. `data_completeness` is a count with its parts, never a word.
6. **Value and provenance are never separated.**
7. **Filters narrow what is shown and say what they hid** ("1 matching · 4 open").
8. **A lead never straddles a page.**

Places worth probing that the last pass did not reach:

- The AI Employee panel's population now includes people who owe steps but
  hosted nothing. What does a row look like when every conversation column is
  zero — does the coaching flag or the "best agent" pick behave sensibly?
- `SalesWorkQueue::leadTotals()` runs an extra grouped query per page. Check the
  query count for a page of 20 leads.
- Timezone: the app runs `Asia/Kuala_Lumpur`. Anything that computes "today"
  must do it server-side. Look for any browser-side date arithmetic that crept
  back in.
- Soft-deleted and purged rows: a lead purged out from under an action item, an
  item soft-deleted mid-page.
- The `agenda` rendering path shares row markup with the workbench. Anything
  that renders in one and not the other, that should?

## How to report and commit

- Fix the agreed list above. Write the failing test **before** the fix, and say
  in the commit body what turns red if the fix is reverted.
- Anything you find beyond that list: **report it, do not fix it** unless it is
  a one-line correctness bug with an obvious fix. Scope creep in a review pass
  is how a branch becomes unreviewable.
- Run the **full** suites and quote the totals, not a filtered subset:
  `herd php artisan test` and `npx vitest run`. The branch has ~104 pre-existing
  PHP failures and 2 pre-existing Vitest failures (PropertyMatch) that are NOT
  yours — compare against `69f0a123` before attributing anything.
- Commit to local `dev-chen`. Do not push, do not open a PR.
- Pint only the paths you touched.

For every finding, say what you did, what you expected, what happened, which
claim it violates, and how sure you are. Separately list the claims you tried
hard to break and could not — so the next reader can tell a tested claim from an
assumed one.