Skip to content

feat(benchmarks): run the ledger tasks through the author operator - #2019

Open
jfrench9 wants to merge 2 commits into
mainfrom
feature/operator-benchmark-row
Open

jfrench9 wants to merge 2 commits into
mainfrom
feature/operator-benchmark-row

Conversation

@jfrench9

@jfrench9 jfrench9 commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Summary

The ledger benchmark can now run its tasks through the stack's own author operator, and the operator can make three choices that go with a classification it was already allowed to make.

Changes

Operators

  • operations/operators/implementations/author.py: the classify guard on update-event-block now passes classified_flow_qname, remember_classification and not_a_settlement beside the account choice. The transition is still classified only; committing, voiding and editing stay a person's step. The operator's prompt names the three keys.

Benchmark (benchmarks/ledger/, not in the image)

  • operator.py (new), rows.py, __main__.py: row O sends each task to the author operator, waits for the run, commits what it classified as the approving person would, and scores the books with the existing scorer. month is not run (a close is a person's step) and a line that needs a new account is expected to be left open. Cost is worked out from the run's tokens; the record keeps the credits it metered.
  • mcp.py: a helper for calls to the stack's API outside MCP.
  • README.md: describes the row.

Worth a close look: not_a_settlement lets the operator classify a line that matches an open bill to another account. It still only classifies, so the choice waits on a person's commit.

Breaking Changes

None.

Testing

  • Targeted runs passed: the author operator tests (60) and the benchmark tests (24); ruff, format and basedpyright clean on both commits.
  • Full suite locally: 18,711 passed, 44 skipped, 2 xfailed, with one test set aside. tests/security/test_cypher_analyzer.py::TestGuardedStringMatchCost::test_many_labels_rebound_many_times_stay_linear is a 2-second timing check that failed twice under just test-all on a heavily loaded machine and passes alone; this branch does not touch it or the code it covers. CI is the check for it.
  • On the local stack: all nine operator-eligible tasks through the author operator, one trial each, scored correct.
  • Not run: a live operator call that uses one of the three new keys; no benchmark task needed one.

Certification

  • I have the right to submit this work under the Apache 2.0 license, and do so. Where any part of it is owned by my employer, I have their permission.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AGZDQdkBCwZfuhmNzUEnPU

… settlement exception

The classify guard now passes classified_flow_qname, remember_classification
and not_a_settlement beside the account choice. None of them posts.

Claude-Session: https://claude.ai/code/session_01AGZDQdkBCwZfuhmNzUEnPU
Row O sends the task to the stack's operator, commits what it classified as
the approving person would, and scores the books. A close is not run and a
line that needs a new account is expected to be left open.

Claude-Session: https://claude.ai/code/session_01AGZDQdkBCwZfuhmNzUEnPU
@github-actions github-actions Bot added the change:normal Normal change: automated review documented on the pull request before merge label Nov 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Change classification: normal — platform or application code changed.

A normal change carries a documented automated review on this pull request before merge, in place of a second human approver. The change-classification job requests that review and passes once it has been posted. Recorded by the change-classification job.

@jfrench9

Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude

claude Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Claude finished @jfrench9's task in 27s —— View job


Review of PR #2019: run the ledger tasks through the author operator

I read the diff, and checked the result and metadata keys the benchmark reads against analyst.py and the worker. I did not run the tests or the benchmark. Overall it looks good and I found nothing that blocks merging. The notes below are minor.

Operator change (author.py)

  • Adding classified_flow_qname, remember_classification and not_a_settlement to _CLASSIFY_METADATA is safe. The classified-only transition guard is untouched, so the operator still can't commit, void or edit.
  • not_a_settlement is the one to look at, as you flagged. It only changes which account a line is classified to, and a person's commit is still required.
  • remember_classification is the stickiest of the three. Once committed, it makes the account the counterparty's default, so one bad classification carries into later lines. It is reasonable to allow, but the live-call gap you note in Testing matters most here. I'd add a unit test that passes all three keys through the guard together, if the existing test only covers one.
  • The tool description and the guard's error message agree.

Benchmark (operator.py, __main__.py)

  • The result keys (loop_iterations, hit_step_limit, hit_credit_ceiling, credits_consumed, writes) match what analyst.py emits.
  • Poll loop (operator.py, in run): the condition while response.status_code == 202 or not status has two problems.
    • On a 200 it polls exactly once, then exits even if that status is still running. The record then shows a non-completed stop reason, which is misleading. Looping until the status is settled, whatever the HTTP code, would be simpler and safer.
    • If operation_id is missing, it requests /operations/None/status and spins until the timeout. A check that fails fast when there is no id would be better.
  • Status codes: .json() on the status response isn't guarded. A non-200 or non-JSON reply raises instead of recording an error.
  • Error message (__main__.py, _run): "a close is a person's" reads as cut off. Something like "a close is a person's step" would be clearer. The check also raises when O is combined with other rows and a close task is named, even though the other rows could run it. Skipping per row, as the all path does, would be more consistent.
  • approve:
    • It commits every classified line in line_ids, including inbox lines the prompt told the operator to leave alone. If the operator wrongly classifies one, the harness commits it and scoring catches it, which is the right behaviour. The README or docstring could say so.
    • Commit refusals are dropped silently, since approved only lists successes. Recording the refused keys with their error text would make a failing trial easier to read.
  • tool_events is rebuilt from tools_called and writes with zeroed fields. That is fine, but errors= and calls= in the summary line are approximations for this row. Worth a line in the README.
  • Pricing: trial_cost(priced_as, ...) depends on --operator-model matching what the stack's author tier actually runs, and the default is hardcoded. The record does keep the metered credits, which is a good cross-check.

Tests and docs

  • The README addition is clear, and the "--model does not apply" note is useful.
  • Test additions look proportionate. The poll-loop edge cases above (200 with an unsettled status, missing operation_id) are worth a small test, since they are cheap to fake.

just test-all was still unconfirmed when the PR was opened, so please wait for it to pass before merging.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:normal Normal change: automated review documented on the pull request before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant