Skip to content

feat: add converged clean-slate QAR-N1 identity allocation - #45

Closed
Pigbibi wants to merge 2 commits into
mainfrom
codex/qar-vnext-n1-converged-reslice
Closed

feat: add converged clean-slate QAR-N1 identity allocation#45
Pigbibi wants to merge 2 commits into
mainfrom
codex/qar-vnext-n1-converged-reslice

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

QAR-N1 converged clean-slate identity allocation

This PR is a fresh implementation from origin/main@d13dad1cfd2680cfdf9c96bc4687fc3d32ffc015. It does not reopen, reuse, cherry-pick, or inherit PR #44. The vNext namespace is clean-slate only: no legacy filename/index dual-read, migration, fallback, or compatibility mode.

Scope

  • Pure qar_vnext.identity_index.v1 wire codec and whole-index validation.
  • Shared strict vNext binding validator used by parser, allocator, and vNext PublicationPlan entries.
  • Pure allocation for EXACT_ARTIFACT_REUSE, CURRENT_MANDATORY, and HISTORICAL_RECOVERY.
  • No filesystem/network/publisher/build/workflow/Pages/producer/migration/real I/O.

Contract matrix covered locally

  • Strict top-level/entry keys; only V3_CANONICAL/V3_VARIANT; legacy and old versions rejected.
  • Period/as_of/cadence consistency; schema support and schema-contract pairing delegated to contract_version_for_schema.
  • Exact fingerprint/artifact algorithms and full lowercase digest validation.
  • Cadence-aware JSON/HTML/MD/manifest names; canonical unsuffixed; variants use the full artifact digest suffix.
  • Optional attachment key presence: omitted is not requested; present must be a valid basename string; null/empty/wrong type rejected and never silently dropped.
  • Empty current bootstrap; occupied current changed-artifact variant; identical current canonical reuse; historical no-canonical fail-closed.
  • Attachment policy across all modes; EXACT stored-display equality; CURRENT new display without binding mutation; HISTORICAL display independent of identity.
  • Exact-miss prevalidation, whole-index collisions/canonical invariants, permutation stability, deterministic parse/serialize round-trip.
  • Adversarial forged metadata, legacy names/classes, unknown/mismatched schema, digest/name collisions, optional nulls, and malformed report shapes.

Validation

  • Focused vNext tests: 31 passed
  • Full pytest: 296 passed
  • Targeted ruff: passed
  • compileall: passed
  • diff-check: passed
  • Full ruff reports 28 pre-existing out-of-scope violations in scripts/gate_codex_app_review.py.

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

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

🚫 Merge blocked: 1 serious issue(s) found in high-risk files

⚖️ Codex Review Arbitration

🚫 block: The prior blocker is only partially fixed. The diff removes the default by making PublicationEntry.identity_namespace required, but src/quant_advisor_research/publication_plan.py still uses that field only to choose the validator: _validate_entry() accepts either "legacy" or "vnext" and then calls _validate_binding(..., strict_vnext=entry.identity_namespace == "vnext"). In the strict_vnext=False branch, _validate_binding() still accepts any V3IdentityBinding whose identity_class is V3_CANONICAL or V3_VARIANT and runs the legacy _validate_v3_entry path, with no check that the binding itself belongs to the legacy namespace. That means a vNext binding can still be misrouted through identity_namespace="legacy". This matches the current finding and also violates the new contract evidence in docs/qar_vnext_identity_contract.md, which says "vnext" uses the shared clean validator and "legacy" remains an explicitly isolated old-runtime path rather than a compatibility mode. The current finding does not conflict with the prior one; it is the remaining fail-closed requirement for the same issue.

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Logic in src/quant_advisor_research/publication_plan.py

identity_namespace is only used to choose which validator runs, but there is no check that the namespace matches the binding shape. As written, a caller can pass a cadence-aware vNext V3IdentityBinding with identity_namespace="legacy", and _validate_entry() will accept it via the legacy/non-strict path. That breaks the PR's stated isolation guarantee ("legacy" remains an explicitly isolated old-runtime path) and creates a real bypass where vNext publications can be routed through legacy behavior instead of the strict clean-slate contract. (line 274)

Suggestion: Make namespace/binding pairing explicit and fail closed. When identity_namespace == "vnext", require the binding to satisfy the strict vNext contract; when identity_namespace == "legacy", reject vNext-shaped bindings (or add an explicit binding namespace/discriminator and validate it before selecting the validator).


Review by Codex PR Review bot • PR

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

Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Permanent freeze: closure_count=1/max=1, absolute_final=true. Remaining HIGH is the vNext binding type/namespace discriminator gap: caller routing can label a vNext-shaped binding as legacy. Preserve branch/worktree/review evidence; no further code action.

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.

1 participant