Skip to content

feat(skills): gate every figure on data coverage, and offer the control that fixes it - #325

Open
JulianWell wants to merge 1 commit into
mainfrom
feat/skills-coverage-gate
Open

feat(skills): gate every figure on data coverage, and offer the control that fixes it#325
JulianWell wants to merge 1 commit into
mainfrom
feat/skills-coverage-gate

Conversation

@JulianWell

Copy link
Copy Markdown

Problem

From Maxime, after client calls: the charts the skills produce are incomplete, and nothing says so. Two gaps, neither of which announces itself:

  1. No bank source — cash movement that was never synced can't appear in a cash position, a runway, a P&L or a reconciliation. The number computes, looks plausible, and is missing a whole account.
  2. Uncategorized transactions — money that moved but carries no category lands nowhere in a cost structure. The categories shown are real; the total under them isn't the total that moved.

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 caveat

Runs before the figure exists, and for each gap offers the control rather than a description of one:

  • No bank source → call 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_url is 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.
  • Uncategorized transactions → the named rows, not a count. transactions.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 drops the nulls and silently undercounts the gap. The statuses are distinguished when named: pending is work in flight, classifier_abstained/classifier_failed need a human decision.
  • Detection from the data, remediation from the catalog — a connector can be connected and have synced nothing, so the presence check reads accounts/transactions, not the connector list.

Wired in as step 1 of compte-de-resultat, balance-sheet, vat-summary, ar-aging, reconciliation and month-end-close, with the coverage line carried into each "Present it".

cash-flow-forecast no longer hand-rolls runway

It told the model to rebuild runway from account_balances. well_get_runway already computes it server-side — with the sign-convention detection, internal-transfer exclusion and FX conversion a hand-rolled sum gets wrong — and returns status: "insufficient_data", partial, excluded: {accounts, transactions} and hints[]. 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-data said something false

"Reads are scoped to the authenticated workspace automatically — you never pass a workspace id."

Untrue 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 → pass workspace_id → name the workspace in the answer). The tool list was also missing well_list_workspaces, well_list_connectors and the three deterministic KPI tools; the README's list was staler still.

Verification

  • All 10 skills: frontmatter parses, name matches its directory, descriptions within limits
  • Every well:<skill> cross-reference resolves (no dangling refs)
  • Ordered lists renumbered after the inserted step
  • category_status confirmed exposed to well_query_records (fields-transactions.json) and its enum values taken from TransactionCategoryStatusEnum, not guessed
  • well_list_connectors confirmed to be in the platform's UI_TOOL_NAMES and mapped to the ConnectProviders widget — so "call the tool and the connect card renders" is a real behaviour, not an aspiration
  • Plugin version 0.2.0 → 0.3.0

Known limitation

well_list_connectors has no category filter, so "show me the banks" is expressed as a name search (q) or the curated view. A category_id filter 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

…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 hippolytewellapp left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_records on accounts / transactions, which is fan-out-eligible — on a multi-workspace token with no workspace_id it merges rows across the whole grant set. But well_get_runway, well_get_cash_position, well_get_cost_structure and well_list_connectors all carry fanOutAcrossWorkspaces: 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 "pass workspace_id on every subsequent call" prevents it, but data-coverage never restates the requirement, and its own description ("Use BEFORE producing any figure") makes it the skill most likely to be invoked standalone. One clause in data-coverage — run the gate in the same named workspace the figure will be for — closes it. Worth knowing the dispatcher does honour workspace_id on the KPI tools even though their inputSchema is z.object({}).strict(): the field is injected into the registered schema and stripped before the tool body runs, so the advice is safe to give.

  • reconciliation isn't gated the way the description says it is. The body states the gate is "wired in as step 1 of ... reconciliation and month-end-close". In reconciliation the only change is the "Present it" line, so the gate lands in the output ordering rather than in the build procedure; in month-end-close it is step 2, behind schema discovery. Both are defensible as written — a coverage line can't be presented without running the gate — but reconciliation is 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 is q / limit / offset with no category filter, and its output does carry category_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.

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.

3 participants