Skip to content

feat(sei-agent-driver): set the review's reasoning effort per session - #468

Merged
bdchatham merged 3 commits into
mainfrom
feat/driver-review-effort
Oct 1, 2026
Merged

bdchatham merged 3 commits into
mainfrom
feat/driver-review-effort

Conversation

@bdchatham

@bdchatham bdchatham commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

SEIDROID_EFFORT sets the reasoning effort for the review's own agent, for example high. Empty leaves the agent spec's own. Today the review runs at the spec's effort, and the seidroid spec sets none.

Why per session

seidroid is the shared platform bot. Setting effort in its spec would change all of its work. The session API takes reasoning_effort at create, and SetReasoningEffort / ClearReasoningEffort move it later (omnigent-go-sdk v0.3.0). That matches how SEIDROID_MODEL already works.

Change

  • Config.Effort reads SEIDROID_EFFORT. Work.Effort is three-valued like Work.Model: nil leaves it alone, a value sets it, empty clears it.
  • workFor gives the review's work the configured effort. A scout gets nil, because a scout takes its effort from its own bundle.
  • Session create sends ReasoningEffort.
  • On adopt, reconcileOverrides moves both the model and the effort through one function, reconcileOverride. It replaces reconcileModel. Log messages now carry an override attribute.
  • README: a SEIDROID_EFFORT row.
  • A session create that the server refuses with a 400 now exits 2 (ExitConfig), not 6. The server refuses an unknown effort that way, and the SDK reports it as ErrInvalidInput, which the driver did not map.

Tests

Test What it proves
TestCreateCarriesTheConfiguredEffort Create sends reasoning_effort: high. Fails without the create field: <nil>, want high.
TestAdoptedSessionIsPointedAtThisRunsEffort Adopt sends one patch that sets the effort and leaves the model alone. Fails without the reconcile entry: sent 0 session patches.
TestARejectedEffortFailsAsConfiguration A 400 on create exits 2 and is not retried. Fails without the mapping: ExitCode = 6.
TestCreatedScoutSessionCarriesNoModel Now also asserts a scout session carries no effort.
Existing model tests Unchanged and passing on the shared reconcile.

Verification

gofmt -l .                     clean
go vet ./...                   clean
go test -race -count=1 ./...   ok (cmd, driver, omni, review)
golangci-lint run ./...        5 findings, all present on main unchanged
govulncheck ./...              No vulnerabilities found
vale README.md                 the new row is clean

Rollout

Takes effect once uci passes SEIDROID_EFFORT (sei-protocol/uci#118, claude-effort input, default high) and pins a driver that reads it. An adopted session whose harness is already up answers that run at its launch effort. The new value applies from the next launch.

🤖 Generated with Claude Code

SEIDROID_EFFORT sets the reasoning effort for the review's own agent, for
example `high`. Empty leaves the agent spec's own. The driver sends it at
session create and moves it on adopt, the same way as SEIDROID_MODEL.

The review agent, seidroid, is the shared platform bot. Setting effort per
session changes only reviews, not its other work. A scout keeps the effort
its own bundle sets.

The adopt path now reconciles both overrides through one function.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes session create/adopt and override reconciliation for every review run; misconfiguration or reconcile bugs could affect which model/effort answers a PR, though invalid effort fails fast at create.

Overview
Adds SEIDROID_EFFORT so review runs can override the agent spec’s reasoning effort (e.g. high) per Omnigent session, mirroring SEIDROID_MODEL. The driver loads it into config, attaches a three-valued Work.Effort for the review agent only (scouts keep nil so they use their own bundle), sends reasoning_effort on session create, and on adopt reconciles model and effort together via a shared reconcileOverrides path (replacing model-only reconcile).

Invalid effort at create (server 400 / SDK ErrInvalidInput) now maps to ExitConfig instead of a generic transport failure. README documents the new variable; tests cover create, adopt, clear, scout isolation, and config exit behavior.

Reviewed by Cursor Bugbot for commit 1bfe707. Bugbot is set up for automated code reviews on this repo. Configure here.

@seidroid seidroid Bot 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.

Adds SEIDROID_EFFORT, a per-session reasoning-effort override for the review's own agent. It is set when a session is created and moved when an existing session is adopted, using the same reconcile path as the model override, which this change generalises. Every call matches omnigent-go-sdk v0.3.0: the create field is omitempty, Set/ClearReasoningEffort exist, and SessionResponse.ReasoningEffort is *string. Scouts correctly get nil, and nothing blocks; codex found nothing, and I agree there are no defects, with two minor suggestions below (no REVIEW.md on the base branch; Go is not installed here, so the tests were not run and this rests on reading the PR tree at cf70afb).

Non-blocking

1 finding on the changed lines, as inline comments.

  • No test covers the clear branch for effort: an adopted session that already has a reasoning_effort while SEIDROID_EFFORT is empty should get ClearReasoningEffort. The shared reconcileOverride makes this low-risk, but the model override has its own clear test and effort has none.

seidroid review · decision approve · session 203bb184497c42bea42e0c97f9771b9a · turn resp_claude_6d9c5a8e4691fa20ce389370e0708410 · item 1211545875ca5a48bcd547b5b5955050

Findings: 0 blocking | 2 non-blocking | 1 posted inline

Comment thread sei-agent-driver/internal/driver/config.go
…configuration

The server refuses an unknown reasoning effort with a 400 at create. The
SDK reports that as ErrInvalidInput, which the driver did not map, so the
run exited 6 (transport) and pointed at the network. A 400 on create now
exits 2 (ExitConfig), like an argument the SDK refuses before sending.

The README row now says the server rejects an unknown value at create,
rather than at turn start.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Collaborator Author

@seidroid review

@seidroid seidroid Bot 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.

Since the last review, a 400 on session create (the SDK's ErrInvalidInput, which is how the server refuses an unknown effort) now exits 2 (ExitConfig) instead of 6, and the README says so. TestARejectedEffortFailsAsConfiguration pins both the exit code and that the create is not retried, which addresses my earlier finding at config.go:242, so that thread is resolved. One non-blocking test gap is still open; codex found nothing again, and I agree there are no defects (reviewed against the PR tree at 9cace77; Go is not installed here, so the tests were not run).

Non-blocking

  • No test covers clearing effort on adopt: when an existing session already has a reasoning_effort and SEIDROID_EFFORT is empty, the driver should call ClearReasoningEffort. The model override has its own clear test; effort relies only on the shared reconcileOverride.

seidroid review · decision approve · session 203bb184497c42bea42e0c97f9771b9a · turn resp_claude_5e249e98e98e065764c6095bd85faeb9 · item d3330648f7de509193ce5d999d9dfa4e

Findings: 0 blocking | 1 non-blocking | 0 posted inline

The model override had a clear-on-adopt test, and effort relied on the
shared reconcile. adoptFakeServer generalises modelFakeServer to any session
field, and the new test requires an empty SEIDROID_EFFORT to clear an
adopted session's effort.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Collaborator Author

@seidroid review

@seidroid seidroid Bot 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.

Since the last review, a new test covers clearing effort on an adopted session (TestAdoptedSessionsEffortIsClearedWhenNoneIsConfigured), using a generalised adoptFakeServer, which closes the test gap I raised last time; the earlier config.go:242 thread is already resolved and holds. Nothing blocks; I kept codex's one finding that the clear assertion is loose, as a nit, since with an empty want only the clear path can run and the test can only miss a wrong alias value (reviewed against the PR tree at c0d27f4; Go is not installed here, so the tests were not run).

1 nit, not posted on the code
  • sei-agent-driver/internal/omni/effort_test.go:90 — From codex, confirmed: the assertion accepts any non-empty reasoning_effort, so it does not check that the clear alias ("default" in omnigent-go-sdk v0.3.0) was the value sent. The model clear test at model_test.go:137 has the same looseness.

seidroid review · decision approve · session 203bb184497c42bea42e0c97f9771b9a · turn resp_claude_a69e7c8bf9ec00017efc1208ff56ce0e · item b64a3901e3ea50efa0c26054ef8ed8ff

Findings: 0 blocking | 0 non-blocking | 0 posted inline

bdchatham added a commit that referenced this pull request Oct 1, 2026
…efused (#467)

## Summary

A run that ends with no verdict (exit 5) now deletes its session. The
next dispatch opens a fresh one instead of adopting a conversation whose
last reply was refused. Without this, one bad reply blocks every re-run
of a pull request until someone posts `@seidroid review close`.

## Root cause

On sei-chain#4387, the review's verdict block was missing one comma
after `"summary"`. `ParseVerdict` refused it: `the closing block is not
a json object`. The re-run adopted the same session (`da2e0278…`,
`continued: true`). `holdsAnswer` was false, so the driver sent the full
prompt again. The agent saw its own earlier reply and repeated it, with
the same missing comma. The first 4 lines of both replies are identical.

Run:
[36782418617](https://github.com/sei-protocol/sei-chain/actions/runs/36782418617),
attempts 1 and 2.

## Change

- `Driver.answer` calls `discard` after a refused reply. `discard` is
best-effort, runs within the run's own deadline, and never changes the
exit code.
- `Conversation` gains `Discard(ctx) error`. It deletes the session the
run holds with one request. `Host.Close` would re-mint a token and
search by run key for a session the run already holds.
- A transport failure or an expired deadline keeps its session, because
the agent may still be working.

## Tests

| Test | What it proves |
|---|---|
| `TestARefusedReplyDeletesItsSession` (omni) | A refused reply sends
`DELETE /v1/sessions/conv_1`. Fails without the change: `deleted = [],
want [conv_1]`. |
| `TestRunReportsNoVerdictWhenTheReplyIsUnfinished` | Exit 5 discards
once, within the run's deadline. Fails without the change (`discarded =
0, want 1`), and fails on a detached context. |
| `TestRunKeepsNoVerdictWhenTheDeleteFails` | A failed delete keeps exit
5 and `TeardownOK`. |
| `TestRunReportsAFinishedAnswer`,
`TestRunCarriesAReplyReadBeforeAFailure` | A finished answer and a
transport failure keep their session. |

## Verification

```
gofmt -l .                     clean
go vet ./...                   clean
go test -race -count=1 ./...   ok (cmd, driver, omni, review)
golangci-lint run ./...        5 findings, all present on main unchanged (errcheck ×2 in test fakes, S1025, SA5011 ×2)
govulncheck ./...              No vulnerabilities found
```

## Rollout

Needs a driver release, then the pin bump in
`uci/.github/workflows/seidroid-review.yml`. Ships in the same release
as #468 (`SEIDROID_EFFORT`).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@bdchatham
bdchatham merged commit 7885882 into main Oct 1, 2026
10 checks passed
@bdchatham
bdchatham deleted the feat/driver-review-effort branch October 1, 2026 15:23
bdchatham added a commit that referenced this pull request Oct 1, 2026
Cuts sei-agent-driver **v0.23.0**:

- A run that ends with no verdict deletes its session, so a re-run
starts fresh instead of repeating the refused reply (#467).
- `SEIDROID_EFFORT` sets the review's reasoning effort per session. A
session create the server refuses with a 400 now exits 2 (`ExitConfig`)
instead of 6 (#468).

Merging this triggers the Releaser (`uci-release-publish.yml`): the
GitHub release, goreleaser, and the `sei-agent-driver/v0.23.0` module
tag that `go install` resolves.

Next: sei-protocol/uci bumps `seidroid-review.yml`'s `driver-version`
default and its pinned sha256 to `v0.23.0`. With sei-protocol/uci#118,
the review then runs at `high` effort.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
bdchatham added a commit to sei-protocol/uci that referenced this pull request Oct 1, 2026
## Summary

A new input, `claude-effort`, sets the review's reasoning effort and
defaults to `high`. The workflow passes it to the driver as
`SEIDROID_EFFORT`. Before this, the review ran at the agent spec's
effort, and the `seidroid` spec sets none.

## Change

- `claude-effort` input next to `claude-model`, default `high`. Empty
leaves the spec's own.
- `SEIDROID_EFFORT: ${{ inputs.claude-effort }}` on the drive step, next
to `SEIDROID_MODEL`.

## Dependency

The driver reads `SEIDROID_EFFORT` from
sei-protocol/sei-internal-skills#468. An older driver ignores the
variable, so this is safe to merge first. It takes effect when this
workflow pins a driver release that reads it.

## Verification

`actionlint`: the same 7 findings as on main, compared with line numbers
removed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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