Add OpenClaw workflow tier routing metadata#2
Conversation
📝 WalkthroughWalkthroughThis PR adds workflow-tier classification (S/M/L/XL) and associated debug metadata to Sage Router, integrates it into routing logic and persisted route events, exposes tier/mode via response headers and /health fields, and updates README to document the new advisory metadata and clarify Router’s advisor role. ChangesWorkflow Tier Classification
Sequence DiagramsequenceDiagram
participant Client
participant Router as Sage Router
participant Classifier as Workflow Classifier
participant Decider as Route Decider
participant EventLog as Route Event Log
participant Response
Client->>Router: route_request(text, intent, complexity, requirements)
Router->>Classifier: classify_workflow_tier(...)
Classifier-->>Router: (tier, mode, agents, meta)
Router->>Decider: prepare_route() with workflow debug
Decider-->>EventLog: append_route_event(...) including workflow metadata
Decider-->>Router: selected provider & debug
Router->>Response: build response, set headers (X-Sage-Workflow-Tier/Mode)
Router-->>Client: response with workflow headers
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41f4e97c86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| total_elapsed = time.time() - overall_started | ||
| LAST_ROUTE_DEBUG.update({'selected': {'provider': pn, 'model': model}, 'status': 'ok', 'error': None, 'totalElapsedMs': round(total_elapsed * 1000.0, 2)}) | ||
| append_route_event({'request_id': request_id, 'status': 'ok', 'intent': intent.name, 'complexity': complexity.name, 'thinking': thinking.value, 'routeMode': route_mode, 'estimatedTokens': estimated_tokens, 'json': want_json, 'stream': bool(want_stream), 'requirements': requirements, 'selected': {'provider': pn, 'model': model}, 'attempts': attempts[-12:], 'totalElapsedMs': round(total_elapsed * 1000.0, 2), 'chain': [{'provider': cp, 'model': cm} for cp, cm in chain[:MAX_PROVIDER_ATTEMPTS]]}) | ||
| append_route_event({'request_id': request_id, 'status': 'ok', 'intent': intent.name, 'complexity': complexity.name, 'workflowTier': LAST_ROUTE_DEBUG.get('workflowTier'), 'workflowMode': LAST_ROUTE_DEBUG.get('workflowMode'), 'recommendedAgents': LAST_ROUTE_DEBUG.get('recommendedAgents'), 'thinking': thinking.value, 'routeMode': route_mode, 'estimatedTokens': estimated_tokens, 'json': want_json, 'stream': bool(want_stream), 'requirements': requirements, 'selected': {'provider': pn, 'model': model}, 'attempts': attempts[-12:], 'totalElapsedMs': round(total_elapsed * 1000.0, 2), 'chain': [{'provider': cp, 'model': cm} for cp, cm in chain[:MAX_PROVIDER_ATTEMPTS]]}) |
There was a problem hiding this comment.
Use request-local workflow data when appending route events
In handle_openai_chat_completions, workflow fields are read from the shared LAST_ROUTE_DEBUG dict when writing route events. Because the server runs on ThreadingHTTPServer, concurrent requests can overwrite this global between prepare_route and append_route_event, causing workflowTier/workflowMode/recommendedAgents to be logged for the wrong request. This breaks the integrity of offline routing analytics under normal concurrent traffic.
Useful? React with 👍 / 👎.
| if LAST_ROUTE_DEBUG.get('workflowTier'): | ||
| headers['X-Sage-Workflow-Tier'] = LAST_ROUTE_DEBUG.get('workflowTier') | ||
| if LAST_ROUTE_DEBUG.get('workflowMode'): | ||
| headers['X-Sage-Workflow-Mode'] = LAST_ROUTE_DEBUG.get('workflowMode') |
There was a problem hiding this comment.
Populate workflow headers from per-request routing state
routing_headers now emits X-Sage-Workflow-* from the global LAST_ROUTE_DEBUG state. In a threaded server, another in-flight request can update this global before the current response is written, so clients may receive workflow headers that belong to a different request. This can mislead downstream orchestrators that consume these headers for execution strategy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
router.py (1)
3711-3742:⚠️ Potential issue | 🟠 MajorAdd thread-safe isolation for per-request workflow metadata in
LAST_ROUTE_DEBUG.The global
LAST_ROUTE_DEBUGdict is written inprepare_route()and read inrouting_headers()with no synchronization. UnderThreadingHTTPServer, concurrent requests race to overwrite request-specific fields (request_id,intent,complexity,workflowTier,workflowMode), causing metadata leakage—e.g., response headers for request A contain request B's routing metadata.Use request-local storage (e.g.,
contextvars.ContextVaror request-scoped dict keyed byrequest_id) to isolate each request's metadata from concurrent threads.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router.py` around lines 3711 - 3742, The LAST_ROUTE_DEBUG global is being mutated concurrently in prepare_route() and read in routing_headers(), causing cross-request metadata leakage under ThreadingHTTPServer; replace the global with request-scoped storage (e.g., a contextvars.ContextVar holding a dict or a module-level dict keyed by request_id) and update all read/write sites (prepare_route(), routing_headers(), and any place that updates LAST_ROUTE_DEBUG) to use that request-local map instead of the global, include the request_id key in the context or lookup, ensure writes merge only into that request's entry (not the shared dict), and ensure the entry is cleaned up after the request completes to avoid memory growth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@router.py`:
- Around line 3658-3659: The 503 error path emits only X-Sage-Router-Request-Id
and omits workflow headers; update the failure branch so write_json includes the
same workflow headers as routing success paths by merging routing_headers() (or
at minimum X-Sage-Workflow-Tier and X-Sage-Workflow-Mode derived from
LAST_ROUTE_DEBUG) into extra_headers; locate the append_route_event /
self.write_json(...) calls and replace
extra_headers={'X-Sage-Router-Request-Id': request_id} with a merged headers
dict that includes routing_headers() (or the specific X-Sage-Workflow-* keys)
plus the request id so the workflow headers are present on 503 responses too.
- Around line 249-250: The WorkflowTier enum definition packs multiple members
on one line which violates PEP8; update the WorkflowTier Enum (symbol:
WorkflowTier) so each member is on its own line (e.g., S = auto(), M = auto(), L
= auto(), XL = auto()) replacing the single-line semicolon-separated form to
satisfy the linter and improve readability.
---
Outside diff comments:
In `@router.py`:
- Around line 3711-3742: The LAST_ROUTE_DEBUG global is being mutated
concurrently in prepare_route() and read in routing_headers(), causing
cross-request metadata leakage under ThreadingHTTPServer; replace the global
with request-scoped storage (e.g., a contextvars.ContextVar holding a dict or a
module-level dict keyed by request_id) and update all read/write sites
(prepare_route(), routing_headers(), and any place that updates
LAST_ROUTE_DEBUG) to use that request-local map instead of the global, include
the request_id key in the context or lookup, ensure writes merge only into that
request's entry (not the shared dict), and ensure the entry is cleaned up after
the request completes to avoid memory growth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| class WorkflowTier(Enum): | ||
| S = auto(); M = auto(); L = auto(); XL = auto() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the exact semicolon-packed enum line causing E702
rg -n "class WorkflowTier|S = auto\\(\\); M = auto\\(\\); L = auto\\(\\); XL = auto\\(\\)" router.py -n -C2Repository: earlvanze/sage-router
Length of output: 323
Split WorkflowTier enum members onto separate lines.
Line 250 packs multiple enum members separated by semicolons on a single line, which violates PEP 8 and will trigger linter rules if lint enforcement is enabled.
Suggested fix
class WorkflowTier(Enum):
- S = auto(); M = auto(); L = auto(); XL = auto()
+ S = auto()
+ M = auto()
+ L = auto()
+ XL = auto()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class WorkflowTier(Enum): | |
| S = auto(); M = auto(); L = auto(); XL = auto() | |
| class WorkflowTier(Enum): | |
| S = auto() | |
| M = auto() | |
| L = auto() | |
| XL = auto() |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 250-250: Multiple statements on one line (semicolon)
(E702)
[error] 250-250: Multiple statements on one line (semicolon)
(E702)
[error] 250-250: Multiple statements on one line (semicolon)
(E702)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router.py` around lines 249 - 250, The WorkflowTier enum definition packs
multiple members on one line which violates PEP8; update the WorkflowTier Enum
(symbol: WorkflowTier) so each member is on its own line (e.g., S = auto(), M =
auto(), L = auto(), XL = auto()) replacing the single-line semicolon-separated
form to satisfy the linter and improve readability.
| append_route_event({'request_id': request_id, 'status': 'failed', 'intent': intent.name, 'complexity': complexity.name, 'workflowTier': LAST_ROUTE_DEBUG.get('workflowTier'), 'workflowMode': LAST_ROUTE_DEBUG.get('workflowMode'), 'recommendedAgents': LAST_ROUTE_DEBUG.get('recommendedAgents'), 'thinking': thinking.value, 'routeMode': route_mode, 'estimatedTokens': estimated_tokens, 'json': want_json, 'stream': bool(want_stream), 'requirements': requirements, 'selected': None, 'attempts': attempts[-12:], 'totalElapsedMs': round(total_elapsed * 1000.0, 2), 'chain': [{'provider': cp, 'model': cm} for cp, cm in chain[:MAX_PROVIDER_ATTEMPTS]], 'error': 'All providers failed'}) | ||
| self.write_json(503, {'error': 'All providers failed', 'request_id': request_id, 'attempts': attempts, 'choices': [{'message': {'content': 'Error: No providers available'}}]}, extra_headers={'X-Sage-Router-Request-Id': request_id}) |
There was a problem hiding this comment.
Emit workflow headers on 503 responses too.
On failure, this path sends only X-Sage-Router-Request-Id; it bypasses routing_headers(), so X-Sage-Workflow-Tier / X-Sage-Workflow-Mode are missing exactly when routing fails.
Suggested fix
- self.write_json(503, {'error': 'All providers failed', 'request_id': request_id, 'attempts': attempts, 'choices': [{'message': {'content': 'Error: No providers available'}}]}, extra_headers={'X-Sage-Router-Request-Id': request_id})
+ failure_payload = {'error': 'All providers failed', 'request_id': request_id, 'attempts': attempts, 'choices': [{'message': {'content': 'Error: No providers available'}}]}
+ self.write_json(503, failure_payload, extra_headers=self.routing_headers(failure_payload, request_id))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router.py` around lines 3658 - 3659, The 503 error path emits only
X-Sage-Router-Request-Id and omits workflow headers; update the failure branch
so write_json includes the same workflow headers as routing success paths by
merging routing_headers() (or at minimum X-Sage-Workflow-Tier and
X-Sage-Workflow-Mode derived from LAST_ROUTE_DEBUG) into extra_headers; locate
the append_route_event / self.write_json(...) calls and replace
extra_headers={'X-Sage-Router-Request-Id': request_id} with a merged headers
dict that includes routing_headers() (or the specific X-Sage-Workflow-* keys)
plus the request id so the workflow headers are present on 503 responses too.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f501fb44d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rejections = forced_rejections | ||
| logger.info(f"[{request_id}] Chain (forced): {chain}") | ||
| LAST_ROUTE_DEBUG.update({'updated_at': int(time.time()), 'request_id': request_id, 'intent': intent.name, 'complexity': complexity.name, 'thinking': thinking.value, 'routeMode': route_mode, 'requirements': requirements, 'estimatedTokens': estimated_tokens, 'json': want_json, 'chain': chain, 'scores': score_debug, 'rejections': rejections[:30], 'selected': None, 'attempts': [], 'streaming': streaming_mode or ('buffered-wrapper' if requirements.get('streaming') else 'disabled'), 'status': 'routing', 'error': None, 'totalElapsedMs': None, 'forcedProvider': force_provider}) | ||
| LAST_ROUTE_DEBUG.update({'updated_at': int(time.time()), 'request_id': request_id, 'intent': intent.name, 'complexity': complexity.name, **workflow_debug, 'thinking': thinking.value, 'routeMode': route_mode, 'requirements': requirements, 'estimatedTokens': estimated_tokens, 'json': want_json, 'chain': chain, 'scores': score_debug, 'rejections': [], 'selected': None, 'attempts': [], 'streaming': streaming_mode or ('buffered-wrapper' if requirements.get('streaming') else 'disabled'), 'status': 'routing', 'error': None, 'totalElapsedMs': None, 'forcedProvider': force_provider}) |
There was a problem hiding this comment.
Preserve forced-provider rejection reasons in debug state
This update drops forced_rejections by writing 'rejections': [] into LAST_ROUTE_DEBUG for forced-provider routing, so when every candidate model is filtered out (disabled, not chat-capable, requirement mismatch, etc.) /health.lastRoute no longer reports why routing failed. The previous behavior preserved those rejection reasons, and losing them makes production routing failures much harder to diagnose.
Useful? React with 👍 / 👎.
| total_elapsed = time.time() - overall_started | ||
| LAST_ROUTE_DEBUG.update({'selected': None, 'attempts': attempts[-12:], 'status': 'failed', 'error': 'All providers failed', 'totalElapsedMs': round(total_elapsed * 1000.0, 2)}) | ||
| append_route_event({'request_id': request_id, 'status': 'failed', 'intent': intent.name, 'complexity': complexity.name, 'thinking': thinking.value, 'routeMode': route_mode, 'estimatedTokens': estimated_tokens, 'json': want_json, 'stream': bool(want_stream), 'requirements': requirements, 'selected': None, 'attempts': attempts[-12:], 'totalElapsedMs': round(total_elapsed * 1000.0, 2), 'chain': [{'provider': cp, 'model': cm} for cp, cm in chain[:MAX_PROVIDER_ATTEMPTS]], 'error': 'All providers failed'}) | ||
| append_route_event({'request_id': request_id, 'status': 'failed', 'intent': intent.name, 'complexity': complexity.name, 'workflowTier': LAST_ROUTE_DEBUG.get('workflowTier'), 'workflowMode': LAST_ROUTE_DEBUG.get('workflowMode'), 'recommendedAgents': LAST_ROUTE_DEBUG.get('recommendedAgents'), 'thinking': thinking.value, 'routeMode': route_mode, 'estimatedTokens': estimated_tokens, 'json': want_json, 'stream': bool(want_stream), 'requirements': requirements, 'selected': None, 'attempts': attempts[-12:], 'totalElapsedMs': round(total_elapsed * 1000.0, 2), 'chain': [{'provider': cp, 'model': cm} for cp, cm in chain[:MAX_PROVIDER_ATTEMPTS]], 'error': 'All providers failed'}) |
There was a problem hiding this comment.
Prevent stale workflow fields in failed route events
These failed-event payloads now read workflow metadata from LAST_ROUTE_DEBUG, but prepare_route's forced-provider/local-first rejection path updates LAST_ROUTE_DEBUG without setting workflowTier/workflowMode/recommendedAgents (it only sets intent/complexity/rejections). In that branch, a failed request can inherit workflow values from a prior request, producing incorrect event data even without concurrent traffic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
router.py (1)
5149-5161:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThese workflow fields are dropped before persistence.
The new
workflowTier/workflowMode/recommendedAgentsfields added here never surviveappend_route_event(), becausesanitize_route_event()still strips them. That means the JSONL, Firestore, and Supabase mirrors will not contain the metadata this PR is adding.Suggested fix
def sanitize_route_event(event): """Keep analytics useful while explicitly excluding prompt/user content and credentials.""" allowed = { 'request_id', 'ts', 'status', 'intent', 'complexity', 'thinking', 'routeMode', 'estimatedTokens', 'json', 'stream', 'requirements', 'selected', 'attempts', - 'totalElapsedMs', 'chain', 'error' + 'totalElapsedMs', 'chain', 'error', + 'workflowTier', 'workflowMode', 'recommendedAgents', 'workflow', }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router.py` around lines 5149 - 5161, The new workflow fields ('workflowTier', 'workflowMode', 'recommendedAgents') are being added to events before calling append_route_event(...) but are removed by sanitize_route_event(), so they never persist; update sanitize_route_event (and any related sanitizer used by append_route_event) to allow and pass-through these keys (workflowTier, workflowMode, recommendedAgents) when present (or add them to the allowed/whitelist of preserved fields), and ensure append_route_event and LAST_ROUTE_DEBUG usage keep these fields intact when building events and when serializing to JSONL/Firestore/Supabase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@router.py`:
- Around line 5214-5215: The code currently writes per-request workflow metadata
into the global LAST_ROUTE_DEBUG (via workflow_route_debug_fields and the
workflow_debug variable) which causes cross-request leaks under
ThreadingHTTPServer; instead, stop mutating the global and keep the metadata
request-scoped: compute workflow_debug with
workflow_route_debug_fields(request_id, user_text, intent, complexity,
requirements) and attach it only to the current response (e.g., set response
headers X-Sage-Workflow-* and lastRoute on the response object or store on a
request-local context/threading.local) rather than assigning LAST_ROUTE_DEBUG;
apply the same change where similar writes occur (the other occurrences around
the same block noted in the comment).
---
Outside diff comments:
In `@router.py`:
- Around line 5149-5161: The new workflow fields ('workflowTier',
'workflowMode', 'recommendedAgents') are being added to events before calling
append_route_event(...) but are removed by sanitize_route_event(), so they never
persist; update sanitize_route_event (and any related sanitizer used by
append_route_event) to allow and pass-through these keys (workflowTier,
workflowMode, recommendedAgents) when present (or add them to the
allowed/whitelist of preserved fields), and ensure append_route_event and
LAST_ROUTE_DEBUG usage keep these fields intact when building events and when
serializing to JSONL/Firestore/Supabase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aba958ec-e871-4258-994d-51c71ce525ab
📒 Files selected for processing (2)
README.mdrouter.py
✅ Files skipped from review due to trivial changes (1)
- README.md
| workflow_debug = workflow_route_debug_fields(user_text, intent, complexity, requirements) | ||
| logger.info(f"[{request_id}] Intent: {intent.name}, Complexity: {complexity.name}, Thinking: {thinking.value}, Route: {route_mode}, JSON: {want_json}, EstTokens: {estimated_tokens}, ForceProvider: {force_provider or 'none'}") |
There was a problem hiding this comment.
Make workflow debug request-scoped.
These updates write per-request workflow metadata into the global LAST_ROUTE_DEBUG, but this server uses ThreadingHTTPServer. Concurrent requests can overwrite each other before headers or /health are emitted, so X-Sage-Workflow-* and lastRoute can describe the wrong request.
Also applies to: 5234-5234, 5263-5267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router.py` around lines 5214 - 5215, The code currently writes per-request
workflow metadata into the global LAST_ROUTE_DEBUG (via
workflow_route_debug_fields and the workflow_debug variable) which causes
cross-request leaks under ThreadingHTTPServer; instead, stop mutating the global
and keep the metadata request-scoped: compute workflow_debug with
workflow_route_debug_fields(request_id, user_text, intent, complexity,
requirements) and attach it only to the current response (e.g., set response
headers X-Sage-Workflow-* and lastRoute on the response object or store on a
request-local context/threading.local) rather than assigning LAST_ROUTE_DEBUG;
apply the same change where similar writes occur (the other occurrences around
the same block noted in the comment).
Adds advisory S/M/L/XL workflow tier metadata for OpenClaw Alp River-style orchestration.\n\n- Adds WorkflowTier classification\n- Adds workflow mode and recommended specialist agents to lastRoute\n- Emits X-Sage-Workflow-Tier and X-Sage-Workflow-Mode headers\n- Keeps sage-router as advisor, not orchestrator\n\nValidation: python3 -m py_compile router.py
Summary by CodeRabbit
New Features
Documentation