Skip to content

feat: clean-slate vNext identity schema v1 foundation - #47

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

feat: clean-slate vNext identity schema v1 foundation#47
Pigbibi wants to merge 2 commits into
mainfrom
codex/qar-vnext-n1-schema-v1-reslice

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Scope

Clean-slate QAR-N1 pure identity/index/allocation/PublicationPlan foundation from fresh origin/main@d13dad1cfd2680cfdf9c96bc4687fc3d32ffc015.

Type/wire dispatch is the source of truth. VNextIdentityBinding and qar_vnext_binding.v1 are independent from legacy types. The vNext parser/allocator/PublicationPlan accept only the exact clean-slate schema-v1 namespace; no caller routing flag, filename routing, legacy fallback, dual-read, migration, or compatibility path.

Contract matrix covered

  • fixed schema/namespace and exact immutable semantic/artifact algorithm pair;
  • strict report schema/contract pairing via contract_version_for_schema();
  • cadence-aware daily/weekly/monthly canonical and full artifact-digest variant targets;
  • optional attachment omission vs explicit null/type rejection;
  • exactly one canonical per period, display-primary/order policy and safe integer bounds;
  • exact/current/historical allocation, empty-period bootstrap, idempotent exact reuse, changed-artifact variant;
  • source basename independent from public target;
  • collision, digest, period, class, status, legacy/unknown version rejection;
  • deterministic Mapping snapshot, serializer round-trip, permutation and sanitized adversarial errors.

Explicit non-scope

No legacy compatibility/migration, old index reads, publisher/build/archive/RSS/workflow/Pages integration, filesystem/network I/O, producer changes, or N2.

Validation

  • focused: python3 -m pytest -q tests/test_vnext_schema_v1.py — 24 passed
  • full: python3 -m pytest -q — 289 passed
  • python3 -m ruff check src tests — passed
  • python3 -m compileall -q src tests — passed
  • git diff --check — passed

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: SelectedCandidate is a public exported dataclass with no __post_init__, so callers can construct SelectedCandidate(...) directly with arbitrary report_snapshot, source_identity, and forged digest/schema fields instead of going through from_report()/report_identity_evidence(). allocate_identity() only checks isinstance(candidate, SelectedCandidate) and then trusts those fields in _same_artifact() and _new_binding(), while PublicationEntry.__post_init__() only checks field equality via _validate_candidate_binding(). That means exact-reuse and new-binding paths both accept candidate metadata that was never derived from a validated report, contrary to the documented contract that the source report is the evidence and raw inputs are validated before use. The prior finding was about preserving context.display in bindings; this PR now does that in _new_binding(..., display.primary, display.order), and the current finding does not contradict that behavior.

🚫 Blocking Issues

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

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

SelectedCandidate is the only core value object here without a __post_init__ invariant check. Callers can instantiate it directly with an arbitrary or mutable report_snapshot and forged metadata instead of using from_report(), and both allocate_identity() and PublicationEntry accept it based only on isinstance. That lets unvalidated evidence bypass report_identity_evidence(), including the exact-artifact reuse path, so downstream code can treat a non-matching or later-mutated report snapshot as if it were the validated source for an existing binding. (line 109)

Suggestion: Add SelectedCandidate.__post_init__ (or otherwise prevent direct construction) to enforce immutability and validate all fields. At minimum, freeze report_snapshot, validate source_identity, period/schema/contract/digest formats, and recompute or verify the candidate metadata from report_snapshot so every accepted candidate has gone through the same checks as from_report().


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

Closure evidence (closure_count=1/max=1, absolute_final=true):

  • allocate_identity() now persists the requested AllocationContext.display for new canonical/variant bindings; exact reuse and display-policy validation use the same stored semantics. Added bootstrap/rerun/changed-artifact regression.
  • _validate_index() now rejects duplicate full artifact identity (period_key, as_of, cadence, report_schema_version, contract_version, semantic_digest, artifact_integrity_digest) before target allocation, preventing one exact artifact from acquiring multiple identities.
  • parser and serializer now use one stable binding sort key, making input permutations produce equal index values and deterministic round-trip wire output.

Validation on final head: focused 25 passed; full pytest 290 passed; ruff, compileall, and git diff --check passed. No legacy/compatibility/registry/migration or N2/I/O changes.

@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: 7909f94dac

ℹ️ 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 +221 to +224
previous = artifact_digests.get(binding.artifact_integrity_digest)
current = (binding.period_key, binding.as_of, binding.cadence)
if previous is not None and previous != current:
raise _error("artifact_digest_conflict")

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 repeated artifact digests within a period

When a second binding has the same artifact_integrity_digest and the same period/as_of/cadence, this condition treats it as valid. That lets a forged or merged vNext index contain the exact same report as both canonical and variant, or contain two variants that share the JSON/HTML targets and differ only by optional md/manifest; allocate_identity would have reused the existing binding instead, so downstream publication can see multiple identities/targets for one artifact. The duplicate should be rejected unless it is the exact same binding, which is already caught by identity_duplicate.

Useful? React with 👍 / 👎.

Comment on lines +142 to +147
if not isinstance(self.candidate, SelectedCandidate) or not isinstance(self.binding, VNextIdentityBinding):
raise _error("publication_entry_invalid")
if type(self.role) is not PublicationRole:
raise _error("publication_role_invalid")
_display(self.display_primary, self.display_order)
_validate_candidate_binding(self.candidate, self.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 Validate candidates against their snapshots

When callers instantiate the exported SelectedCandidate dataclass directly, these checks trust the stored digest/metadata fields and never recompute them from report_snapshot or re-sanitize source_identity. A PublicationEntry can therefore be accepted with snapshot A but binding/digest B, so downstream publication code that writes candidate.report_snapshot under binding targets would publish bytes that do not match the identity evidence. Rebuild the candidate from its snapshot here, as the legacy plan validation does, before accepting the entry.

Useful? React with 👍 / 👎.

Comment on lines +258 to +263
binding = _new_binding(
candidate,
identity_class,
context.requested_artifacts,
DisplayPlacement(False, next_order),
)

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 Honor requested display placement when allocating

When a new current or historical binding is allocated, this hard-codes the binding to display_primary=False and an internally chosen order instead of using context.display. For example, a current allocation requested as DisplayPlacement(True, 0) is persisted as non-primary, so a later exact reuse with the same requested placement is rejected as identity_reuse_mismatch, and any consumer that relies on the index display fields will not mark the mandatory current report as primary.

Useful? React with 👍 / 👎.

@Pigbibi

Pigbibi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Permanent freeze/close-reslice: closure_count=1/max=1, absolute_final=true; fingerprint=b573ba2b11fff768d515. Preserve branch/worktree/commits/comments/unresolved review evidence/tests. No further code action.

@Pigbibi Pigbibi closed this Jul 14, 2026
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