ref(agent): canonicalize request language and pending answers (toby) - #2076
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a structured mechanism for handling pending user responses and request-language policies, including the addition of the PendingUserResponse dataclass, serialization helpers, and DAG marker propagation logic, along with comprehensive unit tests. The feedback suggests a robustness improvement in dag.py to safely handle cases where message.metadata might be None to prevent potential TypeError exceptions during dictionary unpacking.
rogercloud
left a comment
There was a problem hiding this comment.
Summary
This PR is rollout Layer B of a 3-layer request-language-canonicalization effort (parent #2062, Layer A #2068 merged). It adds a PendingUserResponse dataclass + pending_user_response() parser (src/xagent/core/agent/context/enrichment.py), a canonical prose language policy + JSON request-language renderer (src/xagent/core/agent/language.py), and changes to the DAG-step-restore path in src/xagent/core/agent/pattern/dag/dag.py that write a response_to_waiting_for_user marker onto ROOT context messages when forwarding a user's response to a waiting DAG step. The PR claims to be "behavior-neutral," deferring activation to a future issue (#2064), per issue #2063's explicit exclusion of root/Auto/ReAct/DAG-planner/DAG-step/completion/final-answer call sites.
Verified: the language.py/enrichment.py half is genuinely inert (zero production callers). The dag.py change is not inert — it is wired into the live DAGPattern._run resume-from-waiting-for-user path (dag.py:500,542) and mutates root_context.messages, which has three pre-existing production readers that change LLM prompt content and language-anchor selection. This contradicts the stated "behavior-neutral" claim.
Blocking: yes — recommended event: REQUEST_CHANGES
Round 0 design verdict: Acceptable-with-reservations, bordering on wrong-direction for the dag.py hunk. The library half (language.py/enrichment.py) is genuinely inert and matches the stated Layer B scope. The dag.py root-mutation half is not inert and needs to either be scoped to the child context only, or moved into the activation layer (#2064) with its own review.
Blocking findings
B1 — src/xagent/core/agent/pattern/dag/dag.py:1900 (Major, Blocking: yes)
TypeError on None metadata — matches an unresolved prior gemini-code-assist bot review comment.
metadata = {
**getattr(message, "metadata", {}),
"response_to_waiting_for_user": marker,
}getattr(message, "metadata", {}) only falls back to {} when the attribute is absent. If message.metadata is explicitly None (reachable via a checkpoint payload with "metadata": null — ExecutionContext.from_dict does item.get("metadata", {}), which does not guard an explicit null, and add_user_message forwards metadata=None unvalidated), getattr returns None and **None raises TypeError: 'NoneType' object is not a mapping. Reproduced.
The sibling implementation at src/xagent/core/agent/pattern/react/react.py:1742 already guards with dict(getattr(message, "metadata", {}) or {}), and a nearby helper in the same file (dag.py:1938, _refresh_restored_step_instruction) also guards correctly with getattr(message, "metadata", None) or {}. This is the same issue gemini-code-assist's bot review already flagged inline on an earlier revision (then line 1902) — it was never fixed or replied to.
Trigger: any _forward_user_response_to_waiting_step call where the target message has metadata=None (reachable via checkpoint restore of legacy/malformed data).
Impact: unhandled TypeError breaks the user-response-forwarding flow for that DAG step.
Suggested fix: **(getattr(message, "metadata", None) or {}).
B2 — src/xagent/core/agent/pattern/dag/dag.py:1877-1908 (Major, Blocking: yes)
Root-context mutation is not behavior-neutral; contradicts PR/issue scope.
The new marker-writing loop mutates root_context.messages in place (root_context.messages[root_index] = replace(message, metadata=metadata) at line 1908), not just the DAG step's private child context. This is reachable from production (DAGPattern._run at dag.py:500,542, the normal resume-from-waiting-for-user path).
The response_to_waiting_for_user metadata key already has three live readers:
src/xagent/core/agent/context/execution.py:455-467(get_messages_for_llmrewrites the marked message's LLM-facing content into a "pending question / user answer" wrapper)src/xagent/core/agent/context/execution.py:552(skips marked messages when picking the language anchor)src/xagent/core/agent/context/enrichment.py:232(top_level_user_requestskips marked messages, flipshas_pending_response)
Empirically confirmed via repro: before this mutation, the root LLM prompt content is the user's raw text and top_level_user_request reports has_pending_response=False; after, the content is rewritten with the "pending agent question" framing and has_pending_response=True. This directly contradicts the PR description's own "behavior-neutral... Root... consumers are unchanged" claim, and issue #2063's explicit exclusion ("No activation in root ... call sites").
Trigger: any DAG run that reaches waiting_for_user and resumes with a new user message (the ordinary DAG pause/resume flow).
Impact: changed LLM prompt content and language-anchor selection on the root context for DAG replan/completion/final-answer calls — a real, silent behavioral change on a path the PR and its issue both explicitly scope out.
Suggested fix: scope this mutation to the child context only (matching the stated Layer B scope), or move it to the activation layer (#2064) with its own tests and an updated PR description acknowledging the behavior change.
B3 — src/xagent/core/agent/context/enrichment.py:232 (Major, Blocking: yes)
Predicate tightening creates 3-way reader disagreement on malformed markers.
- if metadata.get("response_to_waiting_for_user"):
+ if pending_user_response(message) is not None:This tightens top_level_user_request()'s check from "any truthy value" to "a dict with a non-blank str question, on a role=='user' message, with str content." Two other readers of the same key are untouched by this PR and still use looser checks: execution.py:455-458 (isinstance(waiting_response, dict) only) and execution.py:552 (old bare truthy check if message.metadata.get("response_to_waiting_for_user"):).
Concrete disagreement confirmed for marker {"question": ""} on a user message: pending_user_response() returns None (blank question fails validation), so top_level_user_request() treats the message as a fresh top-level request; but execution.py:455-458's looser isinstance(dict) check still treats it as pending and rewrites its LLM content with "pending question" framing, and execution.py:552 also still treats it as pending. This directly contradicts issue #2063's own acceptance criterion "Blank or legacy marker shapes degrade safely" — degradation is inconsistent across the three readers, not safe/uniform.
Trigger: any legacy/malformed response_to_waiting_for_user marker (e.g. {}, {"question": ""}, True, "legacy") on a user message — plausible given this is explicitly a canonicalization/degradation-hardening PR.
Impact: top_level_user_request() and get_messages_for_llm/language-anchor selection disagree on whether the same message is a pending answer vs. a new request, causing inconsistent language/content handling for that turn.
Suggested fix: align all three readers on the same validated predicate (e.g. have execution.py reuse pending_user_response() instead of its own looser checks).
Non-blocking findings (Minor)
M1 — src/xagent/core/agent/pattern/dag/dag.py:1886-1917 — Hydration-ordering staleness. _refresh_restored_step_runtime_metadata runs before the new marker-mutation loop, so when both root and child lack a stored TopLevelUserRequest snapshot (essentially only a legacy pre-migration checkpoint restore), the persisted snapshot can briefly disagree with a fresh recompute. Self-correcting on the next top_level_user_request() call since it recomputes from messages rather than trusting the cache. Low real-world impact; worth a comment rather than a required fix.
M2 — src/xagent/core/agent/pattern/dag/dag.py:1895-1901 vs src/xagent/core/agent/pattern/react/react.py:1746-1751 — Duplicated, inconsistent marker construction. react.py builds the same marker with a 6-key shape (including tool_call_id/interactions/requests) plus an idempotency guard skipping already-marked messages; this PR adds a second, independent 2-key implementation in dag.py with no shared constant/helper for the waiting_request["message"] → marker["question"] mapping and no idempotency guard. Recommend extracting a shared pending_user_response_marker(waiting_request) helper into enrichment.py to prevent future field-name drift between the two call sites.
M3 — src/xagent/core/agent/context/enrichment.py:35,224,233,247,254,265,275 — has_pending_response is a dead, non-persisted field. Every occurrence is a write (field default, local assignment, or constructor kwarg); zero reads anywhere in src/ or tests/. Also not round-tripped through checkpoint persistence (_persist_top_level_user_request omits it; _stored_top_level_user_request always reconstructs with the dataclass default False). Harmless today, but a trap for the future activation layer (#2064) if someone assumes it reflects real state after a restore.
M4 — src/xagent/core/agent/pattern/dag/dag.py:1903-1907 — Fragile next() scan with no default:
root_index = next(
index
for index, root_message in enumerate(root_context.messages)
if root_message is message
)Currently safe only because root_user_messages is filtered directly from root_context.messages via a list comprehension, preserving object identity. Any future refactor that rebuilds/copies the messages list between the filter and this loop turns this into an uncaught StopIteration. Also an avoidable O(n·m) rescan — see the simplification note below, which eliminates this entirely.
M5 — src/xagent/core/agent/pattern/dag/dag.py:1890-1917 — Marker applied uniformly to every forwarded message, not just the answer. If two user messages arrive while a DAG step is waiting (AgentRunner.inject_user_message at runner.py:480-583 does not serialize resume cycles), all messages in root_user_messages[self.planned_user_message_count:] get the same "answer to this pending question" marker and LLM-content rewrite, even though only one is genuinely the answer. Self-correcting on the next turn (framing issue, not data loss); worth fixing by marking only the first new message.
M6 — src/xagent/core/agent/pattern/dag/dag.py:1896 vs src/xagent/core/agent/context/enrichment.py:59-60 vs src/xagent/core/agent/context/execution.py:459 — Non-str question write/read inconsistency. dag.py writes waiting_request.get("message", "") into the marker with no type validation (only reachable via a corrupted/legacy checkpoint restored into active_step_pattern_states). enrichment.py's pending_user_response() rejects non-str questions; execution.py:459's get_messages_for_llm instead coerces with str(...), so it would inject stringified garbage into the LLM prompt for a marker enrichment.py would reject. Narrow, non-blocking, but worth a validation guard at the write site for pipeline consistency.
M7 — src/xagent/core/agent/language.py:227-241 — render_request_language_harness has two unreconciled "is pending" signals: request: TopLevelUserRequest (carrying has_pending_response: bool) and a separate pending_response parameter, with no consistency check between them — calling it with request.has_pending_response=True, pending_response=None silently renders a harness with no pending-response evidence.
M8 — src/xagent/core/agent/language.py:238-241 — Undocumented single-line-JSON format invariant. The harness output ("...evidence (JSON):\n" + json.dumps(evidence) + "\n\n" + policy) requires the JSON to stay on exactly one line — tests/core/agent/test_request_language_policy.py:89-90,117 parse it via harness.split("\n", 2)[1]. Not documented in the docstring or asserted in the function; a future indent=2 would silently break the contract and surface as a confusing JSONDecodeError in tests.
M9 — Test coverage gaps (tests/core/agent/test_dag.py, tests/core/agent/test_request_language_policy.py):
- No test constructs a message with
metadata=None, so B1's TypeError path is uncovered. test_dag_waiting_response_preserves_active_step_statenever asserts onroot_contextafter forwarding — it only checks the restored child — so it would still pass if the root-mutation line were deleted (thoughtest_dag_marker_is_propagated_symmetrically_without_internal_fieldsin test_request_language_policy.py:161 does cover the raw metadata write, just not its downstream behavioral consequence per B2/B3).- No test calls
top_level_user_request()orget_messages_for_llm()after a forwarding/restore call to verify the actual behavioral consequence described in B2. - No test covers the hydration-ordering issue (M1).
- The malformed-marker parametrize list (test_request_language_policy.py:68-71:
[True, False, "legacy", 1, None, {}, {"question": " \n"}]) never includes a non-strquestionormessage_type(M6). test_explicit_answer_override_and_city_negative_control_share_one_policy(lines 101-109) asserts literal substrings of the fixed policy prose — it can only fail if someone edits the wording, and proves nothing about actual language-selection behavior.test_new_policy_is_not_active_in_existing_consumers(lines 124-131) is misleadingly named: it only exercises a bareExecutionContext._system_context()/get_messages_for_llm(), not any of the Root/Auto/ReAct/DAG-planner/DAG-step/completion/final-answer call sites the name and issue #2063 imply — and per B2, the PR is active on one of those (DAG), just not via this renderer.
Simplification opportunities
src/xagent/core/agent/language.py
L196: stdlib hand-rolled dict construction from a frozen dataclass's fields. Use `dataclasses.asdict(response)`.
L205: shrink a zero-argument function that only returns a fixed string. Make it a module-level constant string instead of a function.
src/xagent/core/agent/pattern/dag/dag.py
L1877: shrink builds `root_user_messages` by filtering without indices, then re-finds each message's index via an O(n) identity scan (`next(... if root_message is message)`) inside the loop. Collect `(index, message)` pairs while filtering (e.g. `[(i, m) for i, m in enumerate(root_context.messages) if m.role == "user"]`) and reuse the index directly instead of re-searching — this also eliminates M4's fragile unguarded `next()`.
net: -3 lines possible (roughly; the dag.py fix is more about removing fragility/duplication than raw line count, but it does remove the whole next() block)
Review criteria coverage note
No issues found in: security/secrets handling (the allowlist design is a net privacy improvement — it excludes tool_name/tool_call_id/interactions/requests/event_id from the exposed pending-response view), import cycles, naming, concurrency/async, type annotations, formatting/lint.
Blocking status & recommended decision
Blocking: yes
Recommended event: REQUEST_CHANGES
src/xagent/core/agent/pattern/dag/dag.py:1900(major) — TypeError onNonemetadata, matches unresolved prior gemini-code-assist review comment [prior]src/xagent/core/agent/pattern/dag/dag.py:1877-1908(major) — root-context mutation contradicts stated behavior-neutral scope [new]src/xagent/core/agent/context/enrichment.py:232(major) — predicate tightening creates 3-way reader disagreement on malformed markers, contradicts issue #2063 acceptance criterion [new]
|
Addressed the requested changes in c4ef6dd. Finding disposition:
Validation completed:
|
|
Review round summary Fixed:
Superseded by the DAG removal:
Declined or deferred:
Validation:
Final diff: 3 files, 292 additions, no production consumer activation. |
rogercloud
left a comment
There was a problem hiding this comment.
PR #2076 implements Layer B of request-language isolation: it keeps the canonical user-authored request and pending-response evidence separate from enriched execution text, serializes a three-field pending projection through an explicit allowlist, and centralizes the unpinned language policy while leaving production consumers unchanged for Layer C. The staged approach is coherent, but the pending projection currently selects the enriched channel and the promised renderer surface is incomplete; both are material, non-blocking reservations to fix before activation. Blocking: no — recommended event: APPROVE
Update summary
Since the prior reviewed head d7096e5, c4ef6ddd removes the root/child DAG pending-marker propagation path, preserving behavior-neutral forwarding instead of mutating live root context. It restores the Layer-A truthy-marker predicate in top_level_user_request() and adds focused parser, policy, and non-activation tests, clearing the prior DAG metadata/reader mismatch findings while keeping Layer-B activation deferred to #2064.
Round 0 approach verdict
acceptable-with-reservations. Reusing Layer-A provenance, a frozen three-field pending value, an explicit allowlist, and one canonical policy is the right staged direction; keeping the new representation inactive avoids changing current prompt consumers. The reservations are that pending_user_response() reads enriched Message.content rather than preserved display text, and that only a request-only harness is provided despite #2063's required renderer variants. Both are reported below and are non-blocking because no production caller is active in this PR.
Prior-findings checklist
| Root | Status | Prior anchor | Source and reply citations |
|---|---|---|---|
B1 — DAG marker writer unpacks explicit metadata=None with ** and raises TypeError |
FIXED (prior major; prior Blocking: yes) | src/xagent/core/agent/pattern/dag/dag.py:1900 (earlier original line 1902) |
Sources: review:5098806446:summary-feedback, review:5099122341:B1, inline:3921929314; replies: inline:3922358260, conversation:5522534705:B1 |
B2 — Layer-B DAG resume mutates live root_context with a pending marker |
FIXED (prior major; prior Blocking: yes) | src/xagent/core/agent/pattern/dag/dag.py:1877-1908 |
Source: review:5099122341:B2; replies: conversation:5522534705:B2, conversation:5522537406:fixed-DAG-marker-propagation |
| B3 — Strict pending parser disagreed with permissive readers on malformed markers | FIXED (prior major; prior Blocking: yes) | src/xagent/core/agent/context/enrichment.py:232 |
Source: review:5099122341:B3; reply: conversation:5522534705:B3 |
| M1 — DAG metadata refresh preceded the marker and could leave a stale snapshot | FIXED | src/xagent/core/agent/pattern/dag/dag.py:1886-1917 |
Source: review:5099122341:M1; replies: conversation:5522534705:M1, conversation:5522537406:superseded-DAG-removal |
| M2 — DAG and ReAct independently constructed inconsistent pending markers | DROPPED | src/xagent/core/agent/pattern/dag/dag.py:1895-1901 |
Source: review:5099122341:M2; replies: conversation:5522534705:M2, conversation:5522537406:superseded-DAG-removal |
M3 — TopLevelUserRequest.has_pending_response was written but not read/persisted through reconstruction (tracked in #2064) |
DROPPED | src/xagent/core/agent/context/enrichment.py:35,224,233,247,254,265,275 |
Source: review:5099122341:M3; replies: conversation:5522534705:M3, conversation:5522537406:deferred-renderer-state; tracking: #2064 |
M4 — DAG loop re-scanned message identity with unguarded next() and could raise StopIteration |
FIXED | src/xagent/core/agent/pattern/dag/dag.py:1903-1907 |
Source: review:5099122341:M4; replies: conversation:5522534705:M4, conversation:5522537406:superseded-DAG-removal |
| M5 — DAG marked every forwarded user message as the answer instead of only the actual response | FIXED | src/xagent/core/agent/pattern/dag/dag.py:1890-1917 |
Source: review:5099122341:M5; replies: conversation:5522534705:M5, conversation:5522537406:superseded-DAG-removal |
| M6 — DAG marker accepted a non-string question while enrichment and execution disagreed on types | DROPPED | src/xagent/core/agent/pattern/dag/dag.py:1896; src/xagent/core/agent/context/enrichment.py:59-60; src/xagent/core/agent/context/execution.py:459 |
Source: review:5099122341:M6; replies: conversation:5522534705:M6, conversation:5522537406:superseded-DAG-removal |
M7 — Renderer accepted has_pending_response plus a separate pending_response without a consistency contract (tracked in #2064) |
DROPPED | src/xagent/core/agent/language.py:227-241 |
Source: review:5099122341:M7; replies: conversation:5522534705:M7, conversation:5522537406:deferred-renderer-state; tracking: #2064 |
| M8 — Renderer relied on an undocumented single-line JSON layout | DROPPED | src/xagent/core/agent/language.py:238-241; tests/core/agent/test_request_language_policy.py:89-90,117 |
Source: review:5099122341:M8; replies: conversation:5522534705:M8, conversation:5522537406:declined-single-line-contract |
M9a — Focused tests did not construct metadata=None for the failure branch |
FIXED | tests/core/agent/test_dag.py; tests/core/agent/test_request_language_policy.py |
Source: review:5099122341:M9:bullet-metadata-none; reply: conversation:5522534705:M9:addressed-relevant |
M9b — DAG waiting-response test did not check root_context after forwarding |
FIXED | tests/core/agent/test_dag.py:test_dag_waiting_response_preserves_active_step_state |
Source: review:5099122341:M9:bullet-root-after-forward; reply: conversation:5522534705:M9:addressed-relevant |
| M9c — Tests did not call downstream readers after DAG forwarding | DROPPED | tests/core/agent/test_dag.py; tests/core/agent/test_request_language_policy.py:161 |
Source: review:5099122341:M9:bullet-downstream-readers; reply: conversation:5522534705:M9:addressed-relevant |
| M9d — Tests did not cover the hydration-order stale-snapshot scenario | DROPPED | tests/core/agent/test_dag.py |
Source: review:5099122341:M9:bullet-hydration-order; reply: conversation:5522534705:M9:addressed-relevant |
| M9e — Malformed-marker tests omitted non-string question/message-type cases | FIXED | tests/core/agent/test_request_language_policy.py:68-71 |
Source: review:5099122341:M9:bullet-malformed-types; reply: conversation:5522534705:M9:addressed-relevant |
| M9f — Tests asserted literal policy prose and one city example instead of semantic branches | DROPPED | tests/core/agent/test_request_language_policy.py:101-109 |
Source: review:5099122341:M9:bullet-literal-policy; replies: conversation:5522534705:M9:addressed-relevant, conversation:5522537406:literal-policy-intentional |
| M9g — Non-activation test name/scope did not exercise all named consumers | FIXED | tests/core/agent/test_request_language_policy.py:124-131 |
Source: review:5099122341:M9:bullet-nonactivation-test; reply: conversation:5522534705:M9:addressed-relevant |
Prior simplification status (S1-S3)
All prior simplification candidates were independently dropped; none is carried forward as a finding.
| Root | Status | Prior anchor | Source and reply citations |
|---|---|---|---|
S1 — Explicit three-field serializer allowlist could use dataclasses.asdict |
DROPPED | src/xagent/core/agent/language.py:196 |
Source: review:5099122341:simplification:language.py:196; replies: conversation:5522534705:asdict-declined, conversation:5522537406:asdict-declined |
| S2 — Zero-argument fixed policy function could be a module constant | DROPPED | src/xagent/core/agent/language.py:205 |
Source: review:5099122341:simplification:language.py:205; replies: conversation:5522534705:constant-declined, conversation:5522537406:constant-declined |
| S3 — DAG filtering plus identity rescan could reuse indices directly | DROPPED | src/xagent/core/agent/pattern/dag/dag.py:1877 |
Source: review:5099122341:simplification:dag.py:1877; replies: conversation:5522534705:indexed-scan-superseded, conversation:5522537406:superseded-DAG-removal |
Fresh confirmed findings
1. Pending answer uses enriched execution content instead of the clean display answer
Location: src/xagent/core/agent/context/enrichment.py:68
Severity: major
Blocking: no
Reachable trigger: On a supported WebSocket WAITING_FOR_USER resume with a nonblank answer and an uploaded file or connector enrichment, websocket.py:6110-6133,6590-6699,6771-6785 passes a file-enriched execution_message and clean display_message. runner.py:510-528,567-583 stores them as Message.content and metadata["display_message"], and ReAct's marker-copy path at react.py:1728-1755 preserves that metadata. The new parser then assigns answer = getattr(message, "content", "") at line 68.
Concrete impact: serialize_pending_user_response() and render_request_language_harness() emit that enriched block as pending_response.answer (language.py:196-202,227-240). A future #2064 consumer can therefore treat an uploaded filename, file instructions, or opposite-language connector text as the user's pending answer and infer/select the wrong output language, defeating the clean-versus-execution provenance boundary required by #2062 and #2064. The new representation has no subsequent provenance delimiter that can repair the contamination.
Why the contract permits it: The ingress contract explicitly permits execution_message to contain enrichment while preserving the user-authored display channel; there is no type or upstream validation that removes file/connector text from Message.content. The existing execution path correctly consumes Message.content, but the new language-evidence projection must consume the display channel instead. The helper is inactive in this PR, so this is a major contract defect without a current user-visible failure and remains non-blocking.
Exact fix: At line 68, select the canonical display channel via display_message_override(metadata) or an equivalent tri-state helper: a present display value must win, including an explicit blank/whitespace value, and only genuinely absent display provenance may fall back to Message.content. Leave Message.content enriched for ordinary execution. Add clean-vs-enriched, explicit blank/whitespace, and checkpoint/marker round-trip regressions asserting that serialized pending answer contains only the clean answer.
2. Layer-B renderer surface is incomplete
Location: src/xagent/core/agent/language.py:227
Severity: major
Blocking: no
Reachable trigger: When the planned #2064 activation wires the canonical representation into root-reference, structured planner/completion, and DAG-step surfaces, #2063 requires those consumers to use request-only, structured-field, root-reference, and DAG-step renderings. The only new renderer at line 227 is render_request_language_harness(), which always embeds the full independent_user_request and optional pending_response JSON.
Concrete impact: The structured/root/DAG-step consumers cannot consume the promised single canonical representation without either duplicating the full request in each prompt, reusing pre-existing noncanonical output_language_directives() prose, or redesigning Layer B inside Layer C. That creates parallel policy/provenance logic and leaves #2063's explicit acceptance/API contract incomplete. Because no production path calls the new helpers yet and current consumers intentionally remain unchanged, the impact is a material staged-contract failure rather than a present user-visible blocker.
Why the contract permits it: #2063 remains in force and explicitly requires the four rendering forms; the PR claims to close it, while #2064 defers activation, not the Layer-B renderer definitions. The existing output_language_directives() surface is pre-existing, delegates to old policy helpers, and accepts neither TopLevelUserRequest nor PendingUserResponse; it is not an equivalent implementation. Thus the missing forms are PR-caused and reachable at the documented next rollout boundary.
Exact fix: Before claiming #2063 complete, add pure inactive structured-field, root-reference, and DAG-step renderers (alongside the request-only form) that all mechanically use the same canonical policy and allowlisted pending representation. Make pinned request_context.output_language handling explicit in each form; ensure structured references do not embed a second full request, and add focused tests for every form, exactly-once request/pending presence, and no duplicate full request. Keep production activation in #2064; if Layer B is intentionally narrowed, amend #2063/#2064 and revise the Closes #2063 claim instead.
3. Focused tests do not independently guard all canonical policy clauses
Location: tests/core/agent/test_request_language_policy.py:142
Severity: minor
Blocking: no
Reachable trigger: A supported maintenance edit can remove or narrow the canonical clauses for explicit/implicit target-language intent, Simplified-versus-Traditional Chinese, the pending override's When it is absent scope, or the general non-language-question rule while retaining the currently asserted hard-authority phrase, narrow pending phrase, city example, and ordinary-context text at lines 142-155. The focused suite can then remain green even though the canonical contract has regressed.
Concrete impact: A required #2063/#2062 language, script, precedence, or scope rule can disappear before #2064 activates the helpers, and CI will not detect that regression. This is a maintenance/acceptance-contract failure, not a current selector failure: the canonical helpers have no production caller in this PR, so the finding is minor and non-blocking.
Why the contract permits it: The new tests exercise the canonical policy text and harness, while existing test_context.py and test_output_language_seam.py cover older policy helpers; no independent assertion currently protects each named canonical clause. #2063 expressly requires focused renderer tests and mutation verification, and #2062 repeats the explicit/implicit intent, script, pending-answer, and hard-authority requirements. The inactive Layer-B scope therefore permits a test-only maintenance trigger but does not make it harmless.
Exact fix: Add independent contract assertions for the full explicit-or-implicit target-language clause, the Simplified-versus-Traditional distinction, the caller-pin/pending precedence boundary including When it is absent, and the general non-language-question rule. Extend mutation verification to remove, narrow, or reorder each clause while retaining the existing language-question evidence, allowlist, no-truncation, and non-activation checks; leave a runtime pinned-versus-pending selector test to #2064, where that selector is activated.
Blocking status & recommended decision
No confirmed blocking issue remains. Prior B1/B2/B3 major blockers are fixed, and every fresh confirmed finding is explicitly non-blocking; therefore Blocking: no — recommended event: APPROVE.
Limitations: No local tests, builds, linters, or formatters were run under review policy; CI preflight reported all checks successful, while the preflight review decision was CHANGES_REQUESTED. The update Simplification Lens was unavailable because of usage_limit_reached, so no new simplification opportunities are reported.
Closes #2063.
Parent: #2062
Layer A: #2068
This change defines the canonical unpinned request-language policy and request-only renderer on top of the Layer A provenance model. It also adds an allowlisted pending-response representation that preserves the full answer, pending question, and message type while excluding tool, connector, identifier, and option internals.
The policy supports explicit language changes in an answer and unambiguous language or script selections made in response to an explicit language question. It also makes clear that a language-like answer to a non-language question is not an override.
The change is behavior-neutral for language selection: existing Root, Auto, ReAct, planner, DAG step, completion, and final-answer consumers are unchanged. Activation is deferred to #2064.
Validation: