fix(consult): let the declared domain decide the route, not the question text
A staging consultation asked one question about two domains, career and
wealth. The model planned both, and all four attempts failed identically with
calculation_failed. The server resolves the workflow route from the question
text — an explicit-timing check, then keyword domain_tokens, falling back to
the themes argument only when the text yields nothing — while the frontend
declares strict_workflow_route from the domain it chose. The plan contract then
requires the text-derived route to equal the declared one, and each
RouteContract allows exactly one, so the mismatch became
ConsultationPlanContractError, BadRequest, HTTP 400, workflow_bad_request.
Here "事业" is in the career token list and "财运" is not in the wealth one, so
both calls resolved to career and the wealth call was rejected every time.
Widening the token list would only move the contradiction to the next
phrasing. The frontend sends one Python call per domain with the same question
text, so text routing can agree with at most one domain of a multi-domain plan
and every other domain is refused by construction. Now that 1955ba8c caps the
plan at three domains and merges the per-domain packets into one top-level
contract, two- and three-domain plans are expected to work end to end and this
is what stops them.
Make the server-issued declaration authoritative. declared_workflow_route()
returns the route a complete, version-supported, allowlisted plan declares, and
resolve_route() honours it instead of reading the text; a caller that sends no
plan metadata keeps the text heuristics verbatim, so the MCP strict_workflow
tool and the research callers behave exactly as before. The route packet
records which rule decided, because routing now has two legitimate sources.
This is not a way to silence the 400: the whole packet comes from the declared
domain's RouteDefinition, so the sync steps, the consumer_context required
layers, the evidence packet and the frontend's themes[primary_theme] lookup all
land on the domain that was declared. A call declaring wealth can no longer
execute career and label career evidence as wealth. The contract stays
fail-closed — a route off the allowlist is refused before execution, and
themes, layers, boundary, domains, categories, depth, horizon and precision are
still checked one by one. The surviving resolved_routes check changes meaning
rather than going away: it now asserts the workflow executed what was declared.
The timing prefix "应期与阶段问题:" existed only to inject 应期 so the text
router would agree with the declared route for one domain out of ten. With the
declaration authoritative it fixes nothing and still rewrites the question the
model's answer derives from, so it goes. It influences no other server
behaviour: it matches none of the consumer-context domain regexes, and the
timing route already sets precise_timing_requested.
test_consultation_workflow_domains.py deliberately sent no plan metadata, which
is why this was never caught — its per-domain question happened to route to its
own domain. It now sends the real plan for all ten canonical domains behind one
question whose text routes to career; nine of them fail without this change.
Refs BUG-259.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -121,6 +121,41 @@ class ConsultationPlanContractError(ValueError):
|
||||
"""Raised when a versioned product plan does not match the server allowlist."""
|
||||
|
||||
|
||||
def plan_route_contract(route: Any) -> RouteContract | None:
|
||||
"""Expose the server-owned contract for a route; callers never supply their own."""
|
||||
|
||||
return _ROUTE_CONTRACTS.get(route) if isinstance(route, str) else None
|
||||
|
||||
|
||||
def _declared_route_and_contract(body: dict[str, Any]) -> tuple[str, RouteContract] | None:
|
||||
present_plan_keys = _PLAN_METADATA_KEYS.intersection(body)
|
||||
if not present_plan_keys:
|
||||
return None
|
||||
missing_plan_keys = _PLAN_METADATA_KEYS.difference(body)
|
||||
if missing_plan_keys:
|
||||
missing = ", ".join(sorted(missing_plan_keys))
|
||||
raise ConsultationPlanContractError(f"incomplete consultation plan metadata: {missing}")
|
||||
if body.get("plan_version") != PLAN_VERSION:
|
||||
raise ConsultationPlanContractError("unsupported consultation plan version")
|
||||
strict_route = body.get("strict_workflow_route")
|
||||
contract = plan_route_contract(strict_route)
|
||||
if contract is None:
|
||||
raise ConsultationPlanContractError("unsupported consultation workflow route")
|
||||
return str(strict_route), contract
|
||||
|
||||
|
||||
def declared_workflow_route(body: dict[str, Any]) -> str | None:
|
||||
"""Return the allowlisted route a versioned plan declares, before any text heuristic runs.
|
||||
|
||||
The route the product declares is server-issued, so it decides execution instead of
|
||||
competing with keyword routing. ``None`` keeps question-text routing authoritative for
|
||||
legacy research/MCP callers that send no plan metadata.
|
||||
"""
|
||||
|
||||
resolved = _declared_route_and_contract(body)
|
||||
return None if resolved is None else resolved[0]
|
||||
|
||||
|
||||
def _require_list(body: dict[str, Any], key: str) -> tuple[str, ...]:
|
||||
value = body.get(key)
|
||||
if not isinstance(value, list) or not value or any(not isinstance(item, str) or not item for item in value):
|
||||
@@ -137,23 +172,15 @@ def validate_consultation_plan_contract(
|
||||
"""Validate the optional versioned plan without accepting free-form workflow control.
|
||||
|
||||
Older research/MCP callers may omit ``plan_version``. Product runtime calls include it
|
||||
and must match every server-owned route field exactly.
|
||||
and must match every server-owned route field exactly. ``route_packet`` must already be
|
||||
resolved from :func:`declared_workflow_route`, so the route check below confirms the
|
||||
workflow executed the declared route rather than arbitrating between two routers.
|
||||
"""
|
||||
|
||||
present_plan_keys = _PLAN_METADATA_KEYS.intersection(body)
|
||||
if not present_plan_keys:
|
||||
resolved = _declared_route_and_contract(body)
|
||||
if resolved is None:
|
||||
return None
|
||||
missing_plan_keys = _PLAN_METADATA_KEYS.difference(body)
|
||||
if missing_plan_keys:
|
||||
missing = ", ".join(sorted(missing_plan_keys))
|
||||
raise ConsultationPlanContractError(f"incomplete consultation plan metadata: {missing}")
|
||||
if body.get("plan_version") != PLAN_VERSION:
|
||||
raise ConsultationPlanContractError("unsupported consultation plan version")
|
||||
|
||||
strict_route = body.get("strict_workflow_route")
|
||||
contract = _ROUTE_CONTRACTS.get(strict_route)
|
||||
if contract is None:
|
||||
raise ConsultationPlanContractError("unsupported consultation workflow route")
|
||||
strict_route, contract = resolved
|
||||
if tuple(themes) != contract.themes:
|
||||
raise ConsultationPlanContractError("consultation plan theme mismatch")
|
||||
|
||||
|
||||
@@ -851,19 +851,29 @@ def execute_consultation_workflow(
|
||||
external_parity_gate = validate_manifest(
|
||||
Path(__file__).resolve().parents[1] / 'references/oracle/three_engine_parity_replay_manifest.json'
|
||||
)
|
||||
route_packet = _UNIFIED_CONSULTATION_ORCHESTRATOR.resolve_route(question, themes)
|
||||
try:
|
||||
from scripts.consultation_plan_contract import (
|
||||
ConsultationPlanContractError,
|
||||
apply_plan_precision_boundary,
|
||||
declared_workflow_route,
|
||||
validate_consultation_plan_contract,
|
||||
)
|
||||
except ModuleNotFoundError: # pragma: no cover - direct script execution
|
||||
from consultation_plan_contract import (
|
||||
ConsultationPlanContractError,
|
||||
apply_plan_precision_boundary,
|
||||
declared_workflow_route,
|
||||
validate_consultation_plan_contract,
|
||||
)
|
||||
try:
|
||||
# A versioned plan declares one domain per call, so the declared route decides
|
||||
# execution and question-text routing stays for callers that declare nothing.
|
||||
declared_route = declared_workflow_route(body)
|
||||
except ConsultationPlanContractError as exc:
|
||||
raise BadRequest(str(exc)) from exc
|
||||
route_packet = _UNIFIED_CONSULTATION_ORCHESTRATOR.resolve_route(
|
||||
question, themes, declared_route=declared_route,
|
||||
)
|
||||
try:
|
||||
consultation_plan_contract = validate_consultation_plan_contract(
|
||||
body, themes=themes, route_packet=route_packet,
|
||||
|
||||
@@ -249,7 +249,25 @@ class UnifiedConsultationOrchestrator:
|
||||
def normalize_themes(self, raw: Any) -> list[str]:
|
||||
return normalize_consultation_themes(raw)
|
||||
|
||||
def resolve_route(self, question: str, themes: list[str] | None = None) -> dict[str, Any]:
|
||||
def resolve_route(
|
||||
self,
|
||||
question: str,
|
||||
themes: list[str] | None = None,
|
||||
*,
|
||||
declared_route: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Resolve the workflow route, preferring an explicitly declared one over the question text.
|
||||
|
||||
``declared_route`` comes from server-issued plan metadata and is already allowlisted, so it
|
||||
decides execution: one question asked for several domains would otherwise let keyword
|
||||
matching answer for at most one of them. Callers that declare nothing keep text routing.
|
||||
"""
|
||||
if declared_route is not None:
|
||||
route = self._ROUTE_DEFINITIONS.get(declared_route)
|
||||
if route is None:
|
||||
raise ValueError(f"unknown declared consultation route: {declared_route}")
|
||||
return self._route_packet(route, source="declared_plan")
|
||||
|
||||
text = (question or "").lower()
|
||||
normalized_themes = self.normalize_themes(themes)
|
||||
explicit_timing_tokens = ("when", "timing", "何时", "什么时候", "应期", "几月", "哪月", "哪天", "日期")
|
||||
@@ -301,11 +319,16 @@ class UnifiedConsultationOrchestrator:
|
||||
else:
|
||||
route = self._ROUTE_DEFINITIONS["general"]
|
||||
|
||||
return self._route_packet(route, source="question_text")
|
||||
|
||||
@staticmethod
|
||||
def _route_packet(route: RouteDefinition, *, source: str) -> dict[str, Any]:
|
||||
return {
|
||||
"question_type": route.question_type,
|
||||
"primary_theme": route.primary_theme,
|
||||
"focus_techniques": list(route.focus_techniques),
|
||||
"display_label": route.display_label,
|
||||
"route_source": source,
|
||||
}
|
||||
|
||||
def route_profile(self, question: str, themes: list[str] | None = None) -> dict[str, Any]:
|
||||
|
||||
Reference in New Issue
Block a user