Repository navigation
feat(OMN-13161): drop hardcoded delegate max_tokens constant; resolve from routing contract - #1990
Conversation
… from routing contract Remove DEFAULT_MAX_TOKENS = 2048 from cli_delegate.py and the hardcoded default: 2048 from the delegate entry in skill_mapping.yaml. Both CLI routes (onex delegate and onex skill delegate) now omit max_tokens from the dispatch payload when --max-tokens is not passed, so the delegate node resolves the response budget per-backend from its routing contract. --max-tokens remains an optional explicit override: when supplied, the value is included in the payload as before. - cli_delegate.py: --max-tokens default None; _write_payload omits the key when None; signatures widened to int | None; docstrings/comments updated to drop the 'historical delegate-shim default' reference. - skill_mapping.yaml: delegate max-tokens arg loses 'default: 2048' so the field is omitted (per ModelSkillArgSpec: None default => field omitted, node supplies its own). - tests updated for the new None-default / omitted-key behavior; explicit override path still asserts max_tokens present in the payload.
|
Warning Review limit reached
More reviews will be available in 2 hours, 5 minutes, and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
OMN-13161 — drop hardcoded delegate
max_tokensconstant (CLI side)Ticket: OMN-13161 — "Delegation max_tokens belongs in the routing contract (per-backend, overlay-overridable) — not a CLI constant".
Companion PR (omnimarket, routing-authority resolution + per-backend
bifrost_delegation.yamlmax_tokens+ node contract): OmniNode-ai/omnimarket#1230This PR implements ticket item 4 (the
cli_delegate.pyCLI side) plus theparallel
skill_mapping.yamldelegate entry that carried the same hardcodeddefault.
Problem
onex delegateinjected a hardcodedDEFAULT_MAX_TOKENS = 2048into thedispatch payload, and the declarative
onex skill delegateroute carried ahardcoded
default: 2048inskill_mapping.yaml. The token budget isconfiguration and belongs on the selected backend in the routing contract
(per-backend, overlay-overridable), resolved by the delegate node after
backend selection — not as a CLI constant. The 2048 cap truncated long outputs
(a LinkedList suite was cut off) far below the local model's 128k window.
Change
src/omnibase_infra/cli/cli_delegate.pyDEFAULT_MAX_TOKENS = 2048(and its__all__entry).--max-tokensdefault →None(noshow_default); help text updated to"optional explicit override; omit to let the delegate node resolve
per-backend from its routing contract".
_write_payloadnow omits themax_tokenskey entirely when it isNone, so the node resolves the budget per-backend (relies on theomnimarket fix(ci): re-trigger auto-merge after review and checks complete [OMN-7786] #1230 resolution seam). Signatures widened to
int | None.reference; documented the omit-on-None behavior.
src/omnibase_infra/cli/skill_mapping.yamlmax-tokensarg losesdefault: 2048. PerModelSkillArgSpec, aNonedefault means the field is omitted from the payload so the nodesupplies its own — same per-backend resolution path.
--max-tokensoverride path still asserts the key is present in the payload.--max-tokensremains an optional explicit override: when passed, the value isincluded in the payload exactly as before.
Verification (run in the worktree off
origin/dev)The full
tests/run (serial) exceeds the local 10-minute harness window; theunit suite was run in parallel (
-n auto) and is green except for thedocumented env-only verification-CLI test. CI is the authority for the full
green gate.
dod_evidence (OMN-13161)
DoD per the ticket: zero hardcoded
max_tokensconstants in the CLI;--max-tokensdefault
None(omitted = routing authority decides); optional explicit overrideonly; no env var introduced. This PR satisfies the CLI-side DoD items:
DEFAULT_MAX_TOKENS = 2048removed fromcli_delegate.py(grep-verified: noremaining
DEFAULT_MAX_TOKENSreference incli_delegate.py).--max-tokensdefault is nowNone; the key is omitted from the dispatchpayload when no override is supplied (unit-test asserted, both routes).
skill_mapping.yamldelegatemax-tokensdefault: 2048removed.The per-backend resolution seam (
bifrost_delegation.yamlmax_tokens+node_delegate_skill_orchestrator dispatch resolution) is the companion omnimarket
PR #1230 (ticket items 1-3, 5). This CLI PR relies on that seam: when the CLI
omits
max_tokens, the node resolves it per-backend.Receipt Gate / Evidence-Source
This PR pairs with the OCC evidence record for OMN-13161, authored once the
companion omnimarket PR #1230 is green (the standard cross-repo OCC pairing for
this ticket family — cf. OMN-13159 / OCC#2647 which bound omnimarket #1228 +
omnibase_infra #1989). The
Evidence-Source: OCC#<num>line will be added whenthe OCC evidence PR is minted. Not faking an OCC number here.
Evidence-Ticket: OMN-13161
Evidence-Source: OCC#2651