ecw-charge-validation/docs/superpowers/specs/2026-09-16-condition-framework-design.md

# Condition Framework — Design

**Status:** Approved by user, 2026-09-16
**Supersedes nothing** — this formalizes the pattern Condition 1 (telehealth)
already established informally in `flows/export_medcor_ready.py`.

## Why

The Charge Validation Process is, by the client's own description, a long
list of per-claim conditions to check and — when a condition's criteria are
met — a correction to apply (Condition 1: a telehealth visit needs POS=11 and
M1=GT). Conditions are being implemented one at a time, and there will be
many more of them. This is a healthcare billing bot: every write it makes
changes what gets billed to a real payer, so the process must be:

- **Reusable** — adding Condition N must not require touching the code for
  Conditions 1..N-1, and must follow one obvious, fixed shape every time.
- **Exception-safe** — a bug in one condition's logic must never corrupt
  another condition's result on the same claim, crash the whole overnight
  batch, or leave a claim half-changed with no record of what happened.
- **Provably correct before it ever touches a real claim** — every condition
  must be unit-testable against a fixture, exactly like
  `tests/test_telehealth_fix.py` already does for Condition 1.

This spec explicitly rejects driving the live automation with an
LLM-agent-controlled browser (e.g. Playwright MCP) as the *engine* for this
reason: none of the three properties above hold for an engine whose
per-claim behavior is an LLM's live judgment call rather than fixed,
versioned code. Playwright MCP remains valuable as a *development and
debugging aid* (inspecting live eCW DOM, prototyping selectors) — that usage
is unaffected by this spec and requires no code change.

## Non-goals

- Not changing how claims are found, opened, or read (`claim_form.py`,
  `export_medcor_ready.py`'s claims-phase — untouched).
- Not building a self-healing/adaptive-selector system. If eCW's DOM shifts
  enough to break a selector, that is still a `selectors/claim_form.py`
  update, done and tested like every past one (see CHANGELOG "Day 5").
- Not retrying failed conditions automatically. A condition that throws is
  logged and skipped for that claim, once, this run.

## Architecture

### New package: `src/ecw_bot/conditions/`

```
src/ecw_bot/conditions/
    __init__.py       # CONDITIONS: list[module] — the registry
    base.py            # ConditionResult dataclass + run_all()
    telehealth.py      # Condition 1 — detect/apply/describe wrapper
```

### The Condition contract

Every condition module in the registry exposes exactly these three
functions. This is the whole interface — nothing else is required, nothing
else is read by the runner.

```python
def detect(claim_id: str, data: dict) -> bool:
    """
    Pure. No `page`, no I/O, no side effects. `data` is the same dict
    `claim_form.extract()` already produces (plus `progress_notes`, when
    OPEN_PROGRESS_NOTES is on). Must be safe to call with a partially
    populated dict — return False, never raise, on missing keys.
    """

def apply(page, claim_id: str, data: dict) -> "ConditionResult":
    """
    Only called when detect() returned True. Makes the eCW field edits for
    this condition and returns what changed. MUST NOT click Save/OK and
    MUST NOT show any confirmation dialog itself — the runner's caller owns
    both, once, after every condition has had a chance to run. Raising is
    allowed and expected to be handled by the caller (see Exception
    handling below) — apply() does not need its own try/except for its own
    field-write logic, only for optional recovery it wants to attempt
    itself.
    """

def describe(result: "ConditionResult") -> str:
    """
    One human-readable block for this condition's contribution to the
    combined confirmation dialog and the log line — mirrors the per-row
    lines `_apply_telehealth_fix` already builds today.
    """
```

### `ConditionResult` (`conditions/base.py`)

```python
@dataclass
class ConditionResult:
    condition_name: str
    changed: bool
    changes: list          # same shape as claim_form.set_cpt_pos_modifier()'s
                            # return value: [{"row", "code", "before", "after"}, ...]

    review: str = ""       # added 2026-10-05 (unit D) — see below

    @classmethod
    def none(cls, condition_name: str) -> "ConditionResult":
        return cls(condition_name=condition_name, changed=False, changes=[])

    @classmethod
    def declined(cls, condition_name: str, reason: str) -> "ConditionResult":
        return cls(condition_name=condition_name, changed=False, changes=[],
                   review=reason)
```

**Amendment 2026-10-05 (review items D2/P2, unit D).** `none()` means
"nothing to do — the claim already follows this rule". A condition that
matched the claim but could not act on it (the CPT/HCPCS tab would not
open, an admin code would not add with the grid untouched) now returns
`declined(name, reason)` instead: nothing changed, but the claim is not
clean, and `reason` reaches the Charge Summary as a "Review" row. A
changed result may also carry `review` (a failed add that still moved the
grid). Two optional module hooks join `detect/apply/describe`:

- `needs_review(claim_id, data) -> str | None` — asked only when
  `detect()` returned False, for a claim that matched the trigger but
  cannot be acted on (vaccine_admin_codes on an unmapped payer, no payer
  name, no usable age — never Medicare, which has its own rule).
- `summary_comment(result) -> str` — the Charge Summary comment for a
  changed result, built from what actually happened, in place of the
  summary's fixed `COMMENTS[name]`.

### The runner (`conditions/base.py`)

```python
def run_all(page, claim_id: str, data: dict) -> list[ConditionResult]:
    """
    Runs every registered condition against one claim, in registry order.
    Returns only the results where something actually changed — a claim
    with nothing to fix returns an empty list, and the caller shows no
    dialog and saves nothing.
    """
    results = []
    for cond in CONDITIONS:
        name = cond.__name__.rsplit(".", 1)[-1]
        try:
            if not cond.detect(claim_id, data):
                continue
            result = cond.apply(page, claim_id, data)
        except Exception as exc:
            print(f"[COND {name} {claim_id}] Failed: {exc}")
            shot(page, f"condition_{name}_failed_{claim_id}")
            continue  # this condition's failure must not block the next one
        if result.changed:
            results.append(result)
    return results
```

`shot()` is the existing screenshot helper already used by `open_claim()`'s
diagnostic wrapping (`claim_form.py`) — reused here, not reinvented.

**Amendment 2026-10-05 (review items D2/P2, unit D).** The real signature
is now `run_all(page, claim_id, data, conditions=None, before_apply=None,
report=None)`. Its return value is unchanged — only the changed results —
but that list is not everything that happened, so a caller that passes a
dict as `report` gets the whole account:

| key | contents |
|---|---|
| `fired` | names whose `detect()` returned True |
| `changed` | names with a changed result (= the return value) |
| `failed` | `{"name", "stage", "error"}` per raise; `stage` is `"detect"`, `"apply"` or `"needs_review"`, `error` the exception's first line |
| `review` | `{"name", "reason"}` from every result (changed or not) with a non-empty `review`, and from `needs_review()` of every condition whose `detect()` returned False |
| `comments` | `{name: summary_comment(result)}` for changed results whose module has the hook (a hook that raises is logged and left out — the summary falls back to `COMMENTS[name]`) |

Exceptions are still isolated exactly as in the sketch above: logged,
screenshotted, and the next condition still runs. `detect()` and `apply()`
now each sit in their own try (the sketch's single try is how it began), so
the report can say which stage raised.

### Caller contract (replaces `_apply_telehealth_fix`'s direct call)

`export_medcor_ready.py`'s `_read_claim_form()` changes from calling
`_apply_telehealth_fix(page, claim_id, data)` directly to:

```python
if cfg.APPLY_CORRECTIONS:
    results = conditions.run_all(page, claim_id, data)
    saved_write = _confirm_and_save(page, claim_id, results)
```

(As built since 2026-10-05: `run_all(..., report=report)` and
`_confirm_and_save(page, claim_id, results, report)`; the report is stored
on the claim's data as `data["_conditions"]` = `{"fired", "saved",
"failed", "review", "comments"}`, which the Charge Summary reads.)

`_confirm_and_save` (new, small, lives in `export_medcor_ready.py` — it is
orchestration, not a condition):

1. If `results` is empty → return `False` immediately, no dialog, no save.
   (A claim where nothing fired is not different from today's "no
   telehealth mention" case.)
2. Otherwise build one combined message: every fired condition's
   `describe()` output, concatenated, in the same
   `msgbox.ok_cancel(...)` shape Condition 1 uses today.
3. One Yes/No. Yes → `claim_form.save_claim(page)`. No → log "declined",
   return `False` (the caller's existing `finally` still closes with
   Cancel, unchanged).
4. Return whether the save actually happened, same as
   `_apply_telehealth_fix` returns today — this is what `_read_claim_form`
   uses to decide whether its own `finally` should call `close_claim()`.

**Amendment 2026-10-05 (review item P2, unit D):** between 1 and 2 —
and after the SAVE_CORRECTIONS gate — if the report has any `failed` or
`review` entry, the box is never shown: the log says `[COND <id>] Not
offering Save: <reasons> — closing with Cancel` and it returns `False`.

**Why one dialog, not one per condition:** if two conditions both want
changes on the same claim, saving after only reading half the operator's
decision is exactly the kind of half-applied state this spec exists to
prevent. The operator sees the whole picture — every condition that fired,
every field that would change — before any single Save/Cancel decision.

## Exception handling — the actual rules

This is the section the user asked for explicitly, spelled out rather than
left implicit:

| Failure | What happens |
|---|---|
| One condition's `detect()` raises | Caught by `run_all`'s try/except (its own try since 2026-10-05, so the report can say `stage: "detect"`). Logged, screenshotted, skipped, and recorded in the report's `failed` — the claim reads "Review", not clean. Other conditions still run. |
| One condition's `apply()` raises partway through a multi-row edit | Caught by `run_all`. Logged, screenshotted. That condition contributes nothing to `results`. **Corrected 2026-10-05 (review item P2):** this row used to say its partial edits "are never saved, because Save only happens ... for what's in `results`". That was wrong: Save (`saveAllData`) writes the whole form, partial edits included, and the box would have listed only the other conditions. So the raise is recorded in `run_all`'s report (`failed`, stage `"apply"`), and `_confirm_and_save` never offers Save for a claim whose report has a `failed` or `review` entry — the claim is closed with Cancel and reported "Review" in the Charge Summary. |
| `_confirm_and_save`'s dialog itself fails to show (msgbox error) | Not caught here — surfaces to `_read_claim_form`'s existing outer try/except, which logs "Could not read the form" and closes via `finally`. No new behavior needed; the existing outer boundary already covers this. |
| `save_claim()` returns `False` (save click didn't work) | Same as today: logged, `saved_write` stays `False`, `finally` calls `close_claim()` — the claim is left as read, not half-saved. |
| A condition throws on every claim in the batch (systemic bug in that one condition) | By design, this spec does not add batch-level circuit-breaking — each claim's screenshot + log line is enough signal for the operator to notice a pattern across a run's log file. Adding an auto-disable-after-N-failures behavior is explicitly deferred (YAGNI): today's batches run at `CLAIM_PROCESS_LIMIT` scale, small enough that log review is a real diagnostic step, not a chore. |

## Testing

- `tests/test_telehealth_fix.py` — unchanged. It tests
  `claim_form.note_mentions_telehealth` and `claim_form.set_cpt_pos_modifier`
  directly; those functions do not move.
- `tests/test_conditions_telehealth.py` (new) — tests
  `conditions/telehealth.py`'s thin wrapper: `detect()` delegates to
  `note_mentions_telehealth` correctly on a plain dict (no page needed);
  `apply()` calls `set_cpt_pos_modifier` and returns a populated
  `ConditionResult`; `describe()` produces the expected text.
- `tests/test_conditions_runner.py` (new) — tests `run_all()` and
  `ConditionResult` in isolation with fake condition modules (no real
  telehealth logic involved):
  - a condition whose `detect()` returns `False` is never `apply()`-ed
  - a condition whose `apply()` raises is skipped, and a *second*,
    healthy condition registered after it in `CONDITIONS` still runs and
    its result is still returned
  - `results` is empty when every condition's `detect()` returns `False`
  - a condition's raised exception does not propagate out of `run_all`

## Migration checklist (informs the implementation plan)

1. Create `conditions/base.py` (`ConditionResult`, `run_all`).
2. Create `conditions/telehealth.py` — move `note_mentions_telehealth`-based
   detection logic into `detect()`, wrap `set_cpt_pos_modifier` in
   `apply()`, write `describe()` from the existing message-building lines
   in `_apply_telehealth_fix`.
3. Create `conditions/__init__.py` with `CONDITIONS = [telehealth]`.
4. Add `_confirm_and_save` to `export_medcor_ready.py`; delete
   `_apply_telehealth_fix` (its logic is now split across
   `conditions/telehealth.py` and `_confirm_and_save`).
5. Update `_read_claim_form` to call `conditions.run_all` +
   `_confirm_and_save` instead of `_apply_telehealth_fix` directly.
6. New tests (above) all passing; full existing suite (88+ tests) still
   passing unchanged.
7. Condition 2 (whatever it turns out to be) is the first real proof this
   spec worked: it should require touching only a new
   `conditions/<name>.py` + its test file + one line in `CONDITIONS`.