Skip to content

fix(nugget-extractor): degraded refusal attributes the real stalled dependency (follow-up to #1106) - #1127

Open
mdheller wants to merge 1 commit into
mainfrom
fix/nugget-degraded-attribution-followup-1106
Open

fix(nugget-extractor): degraded refusal attributes the real stalled dependency (follow-up to #1106)#1127
mdheller wants to merge 1 commit into
mainfrom
fix/nugget-degraded-attribution-followup-1106

Conversation

@mdheller

Copy link
Copy Markdown
Member

Follow-up to #1106, which merged with three of its four sub-tier findings resolved. This lands the last one: the degraded (queue-full) back-pressure refusal misreported which dependency was stalled.

The defect (now in main)

A back-pressure refusal contacts nothing (attempted stays False), yet _submit hardcoded hellgraph_ok=False / gateway_ok=False and a hellgraph-centric reason. The pending queue fills just as readily when the compute-gateway receipt step is the wedged side (graph writes fine), so an operator chasing a gateway outage was pointed at hellgraph. The server then overwrote STATE["hellgraph_ok"]/["gateway_ok"] from a call that observed no dependency at all.

The fix

  • emitter: the emitter remembers the last drain's observed health and the degraded refusal reports that — naming hellgraph and/or the gateway receipt step accurately — instead of guessing. attempted is explicitly False.
  • server: the degraded branch no longer overwrites hellgraph_ok/gateway_ok (the same if result.attempted rule the success path already applies); it records only the back-pressure error, and the 503 body carries the last-observed health.

Tests (red → green proven locally)

  • A gateway-wedged case asserts the refusal names the gateway (not hellgraph) and reports gateway_ok False / hellgraph_ok True; the existing hellgraph-wedged case is updated to the accurate attribution.
  • A _parse_max_pending guard test pins non-int → default and negative → 0.
  • A concurrent seal-during-/healthz test reproduces the dictionary changed size during iteration 500 on bare _CHAINS.values() iteration and passes on snapshot_all() — locking in the race fix that landed in fix(gateway): three sub-tier findings — seal race, unbounded pending, silent unsigned #1106.

pytest tests/test_sub_tier_hardening.py (8) and tests/test_pending_cap.py (6) green.

…ependency

The /healthz _CHAINS race and the MAX_PENDING parse were fixed on this branch already
(snapshot_all + _parse_max_pending). This closes the remaining sub-tier finding: the
degraded (queue-full) refusal misreported dependency health.

A refusal contacts NOTHING (attempted stays False), yet it hardcoded
hellgraph_ok=False/gateway_ok=False and a hellgraph-centric reason -- so an operator
chasing a wedged compute-gateway receipt step was pointed at hellgraph, and the server
overwrote STATE dependency flags from a call that observed no dependency at all.

  * emitter: the emitter remembers the last drain's observed health and the degraded
    refusal reports that (naming hellgraph and/or the gateway receipt step accurately)
    instead of guessing. attempted stays explicitly False.
  * server: the degraded branch no longer overwrites hellgraph_ok/gateway_ok -- the same
    attempted-gated rule the success path already applies -- it records only the
    back-pressure error; the 503 body still carries the last-observed health.

Tests: a gateway-wedged case now asserts the refusal names the gateway (not hellgraph)
and reports gateway_ok False / hellgraph_ok True; the existing hellgraph-wedged case is
updated to the accurate attribution; a _parse_max_pending guard (non-int to default,
negative to 0) is pinned; and a concurrent seal-during-healthz test reproduces the
"dictionary changed size during iteration" 500 on bare iteration and passes on
snapshot_all().

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes degraded (queue-full) back-pressure refusals so they accurately attribute the last observed stalled dependency (hellgraph vs compute-gateway receipt), and prevents the server from overwriting dependency-health state on no-op degraded refusals.

Changes:

  • Track and report the last drain’s dependency health in degraded submit refusals (instead of hardcoded hellgraph attribution).
  • Stop the server from mutating STATE["hellgraph_ok"] / STATE["gateway_ok"] on degraded refusals that attempted no work.
  • Extend test coverage for gateway-wedged attribution, _parse_max_pending guards, and a /healthz concurrency regression.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
apps/nugget-extractor/tests/test_pending_cap.py Updates/extends tests to validate correct degraded refusal attribution and max-pending parsing.
apps/nugget-extractor/src/nugget_extractor/server.py Prevents degraded (no-op) results from overwriting last-known dependency health in server state.
apps/nugget-extractor/src/nugget_extractor/emitter.py Persists last drain dependency health and uses it to explain back-pressure refusals.
apps/compute-gateway/tests/test_sub_tier_hardening.py Adds a concurrency regression test for /healthz against receipt sealing mutations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 370 to +375
result.pending = sum(len(b.nuggets) for b in self._pending)
if result.attempted:
# Remember which dependency (if any) was observed stalling, so a later
# back-pressure refusal attributes the outage from a real observation.
self._last_hellgraph_ok = result.hellgraph_ok
self._last_gateway_ok = result.gateway_ok
Comment on lines +232 to +235
who = " and ".join(stalled) if stalled else "the drain target (hellgraph or the gateway)"
reason = (
"nugget-extractor pending queue full "
f"({len(self._pending)}/{MAX_PENDING}) — hellgraph downstream may be "
"unavailable; retry after drain")
f"({len(self._pending)}/{MAX_PENDING}) — {who} is not keeping up; retry after drain")
Comment on lines +245 to +248
receipts.seal(f"churn-{i}", kind="notebook", backend="forge", runtime="python3",
inputs={"i": i}, outputs=[{"i": i}], status="ok", actor="t",
epistemic_status="derived")
receipts._CHAINS.pop(f"churn-{i}", None) # size oscillates 40 <-> 41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants