feat: add stable review contract identity core#80
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 2 serious issue(s) found in high-risk files 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 654a225450
ℹ️ 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".
|
|
||
|
|
||
| def _normalize_contract_text(value: Any) -> str: | ||
| text = _sanitize_history_text(value, 240).lower() |
There was a problem hiding this comment.
Because this normalizer now feeds _contract_subject()/_contract_key(), using _sanitize_history_text() here makes the secret redactor run on code anchors before tokenization. Common identifiers that contain digits and are 8-39 chars long, such as parse_v1_history, parse_v2_history, or fingerprint_v2, are all normalized to redacted, so unrelated findings in the same file/category can get the same contract_key/fingerprint_v2 and be persisted as the same contract. That corrupts the new v2 history identity for versioned APIs or symbols; the identity path needs a sanitizer that preserves code identifiers while still scrubbing secrets from stored text.
Useful? React with 👍 / 👎.
| if not isinstance(findings, list): | ||
| findings = [] | ||
| findings = [ | ||
| {**finding, **_contract_finding(finding)} |
There was a problem hiding this comment.
This enrichment overwrites the live file field with _contract_finding()'s history-safe value before evaluate_findings() runs, and that path is truncated to 300 characters by _sanitize_history_path(). For a high-severity finding on a changed file whose repository path is longer than 300 characters, file_path in changed_paths no longer matches the full GitHub filename, so the serious finding is treated as non-blocking and the PR can pass. The contract fields should be added without replacing the original file path used for risk/blocking decisions.
Useful? React with 👍 / 👎.
| "with", "must", "should", "will", "return", | ||
| } | ||
| } | ||
| return " ".join(sorted(words)) |
There was a problem hiding this comment.
This collapses each finding's behavior into a sorted set of tokens before hashing, so opposite mappings with the same words produce the same behavior_digest/fingerprint_v2. For example, suggestions like “treat pending jobs as blocked and completed jobs as clear” and “treat pending jobs as clear and completed jobs as blocked” normalize identically when they reference the same anchor. That persists contradictory contracts as the same behavior in v2 history, so the digest should retain enough ordering or relational structure to distinguish swapped requirements.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8da2171463
ℹ️ 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".
| marker_version = 1 | ||
| else: | ||
| # Unknown marker versions are ignored so mixed deployments can roll back safely. |
There was a problem hiding this comment.
When a trusted comment was written by a newer history namespace, such as codex-pr-review-history:v3 during a rollback or mixed deployment, this branch treats the marker as absent and valid. That makes finding_history empty, so active blocking history can be forgotten and low-risk or clean runs can proceed without the required arbitration; unknown history markers should be treated as malformed/fail-closed rather than silently ignored.
Useful? React with 👍 / 👎.
1 participant
Summary
This is the C2a replacement slice for #79. Conflict reduction, clear/supersession semantics, and state-machine activation are intentionally deferred to C3.
Verification