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.

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)

@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)

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:

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.