fix(agent): extend grounding rule to quantitative data and add it to all final-answer prompts

[codeacme17]

Summary

Implements proposal A of #1235. Scope is limited to prompt-level grounding constraints; proposals B (evidence-preserving compaction) and C (provenance tracking / data-source gating) are deliberately out of scope and will follow separately.

The issue reports an agent producing a full KPI deck with no data source connected, and only admitting the numbers were unsourced after being challenged. Two prompt-level gaps made that outcome cheap:

• The single anti-fabrication clause we ship enumerated entities, incidents, dates, sources, and causal explanations — not quantitative content. A fabricated KPI table did not violate its letter.
• The clause lived only in the ReAct has_tools branch. Three other paths that emit a final user-facing answer carried no grounding constraint at all.

Changes

Factor the clause into a shared xagent.core.agent.grounding module (mirroring the existing language.py prompt-fragment pattern) and close both gaps:

• Cover quantitative data — metrics, figures, statistics, percentages, table rows, and time series are now named explicitly alongside the original enumeration.
• Require up-front disclosure — when an answer contains figures that no tool result or provided context supports, the model must state before presenting them that they are illustrative placeholders, and must never present invented numbers as real data.
• Wire it into every final-answer path:

  Path	File	Status before
  ReAct, tools available	pattern/react/react.py	had the old clause (now shared + extended)
  ReAct, force_final_answer	pattern/react/react.py	no constraint — this is the path taken when the iteration budget runs out, i.e. on long sessions
  DAG completion assessment	pattern/dag/dag.py	no constraint — the path report-shaped think tasks route to
  Auto routing, direct final_answer	pattern/auto/auto.py	no constraint — the router can emit a user-facing answer directly

grounding_rule(can_call_tools=False) is used on paths that are restricted to a single forced control tool, where the original "use an appropriate tool to verify" escape hatch would be unfollowable; those get "say so instead of filling the gap with invented values". The Auto router additionally gets told to route to react when the answer would need unsupported specifics, since routing — not answering — is its lever.

Testing

• New tests/core/agent/test_grounding.py covering both variants of the rule.
• New assertions in test_react.py, test_dag.py, and test_auto.py exercising the real prompt-building functions (_messages_for_llm with and without force_final_answer, _completion_assessment_messages, and the Auto decision prompt) rather than synthetic strings.
• tests/core/agent/ passes (205 tests across the four affected files re-verified after rebase); ruff check, ruff format --check, and mypy clean on the touched files.

Notes for reviewers

• This is a prompt-only change; no behavioural code paths, no compaction or context-model changes.
• It reduces how often fabricated figures are emitted and makes disclosure the instructed default, but it is a mitigation, not a guarantee — the structural fix is proposal B (compaction currently discards the tool observations an answer is supposed to be grounded in) and proposal C (provenance).
• The open question in bug(agent): fabricated data survives compaction — long sessions lose grounding and report prompts have no anti-fabrication constraint #1235 — whether report-shaped output should be blocked without a connected data source or allowed but hard-labelled — is untouched here and still needs a product decision.

Refs #1235

[gemini-code-assist]

Code Review

This pull request introduces a shared anti-fabrication prompt rule (grounding_rule) in a new grounding.py module to prevent LLMs from generating fabricated data in user-facing answers. This rule is integrated across various agent patterns, including Auto, DAG, and ReAct, with corresponding unit tests added to verify its presence and behavior under different tool-use configurations. There are no review comments, and I have no additional feedback to provide.

[rogercloud]

Summary

This PR closes prompt-level gaps in the codebase's single anti-fabrication ("grounding") clause. Previously it covered only entities, incidents, dates, sources, and causes, and existed only in the ReAct has_tools branch. This change factors it into a shared module src/xagent/core/agent/grounding.py (grounding_rule(can_call_tools: bool)), extends the wording to explicitly cover quantitative data (metrics, figures, statistics, percentages, table rows, time series) plus a new up-front disclosure requirement for unsupported figures, and wires the shared rule into all four final-answer-producing paths: ReAct has_tools, ReAct force_final_answer, DAG completion assessment, and Auto's direct final_answer routing. The diff is prompt-only (8 files, +144/-6); no runtime or control-flow code is touched.

Approach verdict: acceptable with reservations

• Shared-module design is the right call. grounding.py mirrors the existing language.py / final_answer_language_rule convention, and avoids four inline copies of the same clause. Confirmed there are no leftover duplicates of the old clause text anywhere in src/xagent/.
• All three identified gaps are genuinely closed. ReAct force_final_answer, DAG completion assessment, and Auto's direct final_answer routing are each the actual sole path producing user-facing text for their pattern (flash/single_call reuses ReActPattern). The can_call_tools=False variant is semantically correct at all three new sites, since each is a forced/no-tool-choice context where "use a tool to verify" would be an unfollowable instruction.
• Scope/framing note on the fix(agent) title: per issue #1235's own analysis, the primary root cause is context compaction discarding tool-observation evidence on long sessions, explicitly deferred here to "proposal B". On the exact failure path reported (long session, compaction fires, evidence already gone), this clause cannot fire correctly, because the model has no way to know its own numbers are unsourced by the time it writes the answer. The PR's "Notes for reviewers" partly acknowledges this ("a mitigation, not a guarantee"); worth reflecting in the title/changelog framing so the issue is not read as closed end-to-end.
• Testability ceiling: every new and extended test asserts prompt-string substring presence. That verifies wiring, not model behavior, and no eval harness exists in this repo to check the latter. Acceptable for a prompt change, but the effectiveness of the new wording remains unverified.

Findings

All findings are minor or nit level; nothing blocking. Three are left as inline comments (test_auto.py, grounding.py, test_react.py); the fourth doesn't land on a diff line so it's noted here:

• Nit — src/xagent/core/agent/pattern/react/react.py:804-805: the else: return messages branch of _messages_for_llm (unchanged by this PR) carries no grounding instruction, unlike its sibling branches. Verified it is reachable only when tool_choice == "none", which defaults to "required" (line 187) and is never overridden at any of the five production ReActPattern(...) construction sites (auto.py:425, execution_adapter.py:299/312/321, dag.py:864) — only test code exercises it (test_react.py:2953/3259/4553). Currently a dead consistency gap; if tool_choice="none" is ever wired into a real config path, that path would silently produce final answers with zero anti-fabrication constraint. Low priority, not blocking, and out of scope for this PR to fix since the branch predates it.

Testing

Ran .venv/bin/python -m pytest tests/core/agent/test_grounding.py tests/core/agent/test_react.py tests/core/agent/test_dag.py tests/core/agent/test_auto.py -q — 205 tests passed, 0 failures.

[rogercloud]

Summary

An agent session produced a complete KPI report with invented numbers and never disclosed they were unsourced until challenged (#1235). The root cause was twofold: the anti-fabrication clause the codebase shipped covered entities, incidents, dates, sources, and causal explanations but said nothing about quantitative content, and it existed in only one of the four prompts that can emit a final user-facing answer. This PR extracts that clause into src/xagent/core/agent/grounding.py, extends it to metrics/figures/statistics/percentages/table rows/time series plus an up-front disclosure requirement for illustrative figures, and wires it into all four answer-emitting paths (ReAct with tools, ReAct forced final answer, DAG completion assessment, Auto router's direct final_answer). Prompt-only, 144 insertions across 8 files, no behavioral machinery added.

Prior review activity: the only prior review is an automated bot (gemini-code-assist) that reported no additional feedback. There is nothing substantive to reconcile, so this is treated as the first substantive review.

Design verdict: sound

• The pre-PR gap is real. Verified against the base commit: the force_final_answer branch in react.py, the DAG completion-assessment system prompt, and the Auto router's decision prompt each carried zero grounding text. Only the ReAct-with-tools branch had the clause.
• Coverage claim holds. single_call mode maps onto ReActPattern (via execution_adapter.py), and DAG steps also execute through ReActPattern, so the four wired sites genuinely are every path that emits a user-facing answer today.
• Right shape, right size. grounding.py mirrors the established language.py convention — a module of pure string-returning prompt-fragment functions, no registry, no subclassing, no new abstraction. One function with a single can_call_tools axis is correctly scoped: each call site already layers its own remedy semantics (outcome=partial, status=incomplete, action=react) in the surrounding prompt text, so the shared fragment only needs to vary on the one thing that changes its own phrasing — whether a tool call is even possible. The flag is exercised at both True and False across real call sites, so it is not speculative flexibility.
• Correctly scoped to proposal A. Proposals B (evidence-preserving compaction) and C (provenance/data-source gating) stay out, tracked separately. This is a mitigation, not a structural fix, and that is the right call for a P0.

No criticals, no majors. Everything below is minor or informational.

Minor findings

1. src/xagent/core/agent/pattern/dag/dag.py:1405 — disclosure clause vs. status=incomplete. The clause is injected immediately after "if data is missing, choose status=incomplete, leave answer empty…". The disclosure sentence is unconditional prose and is not scoped away from this path, so a model can read it as licensing a labeled-placeholder answer with status=completed rather than reporting the gap via status=incomplete. Nothing in the DAG tool schema mechanically forces either value. Note this is the same "blocked vs. hard-labeled" product question already raised in the PR body — and even read at face value the clause would have prevented the original incident, since disclosure-before-presenting is exactly what was missing. Worth a clarifying phrase that disclosure does not override choosing incomplete when the data source is entirely absent.

2. src/xagent/core/agent/pattern/dag/dag.py:1405 — missing scoping phrase. auto.py:1310 scopes the rule explicitly ("When writing that answer field: …"); dag.py appends it as a flat continuation of a paragraph that just instructed the model to leave answer empty. This is not a hard contradiction (nothing enforces emptiness, and "say so" maps naturally onto the missing_work/replan_instruction fields), but the asymmetry with auto.py leaves avoidable ambiguity. Prefixing with "When writing the answer field: …" would make the two sites read consistently.

3. src/xagent/core/agent/grounding.py:15 — docstring undercounts the can_call_tools=False callers. The parenthetical lists "forced final answers, DAG completion assessment" but omits the third: the Auto routing decision prompt. Exactly three sites pass False — react.py:762, dag.py:1405, auto.py:1310.

4. tests/core/agent/test_grounding.py — substring-only assertions cannot catch malformed concatenation. Every assertion is membership ("table rows" in rule). If the trailing space in insufficient_context_rule (grounding.py:26/:30) were dropped, yielding "…verify.If the answer…", no current test would fail. Asserting a sentence-boundary substring such as "verify. If the answer" would close this.

5. tests/core/agent/test_auto.py:1094 — new test is weaker than its DAG/ReAct siblings. It asserts only "quantitative data", "illustrative placeholders", "choose react instead". The analogous tests in test_dag.py:386-388 and test_react.py:1337-1343 also assert "invented values" / the negative "use an appropriate tool" not in …. Without the negative assertion, this call site silently flipping to can_call_tools=True would not be caught.

Informational (no change required)

6. src/xagent/core/agent/pattern/react/react.py:804 — the has_tools=False branch returns the messages untouched, so it carries no anti-fabrication text at all. This is effectively unreachable today: no ReActPattern( construction site (execution_adapter.py, dag.py, auto.py) passes tool_choice="none". A one-line comment marking the gap would keep it from being silently reintroduced if that ever becomes reachable.

7. src/xagent/core/agent/grounding.py:3-4 states an unenforced invariant — "Every prompt that produces a final user-facing answer must carry this rule". There is no cross-cutting test walking all prompt builders to verify it; coverage is per-path unit tests only, and per (6) one branch genuinely lacks it. Consider phrasing it as a design goal rather than a guarantee.

8. No in-code pointer that this is mitigation-only. That proposals B and C from #1235 remain open lives solely in the PR description. A one-line comment in grounding.py referencing them would stop a future reader treating this clause as the complete fix.

Simplification opportunities

• src/xagent/core/agent/pattern/auto/auto.py:1311 — shrink: redundant with the existing "choose react … do not choose final_answer" steering at auto.py:1326 and auto.py:1356. The new sentence has marginally broader scope than the recency-scoped existing text, but both funnel to the same "prefer react over final_answer when the answer would be ungrounded" instruction, now stated twice in different frames. Consolidate into one instruction, or cross-reference rather than restate.

net: -3 lines possible

Verification

• 205 tests pass across tests/core/agent/test_grounding.py, tests/core/agent/test_react.py, tests/core/agent/test_dag.py, tests/core/agent/test_auto.py.
• ruff check, ruff format --check, and mypy clean on the touched files.
• Correctness spot-checks: every can_call_tools value is factually accurate for its call site (all three False sites genuinely cannot invoke work tools); all four prompt concatenations render grammatically (no double spaces, no missing sentence boundaries); "react" is a valid action value in the Auto decision schema. Tests exercise the real production prompt builders (_messages_for_llm, _completion_assessment_messages, the live decision prompt), not synthetic reimplementations.

No user-facing documentation is needed — the change adds no API, config, or CLI surface.

[codeacme17]

Thanks for both reviews — all nine findings are addressed in d444452. Replies are on each thread; the three that changed shape from what was suggested:

Disclosure wording. The fabricate-and-label reading is real, but gating the disclosure duty on the user having asked for a template would have removed it in #1235 itself, where the user asked for a real KPI report — and it inverts proposal A's trigger, which is a property of the deliverable, not of the request's phrasing. So I withdrew the permission rather than narrowing the duty: unsupported figures may only be produced when a template/mockup/example was explicitly requested, while labeling stays unconditional. New test pins that so it can't be quietly re-gated.

DAG status=incomplete. Confirmed, plus an incentive gradient neither review cited: incomplete consumes a replan and hard-fails the execution at the limit (dag.py:1235-1245), so completed + placeholder is the cheaper exit. I did not force incomplete — that decides #1235's open block-vs-label question unilaterally, via the harshest mechanism available. Instead unsourced data must be named in reason, which is required and persisted, making the choice observable so product can decide from telemetry. Say the word if you'd rather force it now.

The DAG scoping prefix also surfaced a third gap: the assessor usually passes through candidate_output rather than authoring figures, so the rule now explicitly covers carried-over step content.

Auto overlap. The cited L1324-1327 is a different rule (explicit tool requests); the real duplicate is L1344-1350, and only its routing tail. Rather than delete it I retargeted it at existing_context_sufficient=false so it trips the deterministic fallback at auto.py:684-712 instead of relying on the model's routing choice. Net ~1 line instead of 3, but the guarantee moves from prompt adherence to code.

On the forced-final coverage gap: your experiment reproduced, and my first fix didn't actually close it — asserting "quantitative data" still passed with force_final_answer=False injected, since after this PR both branches carry the rule. It now asserts the can_call_tools=False variant, which is the only discriminating signal, and I re-ran your injection to confirm it fails.

Not changed: the has_tools=False branch at react.py:804 still has no grounding text. It's unreachable in production and predates this PR, so per your own note I left it and added a comment marking the gap at the site.

tests/core/agent/ passes; pre-commit run --files clean on all eight touched files.