Skip to content

emit_outcome — the Python mirror of the agent-side claim - #53

Merged
achris7 merged 2 commits into
mainfrom
feat/emit-outcome
Sep 21, 2026
Merged

achris7 merged 2 commits into
mainfrom
feat/emit-outcome

Conversation

@achris7

@achris7 achris7 commented Sep 21, 2026

Copy link
Copy Markdown
Member

The Python half of P3.1, mirroring emitOutcome in the TS SDK field for field. Parity between the two SDKs is an advertised property, so they land together.

What it does

agent.fetch(url, task_id="call-8821")
agent.emit_outcome(
    "call-8821", "meeting_booked",
    idempotency_key="call-8821:meeting_booked",
)

Floe resolves the task id to the call it belongs to and binds the claim there, so cost and outcome sit on one row — which is what makes cost-per-outcome a number rather than an estimate.

What it is not

Not report_outcome. That is the per-action quality signal feeding the quality throttle, and it never reaches an invoice. This is the invoice-grade claim. They share a word and nothing else, which is why OutcomeClaim is a separate dataclass from OutcomeResult rather than a widened one.

An agent key may only report

There is deliberately no status parameter. Confirming a claim, voiding one and resolving a collision are operator acts on the developer-scoped surface: they move money, and the evidence justifying them — a CRM webhook, a calendar invitation — reaches the operator's backend minutes to days after the call, never this process.

confirmed_at is absent from the call signature and always None in the response: emitting never sets the billing anchor.

Validation is local first

As report_outcome does — a bad argument is a FloeAgentError, not a round trip. Two things worth naming:

  • The idempotency key is capped at the server's 200 rather than at MAX_TAG_LENGTH (128). It is not an attribution tag, and rejecting a key the API would have accepted is a client bug, not strictness.
  • external_ref without external_system fails locally. The server has a CHECK constraint saying the same thing, but failing here names the actual mistake instead of returning a constraint message.

Tests

5 new cases; 14 passing in the file, 173 passed / 1 skipped across the suite, ruff check src/ clean — the same gates CI runs. They pin the wire contract (omitted optionals stay absent, not null — the route is strict), the evidence allowlist going out verbatim, and the local guards short-circuiting before any request.

Requires the API side (floe-monorepo #502); POST /v1/agents/outcomes 404s until that ships.

The TS counterpart is Floe-Labs/agentkit-actions.

Parity with the TS SDK's emitOutcome, field for field. Posts a billable
outcome claim against the task id the caller already has; Floe resolves it
to the call and binds the claim there, so cost and outcome sit on one row.

NOT report_outcome. That is the per-action quality signal and never
reaches an invoice; this is the invoice-grade claim. They share a word and
nothing else.

An agent key may only REPORT — no `status` parameter. Confirming, voiding
and resolving a collision are operator acts on the developer surface,
because they move money and the evidence justifying them reaches the
operator's backend long after the call, never this process.

Local validation first, as report_outcome does. The idempotency key is
capped at the server's 200 rather than MAX_TAG_LENGTH: it is not an
attribution tag, and rejecting a key the API would have accepted is a
client bug, not strictness.
@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 6a045c4b-a83d-46e7-91dd-f8858c0e9a6d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@achris7
achris7 marked this pull request as ready for review September 21, 2026 22:41
Copilot AI lite review requested due to automatic review settings September 21, 2026 22:41

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Three moderate validation issues remain in floe_agent.py.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Adds Python SDK support for emitting invoice-grade outcome claims tied to task IDs.

Changes:

  • Adds OutcomeClaim and emit_outcome.
  • Adds local validation and wire-format tests.
  • Exports OutcomeClaim publicly.
File Summary Review findings
tests/​test_attribution_and_advisory.py Tests emission behavior, serialization, and validation. None noted.
src/​floe_agentkit_actions/​floe_agent.py Implements outcome claims and API submission. Three moderate issues: non-string task_id can raise AttributeError (3 votes); occurred_at lacks runtime string validation (2 votes); quantity lacks the server’s upper-bound validation (1 vote).
src/​floe_agentkit_actions/​__init__.py Re-exports OutcomeClaim. None noted.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/floe_agentkit_actions/floe_agent.py
Comment thread src/floe_agentkit_actions/floe_agent.py
Parity fix, carried from review on agentkit-actions #80.

That review found the TS SDK checking occurredAt only for `typeof string`.
This side was worse: occurred_at went into the payload with no validation at
all, so any string reached the API and came back a 400.

Mirrors the route rather than approximating it. The API declares the field
z.string().datetime() — RFC-3339 with a Z — so neither "2026-09-15" nor a
+01:00 offset is accepted there, and a lenient parse would still pass values
the server rejects. The shape check is paired with a real parse, because
"2026-13-45T00:00:00Z" matches the pattern and raises
"month must be in 1..12".

Adds `import re` / `from datetime import datetime` and the _ISO_UTC_RE
constant beside MAX_TAG_LENGTH.

Five parametrised rejections asserting no request is made, plus a
well-formed value asserted to go out verbatim. 178 passing, ruff clean.
Copilot AI review requested due to automatic review settings September 21, 2026 22:48
@achris7

achris7 commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Carrying a fix over from review on agentkit-actions #80 — TS/Py parity is an advertised property of this pair, so it cannot land on one side only.

That review found the TS SDK validating occurredAt with only a typeof check, letting "not-a-date" reach the API. This side was weaker still: occurred_at went into the payload with no validation whatsoever.

The fix mirrors the route rather than approximating it. The API declares the field z.string().datetime() — RFC-3339 with a Z — so a lenient parse would still pass values the server rejects:

Value The route Now
2026-09-15T10:30:00Z ok accepted
2026-09-15 rejected rejected
2026-09-15T10:30:00+01:00 rejected rejected
2026-13-45T00:00:00Z rejected rejected

The shape check is paired with a real parse, because that last one matches the pattern and is not an instant — fromisoformat raises month must be in 1..12 (verified, not assumed).

Five parametrised rejections assert captured == [], so the request is never made; a sixth pins that a well-formed value goes out verbatim. Mutation-proven — removing the guard turns 3 of the 4 invalid forms red (the fourth, "not-a-date", is rejected downstream regardless, so the guard is what makes the other three fail locally).

178 passing, 1 skipped, ruff check src/ clean — the same gates CI runs.

@achris7
achris7 merged commit 22f8ef7 into main Sep 21, 2026
5 checks passed

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Three moderate validation and wire-parity issues remain unresolved.

Review effort: Lite
Findings: None

Resolved since last review (2)

achris7 added a commit that referenced this pull request Sep 21, 2026
Review round on #53.

_validate_tag called value.strip() on whatever it was handed, so passing a
non-string task_id raised AttributeError — while this client promises a
FloeAgentError for every other bad argument, and the TS mirror guards the
type.

Hardened in the HELPER rather than at the call site, because it has four
callers: fetch's task_id and action_id, report_outcome, and emit_outcome.
Patching emit_outcome alone would have left the same AttributeError
reachable through the other three.

Two tests: a parametrised non-string task_id through emit_outcome, and the
same through report_outcome — the second exists to prove the fix is in the
helper rather than one branch. Both assert no request is made.

The occurred_at type check the review also asked for is already present: the
ISO-8601 guard added last round begins with isinstance(occurred_at, str),
so a non-string is rejected locally before any request.

183 passed, 1 skipped, ruff clean.
@achris7

achris7 commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

One fixed, one already done.

task_id raised AttributeError instead of the typed error

Correct. _validate_tag called value.strip() on whatever it was given, so a non-string produced an AttributeError — while this client promises a FloeAgentError for every other bad argument, and the TS mirror guards the type.

Fixed in the helper rather than at the call site, because it has four callers — fetch's task_id and action_id, report_outcome, and emit_outcome. Guarding only emit_outcome, as suggested, would have left the identical AttributeError reachable through the other three:

emit_outcome(task_id=123)      -> FloeAgentError: task_id must be a string (got int).
report_outcome(action_id=42)   -> FloeAgentError: action_id must be a string (got int).

Two tests, and the second one earns its place: it drives report_outcome, so it fails if someone later "fixes" this by patching a single branch.

occurred_at — already guarded

This one reviews the pre-fix state. The ISO-8601 guard added in the previous round opens with exactly the check requested:

if not isinstance(occurred_at, str) or not _ISO_UTC_RE.match(occurred_at):

So occurred_at=123 and occurred_at=True are both rejected locally before any request. The guard goes further than the suggestion — format is validated client-side too, because the route declares the field z.string().datetime() and a merely-a-string value would still round-trip to a 400.

183 passed, 1 skipped, ruff check src/ clean — the gates CI runs.

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.

2 participants