Skip to content

Add get_pnl + get_capacity MCP tools (completes P1a read layer)#127

Open
hhff wants to merge 18 commits into
mainfrom
worktree-mcp-capacity-pnl
Open

Add get_pnl + get_capacity MCP tools (completes P1a read layer)#127
hhff wants to merge 18 commits into
mainfrom
worktree-mcp-capacity-pnl

Conversation

@hhff

@hhff hhff commented Jul 6, 2026

Copy link
Copy Markdown
Member

What

The final two P1a read tools from the Stacksbot roadmapall eight P1a read tools are live after this.

  • `get_pnl` — Profit & Loss (revenue / COGS / expenses / net revenue / profit margin) per enterprise from the nightly-synced QBO P&L reports. Params: `enterprise` (default Sanctuary), `accounting_method` (cash/accrual), `start_date`/`end_date` (default most recent synced period), `vertical` (default All). Unblocks the Financial Reports auto-writer (automation Include contractors in OKRs #9).
  • `get_capacity` — per-person resourcing from the nightly utilization reports: each active person's sellable / billable / internal / time-off / unsold hours, utilization rate, and `benched` flag. Params: `studio`, `gradation` (default month, most-recent period). Unblocks bench-capacity → outreach (P4) and capacity watch.

Safety

  • Never a live external API call. `get_pnl` queries persisted `QboProfitAndLossReport` rows directly and never calls `find_or_fetch_for_range` (which fires live QBO fetches when a row is absent) — locked with a `.expects(:find_or_fetch_for_range).never` test. `get_capacity` reads persisted utilization reports only.
  • In-tool margin. The model's `data_for_enterprise` computes `profit_margin` but discards the result (returns 0 — a latent model bug, noted for a separate PR); `get_pnl` computes margin itself from the model's sound revenue/net bucketing.
  • Read-only annotations; skip+warn+Sentry on per-row failures; no schema/gem/model changes.

⚖️ For your review: capacity per-person data boundary

`get_capacity` is the most individual-level data any Stacks MCP tool exposes — per-person booked hours. I defaulted to named, resourcing-framed (email + hours + benched flag) because the roadmap explicitly names "per-person utilization" as a `get_capacity` deliverable, `get_studio_health` excluded utilization specifically because it's get_capacity's surface, and "who's free to staff" is unactionable without names. The payload emits no comp / salary / performance-evaluative fields — raw resourcing hours + benched status only; archived people excluded; the wall on comp/HR/1:1 content is untouched. If you'd prefer bench-only names or aggregate-only, say so and I'll re-cut it.

Process

Spec-first (design + plan in `docs/superpowers/`, capacity boundary flagged ⚖️), subagent-driven development in an isolated worktree, per-task reviews (get_pnl caught a missing `.distinct` dedup, reproduced against fixtures + fixed; get_capacity clean first pass), whole-branch final review (verdict: ready; its two worthwhile items — no-QBO-account guard, consistent empty-period shape — applied pre-PR). A multi-round adversarial review loop follows; findings land as additional commits.

Tests

Covering suites: 15 runs, 55 assertions, 0 failures. Full suite: 611 runs, 1454 assertions, 0 failures (19 pre-existing skips). Coverage: P&L bucketing + in-tool margin, never-fetch invariant (both tools), accounting-method/most-recent-period selection, range-miss lists available, unknown-enterprise (deduped) + no-QBO-account + invalid-method errors; capacity column mapping + benched flag, archived exclusion, most-recent period, studio scoping, invalid-gradation/unknown-studio errors, empty-period shape; registry (11 tools) + `tools/call` round-trips.

🤖 Generated with Claude Code

hhff and others added 18 commits July 6, 2026 12:18
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-map perf)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ty-period shape

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… dates; capacity mass-drop warn + single-query

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gap)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ti-account scoping, plucked ranges; deterministic capacity period

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nterprise name

Round-4 review: the round-3 malformed-data guard itself did String-indexing on
report.data / data[method], so a non-Hash jsonb value (array/number from sync
drift) raised TypeError ABOVE the rescue → 500. Guard now type-checks with
is_a?(Hash) at every level. Also error on a name that resolves to 2+ enterprises
instead of silently picking one. Multi-QBO-realm P&L aggregation documented as a
v1 limitation (has_one is the domain intent).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… one people load on studio path

Round-5 cleanups. find_rows substring-match (Total Income) deferred to the model-fix
follow-up PR — data_for_enterprise is shared with the admin dashboard, so fixing it
in-tool would make the tool disagree with Stacks' own P&L; fix at the model for both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round-6 review: round-5's reject(&:archived) on the studio path kept NULL-archived
people, while ForecastPerson.active (where.not(archived: true)) excludes them — the
two paths disagreed on the same person. Both paths now derive from the same .active
scope; the extra active.ids query is a deliberate correctness-over-micro-opt choice.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…le); capacity TOCTOU guard

Round-7 review — the important one prior rounds missed: QboAccount syncs monthly+
quarterly+yearly P&L into one table with no period-type column, and the current
year's report is future-dated (Dec 31), so order(:ends_at).last silently returned a
whole-year P&L instead of the latest month. Now a period_type param (month default)
scopes the default selection by report span in days; payload echoes it. Plus a TOCTOU
guard in get_capacity (rows deleted between maximum() and the re-query → empty, not 500).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…); drop dead capacity condition

Round-8 review: round-7's span fix still let the in-progress current month (future-dated
ends_at, only a few days of data) win order(:ends_at).last, so the default returned a
partial month's distorted revenue/margin. Now the default path filters ends_at <= today —
the most recent fully-elapsed period; explicit start_date/end_date still reaches the current
period-to-date. Also simplified a dead '&& records.any?' (records is non-empty past the guard).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…m' span label

Round-9 review (error-message/label polish — selection returns correct data):
- only an in-progress period synced → points caller to explicit start/end dates for
  period-to-date, instead of a misleading 'no reports of this granularity'.
- explicit-range report with an off-bucket span (~60d) → period_type 'custom', not null.
Deferred [0]: when the most-recent completed period lacks the requested accounting method,
erroring ('has no cash data') is more honest than silently substituting an OLDER period's
numbers under the 'most recent' contract — the sync gap is worth surfacing, not hiding.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hhff

hhff commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Self-review loop converged (10 rounds of high-effort multi-agent adversarial review).

This slice needed more rounds than the prior three — the QBO P&L period-selection semantics turned out to be genuinely subtle, and the loop was worth every round. What it caught, in order of severity:

  • round 1: an unrescued 500 on a report with the schema-default empty data; a silently-zeroed P&L for an unknown vertical.
  • round 2: forced the decision to cut the vertical param rather than ship silently-incomplete per-vertical numbers (model's below-the-line bucketing gap).
  • round 7 (the big one): a design flaw my spec, plan, and six prior rounds all missed — QboAccount syncs monthly + quarterly + yearly reports into one table with no discriminator and a future-dated current-year row, so "most recent" returned a whole-year P&L instead of the latest month. Fixed with a period_type param + span classification.
  • round 8: a deeper layer — the in-progress current month (future-dated, ~few days of data) still won; now defaults to the most recent completed period (ends_at <= today).
  • rounds 3–6, 9: NULL/non-Hash/malformed-data guards, multi-account scoping, a NULL-archived consistency bug (a regression from my own round-5 optimization, reverted), and error-message/label polish.

Rounds 9 and 10 returned zero wrong-data/crash findings — the P&L numbers and capacity roster are correct across a large matrix of real data-shape edges; the tail was error-message wording only. Round 10 was a genuine clean (all four finder angles completed).

Deferred (documented in the spec / code): three shared data_for_enterprise model defects (discarded margin, per-vertical below-the-line bucketing, find_rows substring match) → a separate model-fix PR so the tool and admin dashboard stay consistent; cross-QBO-realm P&L aggregation (has_one is the domain intent); the accounting-method-fallback (erroring is more honest than silently substituting a different period).

⚖️ Still for your call (PR body): the get_capacity per-person data shape — named/resourcing-framed by default, no comp/performance fields.

Full suite green (627 runs, 0 failures; 19 pre-existing skips). CI check in progress — will confirm before this is ready to merge.

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.

1 participant