Skip to content

feat(ledger): flag a learned suggestion on an unusually large line; batch accept skips it - #2016

Merged
jfrench9 merged 1 commit into
mainfrom
feature/suggestion-amount-note
Nov 11, 2026
Merged

jfrench9 merged 1 commit into
mainfrom
feature/suggestion-amount-note

Conversation

@jfrench9

@jfrench9 jfrench9 commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Summary

A learned suggestion on a bank line now says when the line is far larger than anything that counterparty has posted, and a batch accept leaves such a line to be classified on its own. Found with the ledger benchmark: weaker models took a vendor's usual account, in bulk, on a line that needed a split.

Changes

  • operations/roboledger/classification.py: when a line is more than three times the largest of the counterparty's posted lines (at least two posted), suggestion_basis names both amounts and the line carries suggestion_caution: unusual_amount. Applies to the learned default only; the open-item and bank-category rungs are unchanged.
  • operations/roboledger/commands/suggestions.py: accept-suggestions skips a line that carries a caution, with the reason needs_review and the basis as its detail.
  • operations/event_block/commands.py: suggestion_caution joins the metadata keys a caller cannot set.
  • models/api/extensions/agent.py, middleware/mcp/tools/instructions.py, docs/technical/Event-Block-Reference.md: describe the caution and the new skip reason.

Worth a close look: the threshold (three times, two posted lines) is a first setting, not a measured one.

Breaking Changes

None. needs_review is a new value of the free-text reason on a skipped line; suggestion_caution is a new metadata key. A line that a batch took before may now be skipped.

Testing

  • Targeted runs passed: classification, accept-suggestions, MCP tools, bank feed adapter and event block tests (1,644 passed); ruff, format and basedpyright clean on the commit.
  • just test-all: 18,683 passed, 44 skipped, 2 xfailed; lint, format and type checks clean.
  • On the local stack with the ledger benchmark: one model that had posted the whole line to the usual account split it correctly once the batch skipped it.

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

…counterparty's usual

Its basis names the amount and the largest posted before it, the line carries
suggestion_caution, and accept-suggestions skips it as needs_review so it is
classified on its own.
@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 25s —— View job


Review of #2016: unusual-amount caution on learned suggestions

Verdict: looks good to merge once just test-all is green. I found no correctness bugs. The notes below are minor. I read the diff and the surrounding classification.py. I did not run the tests.

What I checked

  • Caution lifecycle. out.pop(SUGGESTION_CAUTION, None) runs before every ladder branch, and SUGGESTION_CAUTION is in LADDER_KEYS. A caution therefore can't survive a re-run that lands on the settlement rung, the bank-category rung, or a normal-sized learned default.
  • Learned default only. The note is computed only in the SOURCE_AGENT_DEFAULT branch, as the description says.
  • Self-exclusion. Event.id != event_id is applied when event_id is given. A captured line isn't in the posted statuses anyway, so this is just a safeguard.
  • Caller can't set it. suggestion_caution is in the reserved metadata keys in _refuse_system_metadata. A client therefore can't plant or strip the caution to steer or bypass accept-suggestions.
  • Skip placement. The needs_review check in _skip_before_writing comes after not_open, no_suggestion and source_changed, so the more specific reasons still win.
  • Test arithmetic. The test numbers hold: 300,000 is not above 3 × 120,000, and 900,000 is.

Notes (non-blocking)

  1. Extra query per ladder run. _unusual_amount_note runs count and max queries for every learned-default evaluation. resuggest_open_lines loops over all of a counterparty's captured lines, so each learned-default line costs one extra aggregate. It is cheap with an index on (agent_id, entity_id), but a counterparty with a large captured backlog now does N aggregates after every commit. The aggregate doesn't depend on the line, apart from excluding the line's own id. You could compute it once per (agent, entity) and pass it in. Not worth doing unless you see it in profiles.

  2. "pending" in _POSTED_STATUSES. resuggest/learning elsewhere in this file counts ("committed", "fulfilled") as posted (the seeding query around line 564). Adding pending here makes "posted" mean something different from the learning path. If pending can hold a line that isn't posted, such as an unreviewed draft, it could inflate largest and mute the caution. If it is intentional, a one-line comment saying why would help.

  3. Sign and currency. abs(Event.amount) treats refunds and charges alike, and the comparison ignores currency. Both are probably fine for a first-pass heuristic. Worth knowing that a large refund from a vendor with small charges will be flagged.

  4. Threshold is a guess. The PR body says so. Consider making UNUSUAL_AMOUNT_MULTIPLE and the two-line minimum tunable through config/tuning.py (the SSM-backed tuning values) once there is data. That lets you change them without a deploy.

  5. Test gaps. Two cases would be worth adding:

    • A later re-run with a normal amount clears suggestion_caution.
    • The open-item and bank-category rungs never carry the caution.

    The pop handles both today, but nothing tests it.

  6. Docs. The wording is consistent across the Event-Block reference, the MCP instructions and the API model. The Event-Block doc could also name the minimum of two posted lines, since a reader may otherwise wonder why a first large line isn't flagged.

@jfrench9
jfrench9 merged commit cd9dda5 into main Nov 11, 2026
10 checks passed
@jfrench9
jfrench9 deleted the feature/suggestion-amount-note branch November 11, 2026 04:10
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