feat(skills): gate every figure on data coverage, and offer the control that fixes it - #325
feat(skills): gate every figure on data coverage, and offer the control that fixes it#325JulianWell wants to merge 1 commit into
Conversation
…ol that fixes it Charts from the skills were incomplete in two ways nothing announced: no check that a bank source exists and is healthy, and no check that the period's transactions are categorized. The close-books flow already treats both as blockers; the analysis skills didn't check either. New `well:data-coverage` runs before any figure is produced and, for each gap, offers the CONTROL rather than a description of one: - No bank source → call `well_list_connectors`, which renders the Well connect card (searchable provider picker, one-click install) in widget-capable hosts, with the `install_url` as the text-only fallback. A connected-but-unhealthy connector is named as its own errand, not as "connect a bank". - Uncategorized transactions → the named rows, not a count. `category_status` is a typed enum that is NULL until something looks at the row, so the filter spells out the null branch; `_neq: "categorized"` alone silently undercounts. Wired into compte-de-resultat, balance-sheet, vat-summary, ar-aging, reconciliation and month-end-close as step 1, with the coverage line carried into each output. Two further fixes to the same failure mode: - `cash-flow-forecast` told the model to rebuild runway by hand from account_balances. `well_get_runway` already computes it server-side AND reports `status: insufficient_data` / `partial` / `excluded` / `hints` — every completeness signal a re-derivation destroys. Now starts from the tool. - `querying-well-data` claimed "you never pass a workspace id". False on a multi-workspace token: the read fans out and merges, so a figure can span two companies. Replaced with a scope-first step, and the tool list is no longer missing the KPI and connector tools. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GYLCkxuwbzp85jUwRUYaAF
hippolytewellapp
left a comment
There was a problem hiding this comment.
Sound, and unusually well-grounded — I checked every factual claim in the Verification section against the platform source and each one holds exactly. TransactionCategoryStatusEnum really is those six values and really is nullable (category_status? on Transaction.ts, with the IS NULL guard in stampPendingCategoryStatusSql confirming the "null until something looks at the row" premise); well_list_connectors is in UI_TOOL_NAMES; well_get_runway's output schema is status / partial / excluded: {accounts, transactions} / hints[].detected_gap + suggested_action verbatim; the fan-out-and-merge behaviour and the multi-workspace write refusal are exactly what workspace-dispatch.ts implements (Rule C and Rule D); well_list_workspaces returns registered_name / trade_name / country / base_currency / fiscal_year_start_month as promised; and every tool in the README's expanded Read and Write lists exists, including the six company/person ones that are registered through the defineWriteTool factory rather than as literal tool files. The _neq-drops-nulls gotcha is real three-valued-logic and the _or spelling of it is correct. Skills that assert things about an API they don't live next to normally rot on contact; this one doesn't.
The framing is also the right one. Gating before the figure rather than caveating after it, and handing over the control (well_list_connectors so the connect card renders) instead of prose describing the control, is the difference between a diagnosis and a complaint — and killing the hand-rolled runway re-derivation in cash-flow-forecast is the same argument applied to itself, since re-deriving is precisely what discards partial / excluded / hints.
Worth tightening
-
Tripwire: the gate and the figure can end up scoped to different workspaces. The coverage probes in step 1 go through
well_query_recordsonaccounts/transactions, which is fan-out-eligible — on a multi-workspace token with noworkspace_idit merges rows across the whole grant set. Butwell_get_runway,well_get_cash_position,well_get_cost_structureandwell_list_connectorsall carryfanOutAcrossWorkspaces: false, so under the same conditions they silently run in the primary workspace only. A model that skips the scope step therefore gets a coverage line computed over two companies and a runway computed over one — the gate reads clean and the figure still has the hole.querying-well-data's "passworkspace_idon every subsequent call" prevents it, butdata-coveragenever restates the requirement, and its own description ("Use BEFORE producing any figure") makes it the skill most likely to be invoked standalone. One clause indata-coverage— run the gate in the same named workspace the figure will be for — closes it. Worth knowing the dispatcher does honourworkspace_idon the KPI tools even though theirinputSchemaisz.object({}).strict(): the field is injected into the registered schema and stripped before the tool body runs, so the advice is safe to give. -
reconciliationisn't gated the way the description says it is. The body states the gate is "wired in as step 1 of ...reconciliationandmonth-end-close". Inreconciliationthe only change is the "Present it" line, so the gate lands in the output ordering rather than in the build procedure; inmonth-end-closeit is step 2, behind schema discovery. Both are defensible as written — a coverage line can't be presented without running the gate — butreconciliationis the skill where the failure bites hardest, by its own added clause ("an unsynced period makes every invoice in it look unpaid"), so it is the odd one out to leave un-numbered while the other five got an explicit step 1. Either add the step or soften the claim. -
Heads-up, not a blocker: the known limitation is accurate —
well_list_connectors' input schema isq/limit/offsetwith no category filter, and its output does carrycategory_id, so the follow-up you describe is a small one.
CI
No checks configured on this branch; mergeStateStatus is CLEAN. Nothing to verify mechanically — the claims were the reviewable surface here, and they check out.
Problem
From Maxime, after client calls: the charts the skills produce are incomplete, and nothing says so. Two gaps, neither of which announces itself:
The close-books flow already treats both as blockers (
CLOSE_BLOCKER_CODE.MISSING_BANK_CONNECTOR/CONNECTOR_UNHEALTHY/UNCATEGORIZED_TRANSACTIONS, each mapped to one errand). The analysis skills checked neither.What changed
New
well:data-coverage— a gate, not a caveatRuns before the figure exists, and for each gap offers the control rather than a description of one:
well_list_connectors. That tool renders the Well connect card — the searchable provider picker with one-click install — in widget-capable hosts. The skill says to call it rather than summarise the catalog into a paragraph, which replaces a working control with prose about one.install_urlis the text-only fallback. A connected-but-unhealthy connector is named as its own errand, not as "connect a bank" to someone who already did.transactions.category_statusis a typed enum that is NULL until something looks at the row, so the filter spells out the null branch —_neq: "categorized"alone drops the nulls and silently undercounts the gap. The statuses are distinguished when named:pendingis work in flight,classifier_abstained/classifier_failedneed a human decision.accounts/transactions, not the connector list.Wired in as step 1 of
compte-de-resultat,balance-sheet,vat-summary,ar-aging,reconciliationandmonth-end-close, with the coverage line carried into each "Present it".cash-flow-forecastno longer hand-rolls runwayIt told the model to rebuild runway from
account_balances.well_get_runwayalready computes it server-side — with the sign-convention detection, internal-transfer exclusion and FX conversion a hand-rolled sum gets wrong — and returnsstatus: "insufficient_data",partial,excluded: {accounts, transactions}andhints[]. Every completeness signal a re-derivation destroys is exactly what this PR is about. Now starts from the tool and carries those signals into the output.querying-well-datasaid something falseUntrue on a multi-workspace token: the read fans out across the grant set and merges the rows into one flat result, so a single figure can span two companies. Replaced with a scope-first step (
well_list_workspaces→ passworkspace_id→ name the workspace in the answer). The tool list was also missingwell_list_workspaces,well_list_connectorsand the three deterministic KPI tools; the README's list was staler still.Verification
namematches its directory, descriptions within limitswell:<skill>cross-reference resolves (no dangling refs)category_statusconfirmed exposed towell_query_records(fields-transactions.json) and its enum values taken fromTransactionCategoryStatusEnum, not guessedwell_list_connectorsconfirmed to be in the platform'sUI_TOOL_NAMESand mapped to theConnectProviderswidget — so "call the tool and the connect card renders" is a real behaviour, not an aspirationKnown limitation
well_list_connectorshas no category filter, so "show me the banks" is expressed as a name search (q) or the curated view. Acategory_idfilter on that tool would make the missing-bank case one clean call — worth a follow-up on the platform side.Related
Companion fix on the platform: WellApp-ai/platform#5778 makes every MCP widget name the workspace(s) its result belongs to, which was the other half of the same feedback.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GYLCkxuwbzp85jUwRUYaAF