Skip to content

feat(processing): classify artifact processing failures with safe diagnostics (#2126) - #2189

Open
guamiao wants to merge 3 commits into
oceanbase:masterfrom
guamiao:fix/2126-processing-failures
Open

guamiao wants to merge 3 commits into
oceanbase:masterfrom
guamiao:fix/2126-processing-failures

Conversation

@guamiao

@guamiao guamiao commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Part of #2126.

Rationale for this change

Issue #2126 asks for one failure-classification and safe-diagnostic contract across artifact processing. A PowerContext 1.4.0 POC saw 19 of 40 Memory processing invocations fail with InvalidInferenceOutputError, and the available logs could not establish whether the failure came from generation, embeddings, output validation, or another dependency.

This PR establishes the shared contract and lands the Memory / Atomic Memory slice. It is deliberately one slice: the coverage matrix leaves the other Families Partial or Missing, and #2126 stays open until their own slices land.

What changes are included in this PR?

  • Shared contract: ProcessingStage, ProcessingFailureReason, and ProcessingDiagnostics. Every field is bounded and content-free: counts are non-negative, printable text is length-limited, and no field carries Source/Artifact bodies, prompts, model responses, credentials, or SQL parameters.
  • Provider classification: timeout, rate limit, transient unavailability, provider-confirmed context overflow, schema failures, usage limits, and embedding validation. Classification is only reported when the evidence supports it; otherwise the reason is omitted. 409/425 keep their previous transient InferenceUnavailableError semantics.
  • Memory / Atomic Memory validation: extraction/reconciliation business rules, content validation, and token-budget exhaustion attach business_validation / invalid_output or context_overflow with safe accounting fields.
  • Propagation: ArtifactProcessingWorkerFailure carries the diagnostics across the child process boundary; child and supervisor artifact_processing.failed logs emit the same allowlisted fields, and JSON and console formatting use the same contract.
  • Resilience: a diagnostics classification failure degrades to no diagnostic fields and never suppresses the failure record or changes the processing outcome.
  • Documentation: a per-Family coverage matrix plus a Memory / Atomic Memory operation-level table in both locales, the new fields in the operational field contract, and a link from the observability guides. The stage field's full value domain (Worker/supervisor boundaries, domain block names such as topic_memory, and the child worker fallback) is documented next to processing_stage.

No endpoint, status-code, or persisted-format changes. One internal exception type changes: a validation failure raised by an injected embedding model now surfaces as InvalidInferenceOutputError (embedding/invalid_output) instead of InferenceUnavailableError.

Are there any user-facing changes?

New allowlisted log fields and documentation. The embedding exception-type change means the FTS/text fallback paths in topic_memory_search, atomic_memory, and atomic_memory_processing, and the Dream retry branch, no longer treat malformed embedding output as a transient availability failure. This matches the existing malformed-output semantics. Other known limits, recorded in the coverage matrix:

  • On the default allow_continuations=True path, a provider finish reason is not always exposed, so some truncation paths are still reported as schema failures.
  • Other Families (Topic Memory, Experience, Profile, Skill, Handoff, Prompt) remain Partial or Missing until their own slices.
  • Repeated-error frequency limiting is not implemented; retry limits bound repetition indirectly.
  • MiniMax and other non-Pydantic-AI adapters attach diagnostics where their transport exposes a status; the remaining paths are listed as Partial.

Rebase and sibling PRs

The branch was rebased onto 0bc42d0b (13 commits after the previous base). The only conflict was the import block in atomic_memory/extraction.py; the diagnostics wrapper is re-applied to the current extract() body. git merge-tree --write-tree upstream/master HEAD exits 0 locally, and a range-diff against the pre-rebase head preserved both commits. #2192 rewrites the same AtomicMemoryContent block, so whichever PR lands second must re-resolve that hunk; the tests listed below were run on this rebased tree.

How was this change tested?

  • pytest tests/builtin/test_processing_diagnostics.py tests/builtin/inference tests/test_server_logging.py tests/builtin/runtime/test_artifact_processing.py tests/builtin/artifacts/atomic_memory tests/builtin/artifacts/memory tests/e2e/test_worker_logging.py -q → 276 passed, 5 skipped.
  • The spawned-worker test starts a real child process and asserts that both the child and the supervisor records carry the full diagnostic payload; a pickle round-trip covers the process boundary.
  • Public-entry tests cover the embedding validation path, the real generate() and embed() usage-limit paths, and the MiniMax status mapping.
  • Tests were checked against the previous implementation: the new behavior fails there.
  • ruff check, ruff format --check, ty check on the changed files, and git diff --check pass.

AI usage statement

AI-assisted: Claude Code (Anthropic, Claude Sonnet 4.6) was used to implement the change and write the tests. The diff was reviewed by two independent agent reviewers, and their findings were fixed before submission.

@hidb4ai hidb4ai 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.

Found three cases where the emitted diagnostics identify the wrong failure cause or stage. Details are inline.

cause_exception_type=cause_type,
),
)
if isinstance(error, (UnexpectedModelBehavior, ValidationError)):

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.

[P2] Route embedding validation failures through this mapping

Through the public PydanticAIEmbeddingModel.embed() entry point, an injected embedding model raising pydantic.ValidationError produces InferenceUnavailableError with embedding/provider_unavailable. ValidationError inherits from ValueError, so _embed() catches it at lines 287-288 before _map_error() is called. This new mapping is therefore unreachable for that failure, although the direct helper test passes. Ensure the public embedding path reports embedding/invalid_output and preserves the original exception type in diagnostics; cover this through embed() rather than only _map_error().

@guamiao guamiao Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased onto 0bc42d0. embed() routes ValidationError through the shared mapping, reporting embedding/invalid_output with cause_exception_type=ValidationError; a public-entry test covers it. The exception-type change and the FTS/text fallback impact are documented in the PR description.

return attach_diagnostics(
InvalidInferenceOutputError(operation, "provider did not return a valid result"),
ProcessingDiagnostics(
stage=ProcessingStage.USAGE_ACCOUNTING,

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.

[P2] Keep inference usage limits in the originating processing stage

With InferenceLimits(output_tokens_limit=10), a valid model response reporting 11 output tokens causes generate() to emit usage_accounting/usage_limit. Pydantic AI raises this exception while enforcing the generation budget, before the usage-reporting wrapper reaches accounting. Labeling it as an accounting failure misattributes a core processing failure and defeats #2126's requirement to distinguish the two. Keep generation/usage_limit for this case and reserve usage_accounting for failures in usage reporting or persistence.

@guamiao guamiao Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased. Usage limits are reported in the originating stage (generation, or embedding when an adapter reports it) with failure_reason=usage_limit; usage_accounting is reserved for usage reporting or persistence. Public generate() and embed() tests cover both.

Comment on lines +141 to +144
reason=(
ProcessingFailureReason.RATE_LIMIT
if rate_limited
else ProcessingFailureReason.PROVIDER_UNAVAILABLE

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.

[P2] Classify MiniMax HTTP failures from their status

Using MockTransport through the public MiniMaxEmbeddingModel.embed() entry point, HTTP 401, 408, and 504 all emit embedding/provider_unavailable: every non-429 status takes this branch. A 401 indicates an authentication/configuration problem, while 408/504 provide timeout evidence, which the Pydantic AI adapter already classifies separately. This gives operators different failure diagnoses for the same HTTP evidence. Derive the diagnostic reason from the status, or omit it when unsupported, while preserving the existing public exception contract if required.

@guamiao guamiao Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased. MiniMax diagnostics follow the HTTP status: 401/403 configuration, 408/504 timeout, 409/425/5xx provider_unavailable, 429 rate_limit, and other 4xx omit the reason. The public exception remains InferenceUnavailableError; parametrized public embed() tests cover these cases.

result = await self.extractor.generate(value)
output = AtomicMemoryExtractionOutput.model_validate(result.output)
try:
output = AtomicMemoryExtractionOutput.model_validate(result.output)

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.

[P1] This branch no longer merges into master

master has moved past the PR base. The merge-base is c0c38dea, master is now 0bc42d0b (13 commits, including #2139 which rewrote input handling in this very function). git merge-tree --write-tree master refs/pr/2189 reports exactly one content conflict, and it is in this hunk: the new try: / except ValidationError wrapper around AtomicMemoryExtractionOutput.model_validate. GitHub agrees with mergeable: false / mergeable_state: dirty, so the PR cannot merge as submitted.

Two facts to fold into the rebase:

Scope note: this is a branch-sync blocker, not a defect in the change. I re-ran the PR's own selection on the unmodified head (61df175a, Python 3.14.3): tests/builtin/test_processing_diagnostics.py tests/builtin/inference/test_pydantic_ai.py tests/builtin/inference/test_minimax_embedding.py tests/builtin/artifacts/atomic_memory tests/builtin/artifacts/memory tests/builtin/runtime/test_artifact_processing.py tests/e2e/test_worker_logging.py -> 213 passed, 5 skipped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased onto 0bc42d0. The only conflict was the import block in atomic_memory/extraction.py; the try/except wrapper still covers model_validate and AtomicMemoryContent. git merge-tree --write-tree upstream/master HEAD exits 0 locally, a range-diff against the pre-rebase head preserved both commits, and the PR description records the #2192 overlap on the same hunk. Tests on the rebased tree: 276 passed, 5 skipped.

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.

@guamiao, GitHub currently reports merge conflicts with master (mergeable_state: dirty). Please update this branch against the latest master and resolve the conflicts, preserving the latest master changes and the new diagnostics. Then rerun the Memory/Atomic Memory, inference, and worker logging tests, and update the PR with the validation results.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Conflict resolutions (70f6aa7e):

  • inference/pydantic_ai.py: kept master's _checked_rule helper and the bounded _map_error noqa.
  • runtime/artifact_processing.py: kept master's processing_error_code(type, error_code) call and re-applied the diagnostics classification.
  • runtime/atomic_memory_processing.py: merged master's new SOURCES_TABLE import with the diagnostics imports.
  • tests/builtin/inference/test_pydantic_ai.py: kept master's expanded imports and added UsageLimitExceeded / _map_error.

atomic_memory/extraction.py merged without a conflict on this head; the try/except ValidationError wrappers around AtomicMemoryExtractionOutput.model_validate and AtomicMemoryContent remain intact on top of the current body. #2192 is still open; if it lands first, this block will need the same re-resolution noted earlier.

Validation on 70f6aa7e (Python 3.14.3):

  • pytest tests/builtin/test_processing_diagnostics.py tests/builtin/inference tests/builtin/artifacts/atomic_memory tests/builtin/artifacts/memory tests/builtin/runtime/test_artifact_processing.py tests/e2e/test_worker_logging.py -q → 272 passed, 5 skipped
  • ruff check, ruff format --check, ty check on the changed files → clean
  • git diff --check upstream/master..HEAD → clean
  • GitHub reports mergeable: true.

The stacked Topic Memory branch was rebased onto this head (f7a06284); its suite is 555 passed, 5 skipped.

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.

复核新 head 70f6aa7e:mergeable=MERGEABLE,rebase 后与 master 无冲突,本线程的 P1 已解除。独立验证(exact head 树 + 隔离 PYTHONPATH):tests/builtin/test_processing_diagnostics.py 与 tests/builtin/inference/test_minimax_embedding.py 50 passed、tests/builtin/inference/test_pydantic_ai.py 39 passed;extraction.py 的 try/except ValidationError 包裹在 rebase 后仍覆盖 model_validate。按已修复记录,不再重复提出。

| `failure_location` | Stable internal location or operation identifier | Bounded string |
| `cause_exception_type` | Original provider or library exception type before mapping | Bounded string |

`processing_stage` is distinct from the existing `stage` field, which names the Worker or

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.

[P2] stage is not limited to Worker/supervisor boundaries

This sentence is the only basis for telling stage and processing_stage apart, but it understates the real value domain of stage. _log_failure emits error.failure.stage, which is copied from block.stage in the child Worker (src/powercontext/builtin/runtime/artifact_processing.py), and the only producer of a block sets stage="topic_memory" (src/powercontext/builtin/runtime/topic_memory_scope.py:85). The child also falls back to _safe_error_attribute(error, "stage", "worker"), so an exception-defined domain value can reach the field as well.

Impact: a real record such as stage=topic_memory next to processing_stage=generation reads, under this contract, as if stage were a boundary name, and the failure gets attributed to the wrong layer. The same wording is duplicated in docs/zh/development/processing-failures.md.

Suggest documenting the actual domain (boundary values, plus domain block names such as topic_memory, plus the worker fallback), or changing the field so that the documented distinction is accurate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated both locales. stage now documents Worker/supervisor values (worker_process, work_admission, supervisor, lease_renewal, scope_discovery, pending_observation, term_revocation, worker_termination), domain block names such as topic_memory, the child worker fallback, and the omitted-stage case. processing_stage remains the bounded domain stage, and usage_accounting is marked reserved.

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.

复核 70f6aa7e:上一条 P2 已闭合。docs/{en,zh}/development/processing-failures.md 现在把 stage 定义为"发出失败的位置或领域位置",并逐项列举 _log_failure 的全部实参——artifact_processing.py:678 supervisor、:736 lease_renewal、:779 scope_discovery、:807 pending_observation、:1141 work_admission、:1434 worker、:1469 term_revocation、:1499 worker_termination,加上 worker_process(:345/352/364)与领域 block topic_memory(topic_memory_scope.py:85),并说明 child Worker 的 worker 兜底与"没有领域 block 时可省略"。逐一比对源码与该表,取值域一致,无遗漏或臆造项;docs/{en,zh}/docs/operate/observability.md 的同一句也已同步。中英一致。按已修复记录,不再重复提出。

guamiao added a commit to guamiao/powercontext that referenced this pull request Oct 22, 2026
…tion (oceanbase#2189)

Route a validation failure from an injected embedding model through the shared
mapping so the public embed() path reports embedding/invalid_output and keeps
the original exception type instead of classifying it as a transient outage.
Report inference usage limits in their originating stage and reserve usage
accounting for usage reporting or persistence. Derive MiniMax diagnostics from
the HTTP status while keeping its public exception contract.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
guamiao added a commit to guamiao/powercontext that referenced this pull request Oct 22, 2026
…tion (oceanbase#2189)

Route a validation failure from an injected embedding model through the shared
mapping so the public embed() path reports embedding/invalid_output and keeps
the original exception type instead of classifying it as a transient outage.
Report inference usage limits in their originating stage and reserve usage
accounting for usage reporting or persistence. Derive MiniMax diagnostics from
the HTTP status while keeping its public exception contract.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
guamiao added a commit to guamiao/powercontext that referenced this pull request Oct 22, 2026
…ceanbase#2189)

Describe every stage value the log can carry, mark usage accounting as
reserved, and report usage limits in their originating generation or embedding
stage. Add the public embedding usage-limit regression test and rebase the
branch onto the current master.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@guamiao
guamiao force-pushed the fix/2126-processing-failures branch from f334987 to 6e13c90 Compare October 22, 2026 10:22
@AlexStocks AlexStocks added the 1.4.1 v1.4.1 release tracking label Oct 23, 2026

@hireview001 hireview001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • Resolve merge conflicts with the target branch.

@hidb4ai hidb4ai added 1.5.0 v1.5.0 release tracking and removed 1.4.1 v1.4.1 release tracking labels Oct 25, 2026
guamiao and others added 3 commits October 25, 2026 13:40
…gnostics (oceanbase#2126)

Add a shared failure contract for artifact processing and apply its first
slice to Memory / Atomic Memory. Failures now carry a bounded processing
stage and reason plus safe diagnostics (input item count, estimated input
tokens and estimator identity, effective budget, provider request id and
finish reason, original exception type, and failure location). The fields are
allowlisted for JSON and console output and travel from a spawned Worker to
the supervisor.

Provider errors are classified only when the evidence supports it: timeout,
rate limit, transient unavailability, provider-confirmed context overflow,
schema failures, usage limits, and embedding validation. Budget exhaustion
and extraction/reconciliation rules attach business-validation diagnostics.
Diagnostics themselves degrade to no fields and never change the processing
outcome.

Publish the per-Family coverage matrix and the Memory / Atomic Memory
operation-level table. Other Families remain Partial or Missing until their
own slices land; this change is part of oceanbase#2126, not its completion.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…tion (oceanbase#2189)

Route a validation failure from an injected embedding model through the shared
mapping so the public embed() path reports embedding/invalid_output and keeps
the original exception type instead of classifying it as a transient outage.
Report inference usage limits in their originating stage and reserve usage
accounting for usage reporting or persistence. Derive MiniMax diagnostics from
the HTTP status while keeping its public exception contract.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ceanbase#2189)

Describe every stage value the log can carry, mark usage accounting as
reserved, and report usage limits in their originating generation or embedding
stage. Add the public embedding usage-limit regression test and rebase the
branch onto the current master.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@guamiao
guamiao force-pushed the fix/2126-processing-failures branch from 6e13c90 to 70f6aa7 Compare October 25, 2026 05:43

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.5.0 v1.5.0 release tracking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants