fix(init): surface generation-phase embed failures in the run's degraded list (#1369) - #2108
Conversation
…ded list (repowise-dev#1369) The init header probes build_embedder and warns on degradation, but the generation phase rebuilds the embedder and a mid-run embed failure (e.g. a local Ollama that goes down) only wrote a structlog warning — and the CLI pins structlog to ERROR unless -v, so the run finished looking clean with semantic search silently off and state.json carrying no degraded entry. Threads an on_warning callback from run_generation down through PageGenerator.generate_all into the orchestrator's embed-batch failure handler, routing text through progress.on_message('warning', ...) so the Rich callback records it; run_repo_generation now merges the generation callback's warnings into the caller's list, so init's persisted 'degraded' (state.json) and the completion panel see it. Wiring: pipeline/phases/generation.py (closure + generate_all kwarg), page_generator/core.py + orchestrate.py (param threading + handler call), init_cmd/generation.py (warnings param + merge), init_cmd/command.py (warnings=run_warnings). Test: stub generator + progress recorder pin the seam; sabotage-verified. 207 gen/pipeline/init tests pass.
|
🔍 3 things to check
✅ Health of changed files: 2.6 → 2.8 (+0.2) 👀 Suggested reviewers @RaghavChamadiya 📊 See the full report for this PR |
RaghavChamadiya
left a comment
There was a problem hiding this comment.
Thanks @sloemo01. I traced the whole chain against the tree before approving, because "the warning now reaches the user" is the kind of claim that is easy to make and easy to get wrong by one hop:
on_warning fires in the embed-batch handler, run_generation routes it to progress.on_message("warning", ...), and on_message appends to self.warnings at ui/progress.py:216. run_repo_generation then extends the caller's list with gen_callback.warnings, and because run_warnings at init_cmd/command.py:1452 is a real list handed down by reference, the extend lands in the same object that becomes base_state["degraded"] at :1715-1716. So this does reach state.json and not just the terminal, which is the half that matters for an agent reading the run afterwards.
A bonus you may not have counted on: on_message runs redact_secrets over the text, and the message interpolates the exception, which for a provider error can carry a URL or a key. Routing through the callback rather than printing directly is what buys that.
Merging. Two non-blocking notes.
The new test pins the seam rather than the behaviour: it stubs PageGenerator, so it proves run_generation threads on_warning down and that invoking it records a warning, but nothing exercises the real except around the embed batch. That is a fair trade for a test that stays fast, and the docstring is honest about it, so I am not asking for more. Worth knowing if this regresses: the seam test will still pass if someone deletes the self.on_warning(...) call in orchestrate.py.
Second, #2106 edits the same _index_with_resume call site a few lines from your warnings=run_warnings argument. Whichever lands second will want a trivial rebase; there is no real disagreement between them.
What
Fixes #1369 — residual from the #852 config-warning fix.
Root cause
The init header probes
build_embedderand warns on degradation, but the generation phase rebuilds the embedder and a mid-run embed failure (e.g. a local Ollama that goes down) only wrote a structlog warning — and the CLI pins structlog to ERROR unless-v, so the run finished looking clean with semantic search silently off, and state.json carried nodegradedentry.Changes
on_warningcallback fromrun_generationdown throughPageGenerator.generate_allinto the orchestrator's embed-batch failure handler, routing the text throughprogress.on_message('warning', ...)so the Rich callback records it.run_repo_generationmerges the generation callback's warnings into the caller's list, so init's persisteddegraded(state.json) and the completion panel see it.Tests
Note:
test_plugin_contentis red on main itself (v0.47.0 release bug, unrelated to this PR).