Issue #1232: feat: mark partial Query Planner answers, 503 when nothing was cached

[dereck-symmetry]

Description of Change

Problem. LIFQueryPlannerService.run_query has paths where it deliberately degrades instead of failing. It returns 200 with the cached records, even though they're missing fields the caller asked for, and nothing in the response tells a partial answer from a complete one (#1232). The worst case is a learner who isn't in the cache yet: if orchestrator submission fails, the answer is an empty 200. GraphQL shows that as "no such learner", and the learner data export API turns it into 404 "Query Planner did not find any results" (learner_data_export_endpoints.py:134-138), which is false.

The three paths are not the same kind of event, so they're treated differently:

Path	What it means	Now
No source can serve the missing fields	Permanent (config); a retry won't help	200 + X-LIF-Partial: no_sources_available
Orchestrator submission failed	Transient; a retry may succeed	200 + X-LIF-Partial: orchestrator_submission_failed, or 503 if nothing was cached
Orchestration ran and a source failed (sync /query)	Transient; Dagster swallows it and the run still succeeds	200 + X-LIF-Partial: source_failed, or 503 if nothing was cached
Orchestration ran, every source succeeded, fields still missing	Most likely the learner has no data for them	Unchanged, deliberately unmarked

Marking that last path would flag every learner who has, say, no EmploymentPreferences. The flag would fire all the time and carry no information.

Solution.

• run_query returns a new component-local type, LIFQueryPlannerPartialRecords(records, reason), on the first two paths. The handlers already dispatch on isinstance(result, LIFQueryStatusResponse), so this follows the same pattern. The reason values are the OUTCOME_* constants the query statistics (Issue #341: feat: emit Query Planner query statistics as structured log events #1273) already record for those paths, so the logs and the header use the same words.
• Source failures during orchestration (second commit, c239d6a): run_post_orchestration_results records the failed sources on LIFQueryPlannerJob.failed_source_ids, and the sync handler passes the job id to the second run_query (query_id=), which returns partial records with reason source_failed. The evidence for this is in the comment below.
• In the base, one helper (respond_to_partial_records) is shared by /query and /query_async. It sets the header, or raises 503 for the empty + submission-failed case. HTTP policy stays in the base.
• The new type lives in query_planner_service/datatypes.py, not the shared lif.datatypes brick. Only lif_query_planner_api packages query_planner_service, and the lif_query_planner_api.yml paths: filter covers both changed bricks.

Why a header, not 206 or a wrapper body. Both direct consumers of /query check for exactly 200: openapi_to_graphql/type_factory.py (== 200) and query_planner_client/core.py (!= 200 raises). A 206 would break both. A wrapper body changes response_model=List[LIFRecord] for every adopter. The header is purely additive.

Why the 503. It matches #1264 / #1291: a total failure is reported as an error, not dressed up as an empty result. The export API now reports "Unable to retrieve learner data" (500) instead of the false 404.

Limitations and follow-ups (not filed; for discussion here):

• Nobody reads the header yet. The Advisor and MCP only see GraphQL, so the signal reaches them only once GraphQL relays it, e.g. into the response extensions. That's the follow-up, and it should come after Issue #1264: fix: surface a Query Planner failure as a GraphQL error, not an empty result #1291, which touches the same resolver. That's why this PR says Refs #1232 rather than Closes.
• The export API could read the header too. Deferred.
• source_failed is sync /query only. An /query_async client re-POSTs after polling, which calls run_query(first_run=True) with no link to the job it polled, so there is nothing to attach the failure to. A query_id on that re-POST, or results served from the status endpoint, would be a contract change, so it's left out here.

How reviewers should test. Stop the orchestrator (or point LIF_ORCHESTRATOR_URL at a closed port), then query a learner who isn't in the cache: you should get 503. Query one who is partly cached: 200 with X-LIF-Partial: orchestrator_submission_failed. Ask for a field no configured source serves: 200 with X-LIF-Partial: no_sources_available.

Related Issues

Refs #1232
Refs #1131

Refs, not Closes, for #1232: its third acceptance criterion (downstream consumers updated or explicitly deferred) needs the GraphQL relay above. #1204 was closed today, pointing to #1235 and #1232.

Type of Change

• New feature (non-breaking change which adds functionality)

The header is additive. The 503 replaces an empty 200 in one failure case. It's recorded in CHANGELOG.md but not in MIGRATION.md; happy to add a MIGRATION entry if you count it as breaking.

Project Area(s) Affected

• bases/
• components/
• test/ or e2e/
• API endpoints
• Documentation (docs/, READMEs, ARCHITECTURE.md, CLAUDE.md)

---

Checklist

• commit message follows commit guidelines (see commitlint.config.mjs)
• tests are included (unit and/or integration tests)
• code passes linting checks (uv run ruff check)
• code passes formatting checks (uv run ruff format)
• code passes type checking (uv run ty check)
• pre-commit hooks have been run successfully
• API changes: base (Python code) documentation in docs/
  and project README updated (both brick READMEs updated)

Testing

• Automated tests added/updated

• Component: the no-sources and submission-failed tests now assert the partial type and its reason. A new test pins the post-orchestration path as a plain, unmarked list.

• Base: header on a partial answer, run through TestClient so the header is shown to reach the wire; no header on a complete answer; 503 for empty + submission-failed on both endpoints; an empty no-sources answer stays a marked 200; /query_async is marked too.

• Source failures: failed sources are recorded on the job; a re-run after a job with a failed source is marked source_failed; a re-run after a clean job stays unmarked; the sync handler passes query_id through and marks the answer; empty + source_failed is 503.

• Guard check: with only the run_query change reverted, exactly the two path tests fail. The base tests failed before the handler change. For the second commit, reverting the component alone fails exactly its 3 new component tests.

• Query Planner suites: 74 passed. pre-commit run --files on all 8 files (ruff, format, cspell, ty, full pytest) passed.

Additional Notes

Merge order with #1301 / #1302. Those two touch the same files, and a trial merge conflicts in query_planner_service/core.py (the run_query signature: #1301 adds client, this adds a return type), datatypes.py (an import line; a new class next to #1302's org_key field), and tests added at the same place in both test files. Every hunk is keep-both; nothing overlaps in meaning. Whichever lands second gets main merged in (not rebased). Trial merges against #1291 and #1299 are clean.

🤖 Generated with Claude Code

[dereck-symmetry]

Evidence for the second commit: a failed source still produces a clean run

When I first opened this PR, the case "orchestration ran and fields are still missing" was left unmarked on purpose, on the reasoning that it most likely meant the learner had no data for those fields. That depended on something I hadn't checked: whether a source failing inside the Dagster run would end the run as a failure (and so already surface as a 500), or whether it would come through looking like success. So I measured it.

How it was measured

An in-process probe runs the real lif_dynamic_pipeline_job from orchestrators/dagster/lif-orchestrator/src/lif_orchestrator/defs/lif_job.py, which is the same file both code-location Dockerfiles copy. Only the adapters are faked: org2 returns a fragment, and org3 raises on every attempt. The Query Planner is a real HTTP stub that captures what the job POSTs to /orchestration/results. That captured payload is then fed to the real LIFQueryPlannerService.run_post_orchestration_results, followed by the second run_query call the sync /query handler makes. A control run with both sources succeeding checks that the probe can tell the two cases apart.

What it showed (before the fix)

	Control: both succeed	Probe: org3 fails every attempt
Dagster run success	True	True
STEP_FAILURE events	none	none (2 retries, then the error is swallowed)
org3 adapter attempts	1	3
org3 in the payload sent to the Query Planner	1 fragment, error=None	0 fragments, error='Pipeline did not run or failed.'
Query Planner job status afterwards	COMPLETED	COMPLETED
Second run_query returns	plain list	plain list, identical to the control

The chain behind it:

1. run_lif_adapter (lif_job.py:119-127) returns data=None on its last retry instead of raising.
2. The step that gathers results (:235-246) fills in an error placeholder for the missing part.
3. The run succeeds and POSTs the results.
4. run_post_orchestration_results logged the error, discarded it, and marked the job COMPLETED regardless.

So a source that was down produced exactly the answer a learner with no data would: a 200 with no signal. This is the gap #1232 describes, arriving by the one path this PR had left unmarked.

The information was never really lost. The query_completed statistics event already records "paths_not_fulfilled": ["Person.credentialAward"] and org3's error for that run. It just never reached the response.

The fix (c239d6a)

• run_post_orchestration_results records failed sources on the job, in LIFQueryPlannerJob.failed_source_ids.
• The sync handler passes the job id to the second run_query (query_id=). If that job had a failed source and fields are still missing, it returns partial records with reason source_failed. The response then carries X-LIF-Partial: source_failed, or is a 503 if nothing at all was returned, matching the empty + failed-submission case.
• A re-run after a job where every source succeeded stays unmarked, as before.

Re-running the same probe against the fix: the control still returns a plain list, and the failing case now returns partial records with reason=source_failed.

Limits worth knowing

• The adapters were fakes. A real adapter's failure lands in the same except Exception in run_lif_adapter, which is what matters here, but no real timeout inside an adapter was exercised.
• In process, not deployed. The job definition does set executor_def=dg.in_process_executor, but the deployed run launcher and the real HTTP path to the Query Planner weren't tested. The Query Cache was mocked.
• Sync /query only. A /query_async client re-POSTs after polling, which calls run_query(first_run=True) with no link back to the job it polled, so there's nothing to attach the failure to. Fixing that would change the async contract, so I've left it out of this PR.

An end-to-end check on local compose (stop one org's GraphQL, query a learner who needs that org's fields) would close the first two limits. Happy to run it if you'd like that before this merges.

[bjagg]

Approving. The table separating permanent from transient failures is the idea that makes this right. Deliberately not marking "every source succeeded, fields still missing" is what keeps the header from firing all the time and carrying no information.

I verified the three claims the design depends on. Each one would break something if wrong.

1. "Both direct consumers of /query check for exactly 200." Confirmed: type_factory.py uses == 200 and query_planner_client/core.py:52 uses != 200. I also searched wider (adapters, orchestrator, Advisor, semantic search) for a third caller that might read a 503 differently. The only other mention is lif_job.py:213's send_results_to_query_planner, which is Dagster posting results back to the callback endpoint, not calling /query. So there are exactly two consumers, and the header-not-206 reasoning holds.

2. Nothing iterates the new return type. run_query has exactly three call sites, all in query_planner_restapi/core.py (:178, :200, :237), and all three dispatch on LIFQueryPlannerPartialRecords before returning. No caller can be surprised by a wrapper where it expected a list.

3. The export API stops reporting a false 404. Confirmed: a 503 makes fetch_query_from_query_planner raise QueryPlannerException, which learner_data_export_endpoints.py:128-130 turns into 500 "Unable to retrieve learner data from Query Planner". It no longer falls through to :136-138's "did not find any results". That false 404 was the worst symptom in #1232, and it's gone.

The 503 doesn't reopen #1291's leak. Its detail carries partial.reason, which is always one of the OUTCOME_* constants and never backend text. Reusing the statistics vocabulary for the header, so logs and response use the same words, is a nice touch.

Mutation-checked, all four killed:

mutation	result
never 503 (empty + transient failure back to an empty 200)	3 failed
also 503 on no_sources_available (permanent, must stay 200)	1 failed
stop setting X-LIF-Partial	4 failed
stop recording failed_source_ids during orchestration	2 failed

The second is the design line in your table, and test_core.py:489, an empty no_sources_available partial expecting 200 plus the header, is what pins it. That's the test I'd most want to exist, since "helpfully" adding the permanent case to the 503 set would tell callers to retry a config problem. 74 pass in the affected components and 873 across the full suite.

On your MIGRATION.md question: yes, add it. The header is additive, but the 503 isn't. A caller that treats an empty 200 as "this learner has no data" now gets an error in one case. #1291 made the same kind of change (a failure reported as an error rather than an empty result) and recorded it in MIGRATION.md, so doing the same here keeps the two consistent.

Merge order. I agree the conflicts with #1301 and #1302 are keep-both. Your GraphQL relay follow-up should come after #1291, which is now approved.

One forward-looking nit, not blocking. Every reader of X-LIF-Partial is server-to-server today, so CORS doesn't apply. If a browser client ever reads it directly, it will also need Access-Control-Expose-Headers, or the header will be invisible to the page.