Repository navigation
Enable Explain keyword - #62
JGagacheva wants to merge 5 commits into
Conversation
Signed-off-by: Jana Gagacheva <jgagacheva@nvidia.com>
P6 was stale (in-process supports explanation since #34), P3 named only the entry points, P5 was silent on forecasts, D1 named a file that does not exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Jana Gagacheva <jgagacheva@nvidia.com>
Signed-off-by: Jana Gagacheva <jgagacheva@nvidia.com>
Signed-off-by: Jana Gagacheva <jgagacheva@nvidia.com>
a87015b to
862db0a
Compare
| """ | ||
| _require_a_request(request) | ||
| _require_output_names(outputs) | ||
| outputs = _explained_if_asked( |
There was a problem hiding this comment.
P2: Reject explained rankings before gathering their contexts
An EXPLAIN ranking gathers a context for every candidate before rejecting the query. Callers pay for graph or warehouse work and memory that cannot produce an answer. Reject this combination during query resolution or decomposition, before gathering any candidate.
There was a problem hiding this comment.
packages/structured-data-models-client/src/sdm_client/client.py:1331
P2: Question: retain forecast IDs and metadata by step?
Question: should forecasts keep each step’s request ID and response metadata? The existing merge retains only the first ID and drops all metadata, so callers cannot recover a later problematic step’s server correlation ID from the result. If forecasts should be diagnosable like batched predictions, retain these values by step.
| @@ -0,0 +1,99 @@ | |||
| # `EXPLAIN` in PQL: design and tracker | |||
There was a problem hiding this comment.
This planning file shouldn't ship; please delete it like the tracker on #34.
| outputs=['prediction', 'probabilities', 'explanation']) | ||
| ``` | ||
|
|
||
| In the query, `EXPLAIN PREDICT …` means the same as `outputs=['explanation']`; |
There was a problem hiding this comment.
EXPLAIN adds 'explanation' to the default outputs, while outputs=['explanation'] asks for it alone, so please say "adds" rather than "means the same as" here and in the changelog.
EXPLAIN PREDICT … now means the same as passing outputs=['explanation'] to predict(): the answer carries gradient explanations. Passing both is allowed and redundant.
How it works. The parser already records the prefix as a flag. The client now reads it before a query is split into parts (the rebuilt part texts carry no prefix) and appends 'explanation' to the outputs at the entry points — the model's defaults when the caller named nothing, the caller's list when it lacks it, a no-op when present. Everything downstream behaves exactly as if the caller had typed outputs=['explanation']:
Also: the relational preflight message that names the predicted column accepts the prefix, and the # TODO: deprecate on ParsedQuery.explain is gone — EXPLAIN is a supported prefix.
Tests live in the three existing explanation test files, covering both model kinds, the outputs variants, ranking refusal, forecast pass-through and warning, both session directions, in-process, and the preflight message. Docs: one sentence plus the grammar line in both skills, both PQL references, beyond-one-answer.md, prediction-output.md, architecture.md, CHANGELOG. No public API change (tests/golden/public_api.json untouched).