Skip to content

feat: add clean-slate vNext identity allocation - #44

Closed
Pigbibi wants to merge 4 commits into
mainfrom
codex/qar-vnext-identity-allocation-n1
Closed

feat: add clean-slate vNext identity allocation#44
Pigbibi wants to merge 4 commits into
mainfrom
codex/qar-vnext-identity-allocation-n1

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Scope

QAR-N1 adds a clean-slate vNext identity/index wire foundation and pure allocation API. It is side-by-side with the legacy system and does not read or migrate old index versions.

Contract

  • Independent wire namespace: qar_vnext.identity_index.v1 with exact {schema_version, entries} shape.
  • Only V3_CANONICAL and V3_VARIANT are accepted; LEGACY_V2 fails closed.
  • Explicit EXACT_ARTIFACT_REUSE, CURRENT_MANDATORY, and HISTORICAL_RECOVERY modes.
  • Raw report, request policy, display evidence, context, and complete vNext index are validated before lookup.
  • Empty current bootstraps canonical; occupied current period allocates full artifact-digest variant; historical without canonical fails closed.
  • Deterministic serializer, whole-index collision/canonical/digest simulation, and direct PublicationPlan binding for current entries.

Boundaries

No old index fallback/migration, allocator compatibility adapter, publisher/build/archive/RSS/workflow/Pages integration, filesystem/network I/O, producer changes, or new dependencies.

Verification

  • Focused vNext tests: 15 passed
  • Full pytest: 280 passed
  • Targeted ruff: passed
  • compileall: passed
  • diff-check: passed
  • Full repo ruff has 28 pre-existing E701/E702/F541 violations in scripts/gate_codex_app_review.py; out of scope and unchanged.

Source of truth: quant_system_clean_slate_vnext_decision_2026-07-15.md

No merge, rerun, polling, or automation changes performed.

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: block because the current finding is proven by the PR source. In src/quant_advisor_research/vnext_binding.py, validate_vnext_binding() allows optional keys whenever the key set matches, then does md = None if "md" not in entry else entry["md"] and only validates md/manifest inside if md is not None / if manifest is not None. Therefore {"md": null} and {"manifest": null} are accepted as present-but-unvalidated wire fields and become markdown_name=None / manifest_name=None in the returned V3IdentityBinding. src/quant_advisor_research/vnext_binding.py binding_payload() and src/quant_advisor_research/vnext_identity.py serialization omit those fields when the binding value is None, so the parser silently normalizes a distinct input payload into a different serialized payload. That contradicts the documented wire contract in docs/qar_vnext_identity_contract.md, which describes optional md/manifest names, not nullable declared attachments, and it is not disproved by any test in tests/test_vnext_identity.py. No contract conflict with the prior blocking finding: the prior issue was about legacy HTML-name compatibility, and this PR explicitly separates legacy vs vNext validation in publication_plan.py and documents that vNext accepts only cadence-aware names, so the current null-attachment fix would not reverse that behavior.

🚫 Blocking Issues

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

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

validate_vnext_binding() accepts md and manifest when the keys are present with null values. Because the code only validates those fields when they are not None, parse_vnext_index() will treat {"md": null} / {"manifest": null} as valid and convert them into markdown_name=None / manifest_name=None. serialize_vnext_index() then drops those keys entirely, so an invalid wire payload is silently accepted and normalized into a different payload. This is a public contract break and differs from the existing v2 parser, which fails closed on explicit null attachments. (line 140)

Suggestion: When md or manifest keys are present, require them to be exact str basenames and reject null explicitly. Mirror the existing v2/v3 attachment validation: omission means 'not requested', but presence must mean a valid filename string.


Review by Codex PR Review bot • PR

@Pigbibi

Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Independent source-of-truth clarification for clean-slate vNext N1:

  • Identity reuse is artifact-identity based. If CURRENT_MANDATORY supplies the exact same validated artifact as an existing canonical binding, it is the same immutable identity and must reuse that binding.
  • Forcing a variant for an identical artifact would create duplicate public identities for the same content and break idempotence.
  • CURRENT_MANDATORY allocates a V3_VARIANT only when the period is occupied and the artifact identity differs.
  • AllocationResult.reused_existing is the future N2 publisher signal to perform an idempotent no-op/byte-identical reuse; N1 performs no I/O.
  • Therefore the first HIGH is rejected as conflicting with the clean-slate identity contract. Add characterization evidence; do not change this behavior.
  • The cadence filename collision HIGH is valid. Every vNext json/html/md/manifest target must include cadence (or equivalent full period identity).

This clarification is limited to PR #44 pure N1 and does not authorize publisher/build integration.

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

Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Independent contract arbitration for clean-slate vNext N1:

  1. The historical review request to force a variant for an identical CURRENT artifact is superseded by issuecomment-4972554876. Exact artifact identity reuse is the approved idempotent contract; a changed artifact in an occupied period allocates a variant.
  2. The current full-binding-validation HIGH is supported. Cadence-aware filenames must not bypass period/schema/contract/version/digest/class/status validation. Use one shared vNext-aware full validator; do not broadly fall back after the legacy validator fails.
  3. The current display-policy HIGH is supported. Attachment presence is immutable target policy and must match on every reuse. Display placement is plan evidence, not identity:
    • EXACT_ARTIFACT_REUSE keeps exact attachment + display equality.
    • CURRENT_MANDATORY exact reuse keeps the immutable binding but applies the newly validated display values to PublicationEntry/PublicationPlan.
    • HISTORICAL_RECOVERY does not treat stored display as identity; input shape is validated, attachment targets still match.
  4. The immutable binding's stored display fields are not rewritten during reuse.
  5. This clears one second-and-FINAL bounded remediation for PR feat: add clean-slate vNext identity allocation #44 only. No publisher/build/Pages integration or compatibility behavior is authorized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed92da627e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +266 to +271
except IdentityMetadataError:
try:
_validate_clean_binding_names(binding)
except (PublicationPlanError, AttributeError, KeyError, TypeError, ValueError, OverflowError, UnicodeError):
raise _error("identity_binding_invalid") from None
return binding

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep rejecting bindings that fail non-name metadata checks

When a vNext-style name makes _validate_v3_entry raise, this fallback accepts the binding after checking only the filenames. In a direct build_publication_plan call, a binding that failed for another reason (for example identity_class='V3_CANONICAL' with canonical_identity=False, or invalid binding display fields) is still accepted as long as its clean-slate target names match. That lets publication plans carry internally inconsistent identity metadata, which the vNext contract says must be validated before binding.

Useful? React with 👍 / 👎.

Comment on lines +202 to +203
md = None if "md" not in entry else entry["md"]
manifest = None if "manifest" not in entry else entry["manifest"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject nullable optional artifact names

If a wire entry includes "md": null or "manifest": null, this treats the present null exactly like an absent optional artifact, so the strict vNext parser accepts malformed wire and serialize_vnext_index silently drops the key. In contexts that depend on the exact {schema_version, entries} wire contract and requested-artifact policy, present optional artifact fields should be validated as strings instead of being normalized away.

Useful? React with 👍 / 👎.

Pigbibi and others added 2 commits July 15, 2026 02:38
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Permanent freeze: remediation_count=2 FINAL, human_exception_count=1/max=1, absolute_final=true. Remaining HIGH is vNext attachment key-presence contract (omitted md/manifest vs explicit null). 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