Skip to content

fix: expose bounded candidate publisher failure diagnostics - #489

Merged
Pigbibi merged 1 commit into
mainfrom
fix/candidate-failure-diagnostics
Nov 1, 2026
Merged

Pigbibi merged 1 commit into
mainfrom
fix/candidate-failure-diagnostics

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

AI assistance

OpenAI Codex; the exact host model identifier is not exposed in this session. The author performed a local self-review of the base-to-head diff; this is not an independent review.

What changed

The scheduled candidate publisher hides binding, archive, schedule and projection failures behind candidate_projection_failed. Add fixed stage and error_code fields using exact allowlisted local codes, preserve the existing storage error_category, and keep unknown failures explicit without printing exception text, identities, credentials or object paths.

Preserve the existing coverage-alert attempt and same-run suppression, but suppress raw legacy-sender output and report only a bounded secondary delivery diagnostic. Keep the original nonzero result, candidate merge/write behavior and success output. Document that uploaded=false does not prove an uncertain write or earlier target was not persisted.

This change does not modify cloud configuration, IAM, schedules, dependencies or trading behavior. No deployment or workflow replay was performed.

How verified

  • Before the patch: 20 new diagnostic/redaction cases failed and the success control passed.
  • Focused publisher, daily-policy, pre-read publisher and candidate tests: 110 passed.
  • Final full local suite, including all 23 new cases: 1,543 tests and 13 subtests passed (uv run --no-sync python -m pytest -q -o pythonpath=.). The path override uses the installed locked dependencies instead of adjacent development checkouts. One existing third-party NumPy deprecation warning.
  • PR CI: 1,543 tests and 13 subtests passed; locked shared refs, pinned-package imports, dependency compatibility, Ruff, QPK pin consistency, lock check and package build all passed. Documentation and engineering-governance workflows passed.
  • Local frozen dependency sync, dependency compatibility, Ruff, lock check, sdist/wheel build and git diff --check passed. Documentation check: zero blocking findings, one existing README advisory.
  • Tests use synthetic values and offline boundaries, including the real candidate merge and idle projection. They check no retry after an uncertain write, redaction, alert-fallback isolation, missing-source handling, output serialization and unchanged success output.
  • Self-review found no blocking defects. Historical incident analysis remains separate from these offline tests.

Co-Authored-By: Codex <noreply@openai.com>
@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5ab3bcd0-3b13-4fc8-9b30-666682b6f169
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@Pigbibi Pigbibi left a comment

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.

Author self-review of 1e3715a..4ed6710 (not an independent review): no in-scope blocking findings. Diagnostic values are selected from fixed vocabularies; unknown exceptions are never stringified. Existing success output, same-run alert suppression, nonzero failure exit and candidate storage retry behavior remain unchanged. Synthetic tests cover raw sender-output suppression and read/write failure boundaries. No workflow, cloud configuration, dependency or trading paths changed. Full local suite and remote CI are still required before merge.

@Pigbibi Pigbibi added the owner-approved Owner approval for high-risk PRs. AI agents must never apply this label. label Nov 1, 2026 — with ChatGPT Codex Connector
@Pigbibi
Pigbibi merged commit df137a2 into main Nov 1, 2026
10 checks passed
@Pigbibi
Pigbibi deleted the fix/candidate-failure-diagnostics branch November 1, 2026 07:45

Pigbibi commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Post-hoc SHA-bound review

  • reviewer=Grok (post-hoc)
  • Execution provenance: Codex performed this review and records it on behalf of Grok under the owner's explicit request. This is a delegated governance record, not a claim that an xAI/Grok model session ran. The publishing GitHub account is also the PR author's account.
  • Reviewed head SHA: 4ed6710fe843c36c5f602de1194daeec6bb02c35
  • Reviewed base SHA: 1e3715a5bd05f0bfb43b4134638f4ad75ac1adb9
  • Merge SHA: df137a203ec89264c4ce6ef0b8463e4ba81a4c82 (df137a2)
  • Verdict: APPROVE — post-hoc code review; no actionable defects found.
  • Scope assessment: R1, ordinary diagnostic/error-handling behavior. This diff does not change authentication, permissions, secret access, deployment, workflows, or live-trading execution.

Findings: none requiring correction. Reviewed all three changed files (238 additions, 6 deletions), including the actual archive/projector/storage and heartbeat-sender call paths. The reviewed head and merge commit have identical Git trees.

  1. Diagnostic values use exact local allowlists; unknown exceptions are not echoed or loosely matched. Existing storage categories are retained, while invalid categories fall back to storage_unknown.
  2. The original nonzero failure result and primary stdout diagnostic remain intact when the alert fallback returns false or raises. Raw sender stdout/stderr is suppressed. Existing same-run alert suppression and candidate merge/write retry behavior are unchanged.
  3. The documentation correctly states that uploaded=false cannot establish that an uncertain write or an earlier target did not persist. The success payload is unchanged.

Verification on the exact reviewed head: 111 relevant tests passed, plus 2 supplemental offline checks of the exact secondary stderr diagnostic, one fallback attempt, redaction, and preservation of the primary failure. Focused Ruff and base-to-head git diff --check passed. Existing head-associated CI, documentation, and engineering-governance runs were read back as successful. The full suite was not rerun for this review.

Governance note: Author self-review is not a substitute for the Grok record. The earlier author self-review and CodeRabbit's skipped-review notice did not supply that record. Under the owner's amendment, R2 requires both a current-head AI review verdict and a SHA-bound Grok approval record; human sign-off remains required for deployment, IAM, secrets, and live-trading execution changes. This comment records the requested delegated post-hoc decision only: it does not establish that review existed before the merge, assert an independent GitHub reviewer identity, or certify acceptance by an automated governance evaluator.

No follow-up PR is warranted because no actionable code issue was found. No deployment, cloud/IAM/secret change, workflow replay, or live notification/trading execution was performed.

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

Labels

owner-approved Owner approval for high-risk PRs. AI agents must never apply this label.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant