Simplify agent reasoning and tighten channel scope - #1320
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces context-aware inbound routing scopes to pin message delivery to the current requester's channel, preventing unintended cross-channel fallbacks. It also refines the sqlite_batch tool with stricter query advisories to block inefficient per-result loops and bulk manual copies. The review feedback highlights two important improvements: initializing the routing_scope_tokens list before appending to prevent potential ContextVar leaks or NameErrors in the finally block, and replacing a nested JSON __iexact query filter with a standard equality lookup to ensure database compatibility across SQLite test environments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| routing_scope_tokens = [ | ||
| bind_inbound_routing_scope( | ||
| capture_inbound_routing_scope( | ||
| agent, | ||
| pending_inbound=( | ||
| max(routing_scope_generation, queued_inbound_generation) > consumed_inbound_generation | ||
| ), | ||
| background_before=process_started_at, | ||
| ) | ||
| ) | ||
| ] |
There was a problem hiding this comment.
Initializing routing_scope_tokens directly with the bound token can lead to ContextVar leaks or NameErrors in the finally block. If an exception occurs during the binding or during subsequent setup (such as PromptRunCache initialization) before entering the main try block, the bound token will either be leaked or cause a NameError because routing_scope_tokens was not yet defined.
To prevent this, initialize routing_scope_tokens as an empty list first, and then append the bound token. This ensures the variable is always defined and any successfully bound token is guaranteed to be cleaned up by the finally block.
| routing_scope_tokens = [ | |
| bind_inbound_routing_scope( | |
| capture_inbound_routing_scope( | |
| agent, | |
| pending_inbound=( | |
| max(routing_scope_generation, queued_inbound_generation) > consumed_inbound_generation | |
| ), | |
| background_before=process_started_at, | |
| ) | |
| ) | |
| ] | |
| routing_scope_tokens = [] | |
| routing_scope_tokens.append( | |
| bind_inbound_routing_scope( | |
| capture_inbound_routing_scope( | |
| agent, | |
| pending_inbound=( | |
| max(routing_scope_generation, queued_inbound_generation) > consumed_inbound_generation | |
| ), | |
| background_before=process_started_at, | |
| ) | |
| ) | |
| ) |
There was a problem hiding this comment.
Fixed in 89322c3 by moving the existing outer try/finally ahead of routing/cache setup, initializing both tokens safely, and conditionally resetting them. This covers the full pre-loop setup window without adding a second lifecycle. Added a regression that forces PromptRunCache setup to fail and verifies the inbound scope is cleared.
| ) | ||
| if exclude_webhooks: | ||
| messages = messages.exclude( | ||
| conversation__channel=CommsChannel.OTHER, raw_payload__source_kind__iexact="webhook" |
There was a problem hiding this comment.
Using __iexact on a nested JSONField key (like raw_payload__source_kind__iexact) is not universally supported across all database backends (especially SQLite, which is commonly used in test environments) and can lead to compatibility issues or unexpected query failures.
Since the webhook payload's source_kind is normalized to lowercase 'webhook' during ingestion, you can safely use a standard case-sensitive equality lookup (raw_payload__source_kind="webhook"). This is fully portable, highly efficient, and avoids database-specific limitations.
| conversation__channel=CommsChannel.OTHER, raw_payload__source_kind__iexact="webhook" | |
| conversation__channel=CommsChannel.OTHER, raw_payload__source_kind="webhook" |
There was a problem hiding this comment.
Fixed in 89322c3: the webhook exclusion now uses the normalized lowercase equality lookup.
1 participant
What changed
__tool_results, stable identity/provenance, and query reuse; reject per-result loops and suspicious manual row copying before execution.Scope and simplification
Representative rendered prompts are smaller than main:
This is not an overall code-deletion claim. The production safeguards add code where enforcement is needed; most changed lines are regression coverage. The production diff is 14 files (+601/-254), while tests/evals are 25 files (+2,196/-279). The prompt path itself is smaller, and the exact ceilings prevent immediate prompt growth.
Evidence
Local and CI:
29673068725passed complexity/tag guards, frontend, sandbox, all 10 Django shards, and combined resultsDeepSeek V4 Flash real harness:
c86359f6-80f2-449e-bfdf-88fe3bd371b7: 298/298 scenarios completed, 1,433/1,446 tasks passed, 13 failed, 0 skipped/errors (99.1%)2b79..., 30/30d2abc4a4-8442-433e-b50a-7a342ac41844, 27/272ff6dfb8-9834-4a7d-b617-7fe9b1452e2c, 9/9485b..., 16/18; two of three runs built and reused the keyed model, one diverted after the guard blocked a weak import245dd782-a171-44ef-a3e7-d0141511220f12/12 and HubSpot7b4a2ff9-4ff1-4b5c-a740-23f6dbc5ba3312/12; both also pass in the final full suite40394f50-031c-46aa-9cda-3be2cc9c014c: 11/12; all three runs preserved exact tool names, two delivered the verified 2-of-8 shortfall, and one over-searched without a final reportThe 13 full-suite misses are retained honestly. They cluster in known stochastic behavior: two tracked-input misses, one native Sheets discovery detour, one image edit checking an eval-only path, one recruitment source miss, and one SQLite modeling miss. Two are evaluator wording/style mismatches: a correctly formatted Sheet said "styling" rather than the accepted variants, and a readable HTML report judge demanded richer visual decoration. Focused reruns made the Sheets chart 5/5; the remaining behaviors have mixed pass/fail history on main or multi-run evidence above. No harness error or silent unsafe cross-channel send occurred.
Preview and review focus
c2cfe656c306c5f5660b4cb4eae74e02369f887029673093774deployed that SHA; Argo reports Synced/HealthyNo schema migration or UI change. Production review is concentrated in inbound routing context, send validation, SQLite query quality, compact prompt guidance, and native connection gating. Both automated review findings were addressed with focused regressions.