From 85b3097202456ca93b19eb850e4beeaf2bae1c9f Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:59:29 +0200 Subject: [PATCH 1/8] Plan toolkit v0.8.3 fixes --- PLANS.md | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 233 insertions(+), 1 deletion(-) diff --git a/PLANS.md b/PLANS.md index ca171a2..3542563 100644 --- a/PLANS.md +++ b/PLANS.md @@ -4,4 +4,236 @@ Use this file for active or blocked repository work. Update it before implementa ## Active Work -No active or blocked repository work. +### Toolkit 0.8.3 — authoritative OCR compatibility status and numeric CLI limits + +Status: `active` +Owner: Codex +Plan Origin: `direct_execution` +Release classification: `release-required` +Target stable version: `0.8.3` +Last Updated: 2026-08-25 + +#### Goal + +Close #139 and #140 as one protected 0.8.3 bug-fix release: make a failed OCR +qualification's closed status authoritative even when stale evidence or invalid +support metadata exists, and replace the stale example-owned `OCR_MAX_TOOLS=30` +contract with installed-OCR delegation plus behaviorally qualified numeric CLI +boundaries. Preserve private diagnostics, aggregate-promotion blocking, the exact +OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. + +#### Requested Scope + +- Select OCR compatibility issue input from the actual qualification outcome; + retained evidence from a failed step must never win over its failed status. +- Ensure every handled qualification failure after evidence creation, including + a late issue-body write failure, commits a closed failure status before the + workflow publishes the issue/artifact and restores the red job outcome. +- Let `upsert-issue --status` validate and publish a closed status independently + of an invalid support manifest that the command does not consume. +- Replace the GitLab example's release-specific max-tools value with OCR's + supported `0` sentinel and document the exact default as inherited from OCR. +- Recognize only the installed OCR's closed max-tools normalization diagnostic; + expose a toolkit-authored operator notice without publishing raw stderr or + treating the diagnostic's reported minimum as proof of the effective loop cap. +- Extend OCR qualification to exercise the behavior and effective semantics of + toolkit-managed numeric OCR review options at omitted/default, sentinel, + invalid-below-boundary, accepted-boundary, representative, and maximum edges + where a maximum exists. Record only bounded closed facts in evidence. +- Add separate bug-fix changelog fragments for #139 and #140, update canonical + development/public contracts, and complete the protected PR and stable-release + lifecycle without a local real-LLM/model invocation. + +#### Requirement Traceability + +| Requirement | Source | Outcome | Work items | Verification | +| --- | --- | --- | --- | --- | +| `REQ-139-A` | #139 acceptance 1 | Qualification outcome, not file existence, selects evidence versus status | `WQ-02` | Workflow contract test with retained evidence and failed outcome | +| `REQ-139-B` | #139 acceptance 2 | Late output failure produces authoritative failed status | `WQ-02` | Injected issue-body and partial/failed write tests | +| `REQ-139-C` | #139 acceptance 3 | Status recovery does not load the support manifest | `WQ-02` | Invalid-manifest CLI recovery test | +| `REQ-139-D` | #139 acceptance 4-6 | Issue/artifact publish before red job; aggregate blocked; projection stays closed and private-safe | `WQ-02`, `WQ-04` | Focused script/workflow tests and privacy diff review | +| `REQ-140-A` | #140 expected behavior/scope | Public example delegates max-tools default to installed OCR | `WQ-03` | Environment, docs, and integration contract tests | +| `REQ-140-B` | #140 acceptance 2-4 | Exact enriched/MCP preview accepts valid sentinel and handles known normalization without ambiguous/raw diagnostics | `WQ-03` | Parser negatives plus production-caller integration with controlled OCR peer | +| `REQ-140-C` | #140 scope/acceptance 5 | Qualification records behavioral numeric boundaries and effective-value ownership | `WQ-03` | Exact OCR 1.10.0 no-model probe and evidence-schema tests | +| `REQ-140-D` | #140 acceptance 6 | Current docs/tests avoid toolkit-release wording where OCR owns the contract | `WQ-03`, `WQ-04` | Documentation contract tests and full-text review | +| `REQ-REL` | Repository release contract | Deliver 0.8.3 through reviewed feature and release PRs with external reconciliation | `WQ-01`, `WQ-04`, `WQ-05` | Protected checks, registries, attestations, tag, receipt, issue/milestone closure | + +#### Explicit Non-Goals + +- Do not update OCR beyond exact 1.10.0 or change its checksum-pinned evidence + except for additive qualification facts required by #140. +- Do not add a toolkit prompt/context max-tokens alias, a new configuration + format, automatic discovery from help text, or a second compatibility service. +- Do not infer a maximum from absent diagnostics or treat OCR's documented + `min 10` help text as authoritative over observed behavior. +- Do not publish raw subprocess output, filesystem paths, provider/model data, + repository content, credentials, or exception strings in issues or receipts. +- Do not change DLP, receipt v5, severity, findings, posting transactions, + telemetry ownership, or automatic-approval eligibility. +- Do not run a local real LLM/model/provider review. A controlled deterministic + HTTP peer may exercise the OCR process boundary but is not model evidence. +- Do not reorganize tests or production modules beyond the cohesive owners + directly required by these two fixes. + +#### Constraints + +- Start from clean synchronized `main` at stable `v0.8.2`; use branch prefix + `codex/` and signed logical commits. +- Before each commit: focused tests, requirement/trust-boundary self-review, + complete staged diff review, and `git diff --check`. +- Run `scripts/quality.sh check` and `scripts/gitleaks.sh` once on the final + feature head. Hosted PR checks own the full OS/Python matrix and package build; + do not duplicate those gates locally without a new package boundary. +- Keep subprocess reads bounded, exact-schema status/evidence hostile on load, + and issue projection based only on closed enums and validated version/run IDs. +- Preserve the user-owned environment: isolated temporary `HOME` only, no OCR + config/credential writes, no global OCR installation change, and full cleanup. + +#### Inputs And Sources + +- GitHub issues #139 and #140, including their complete acceptance criteria. +- `.github/workflows/ocr-compatibility.yml`, `scripts/ocr_compat.py`, + `src/ocr_toolkit/review_runner.py`, current focused tests and public contracts. +- `docs/engineering/project_principles.md` persisted/atomic-state, external-format, + subprocess, outcome-consistency, and integration-proof boundaries. +- Official OCR `v1.10.0` source in `cmd/opencodereview/shared_flags.go`, + `cmd/opencodereview/shared.go`, and embedded `task_template.json`. +- Isolated local OCR 1.10.0 no-model preview evidence: omitted and `0` accepted; + negative rejected; `1..49` report normalization to `50`; `50+` accepted; help + says `min 10`; embedded template owns `MAX_TOOL_REQUEST_TIMES=100` and applies + CLI max-tools only when it raises that template value. + +#### User Decisions And Answers + +- Take both currently open issues into active work. +- Local execution against a real LLM/provider is explicitly waived because this + environment has no access; do not claim that evidence. +- Continue the established efficient workflow: logical commits, self-review, + one final complete local gate, protected hosted CI, then ordinary release. + +#### Completed Baseline State + +- `main` is clean at stable merge `6dae5b3821eb7aa22c8c8d8d9c17f869278340c7` + and tree `ba935a1f0a7f0e6e6c899120ecdc08e3822097c4`. +- Toolkit 0.8.2 and OCR 1.10.0 are published and reconciled; `.next-version` is + `0.8.3`; no active plan or open milestone existed before this activation. +- #139 reproduces three authority/recovery gaps added by the 0.8.2 failure path. +- #140 reproduces before any provider call because successful OCR preview stderr + currently accepts only the background-warning grammar. + +#### Current Work Queue + +| Work item | Status | Scope and commit boundary | +| --- | --- | --- | +| `WQ-01` | `in_progress` | Commit this plan, create `codex/v0.8.3-ocr-boundaries`, milestone `v0.8.3`, attach #139/#140, push once, and open a Draft PR with exact scope/non-claims | +| `WQ-02` | `pending` | Implement #139 outcome-authoritative selection, atomic/closed status recovery, manifest-independent status upsert, tests, and `139.bugfix.md` | +| `WQ-03` | `pending` | Implement #140 sentinel default, closed normalization handling, numeric boundary qualification/evidence, enriched/MCP integration coverage, docs, and `140.bugfix.md` | +| `WQ-04` | `pending` | Reconcile development/public docs and all requirements; run final quality, coverage, manifest, Towncrier, Gitleaks, and complete privacy/data-flow self-review; push completed history and resolve hosted findings | +| `WQ-05` | `pending` | Merge exact reviewed feature head, verify development publication, prepare protected `Release v0.8.3`, publish stable once authorized, and independently reconcile artifacts/provenance/tag/receipt/issues/milestone/clean main | + +#### Locked Decisions + +- Both issues belong to stable 0.8.3 and remain separate changelog entries. +- `OCR_MAX_TOOLS` remains an optional example-local variable but defaults to + sentinel `0`, whose meaning is exactly “inherit installed OCR template”. +- OCR's normalization diagnostic is a bounded installed-component fact. Its raw + text never crosses into result/posting/receipt; the toolkit reports only parsed + integers and distinguishes the reported normalization target from the actual + template-owned loop limit. +- A failed qualification status is authoritative whenever the qualification step + outcome is failure, even if a complete evidence file also exists. +- `upsert-issue --status` has no support-manifest dependency; evidence-driven + discovery, qualification, aggregation, and promotion retain manifest validation. +- OCR remains external and pinned to 1.10.0. No local model result substitutes + for hosted exact-binary or deterministic controlled-peer evidence. + +#### Verification + +- #139: focused `tests/test_ocr_compat.py` and workflow-source tests covering + retained evidence, late write failure, invalid manifest, closed schemas, + publication-before-red ordering, and blocked aggregation. +- #140: focused environment/integration/review-runner/compatibility tests covering + sentinel/default, normalization, duplicate/near-miss/Unicode/oversized stderr, + known/unknown success output, invalid non-zero diagnostics, effective-value + evidence, and the complete evidence/MCP production caller up to the model gate. +- Exact installed OCR 1.10.0: isolated no-real-model boundary probe only; preserve + command/exit/closed diagnostics/effective counts and remove its temporary HOME. +- Final feature head: `scripts/quality.sh check`, coverage floors, + `PYTHONPATH=src python scripts/ocr_compat.py validate`, Towncrier draft, + `scripts/gitleaks.sh`, `git diff --check`, and clean-tree confirmation. +- Hosted: all protected feature checks, one development TestPyPI publication and + exact artifact/provenance/install readback. Stable release follows `docs/release.md`. + +#### Latest Validation Results + +- 2026-08-25 reconnaissance: clean synchronized `main`; workflow audit reports + canonical owners and complete documentation indexes; exactly #139 and #140 are + open; both have no milestone. +- Isolated OCR 1.10.0 preview matrix reproduced #140 without LLM access and was + fully cleaned. Official tag source disproved the assumption that reported + normalization target `50` is necessarily the effective loop cap. +- No implementation, repository metadata, issue, milestone, branch, or PR write + preceded this plan materialization. + +#### Risks And Recovery + +- A stale evidence artifact may coexist with failure status. Recovery: bind issue + selection to step outcome and keep aggregate dependent on the restored red job. +- Status output itself may encounter unsafe/partial filesystem state. Recovery: + use a bounded same-directory atomic writer and fail the job without adopting a + partial status; workflow upload remains diagnostic, never promotion evidence. +- Future OCR diagnostics may change spelling or semantics. Recovery: exact closed + parsing and compatibility probes fail closed; promote a new grammar only with a + checksum-pinned OCR release and tests. +- The OCR help/runtime/template mismatch can mislead documentation. Recovery: + document sentinel ownership and observed behavior separately; never derive a + runtime contract from help text alone. +- External write or hosted CI failure: preserve local commits and Draft PR, record + exact run evidence, fix only the demonstrated boundary, and rerun its focused + gate before a new signed commit. + +#### Resume Point + +`WQ-01`: run the plan-fidelity check, create the feature branch, self-review and +commit this plan, then create/read back milestone `v0.8.3`, attach #139/#140, +push the planning commit, and open the Draft PR before production changes. + +#### Plan Fidelity Check + +- [x] Every requested issue acceptance criterion maps to a stable requirement and work item. +- [x] Release target, local-LLM waiver, privacy boundaries, and unchanged contracts are explicit. +- [x] Inputs distinguish current repository behavior, official OCR source, and observed probes. +- [x] Rejected scope and overengineering boundaries are explicit. +- [x] Each logical commit has focused verification and self-review gates. +- [x] External writes, hosted delivery, recovery, and exact resume state are represented. + +#### Reconciliation Check + +- [x] `PLANS.md` was inactive and the worktree was clean at activation. +- [x] #139 and #140 are the complete open issue set and do not conflict with backlog/roadmap scope. +- [x] Stable 0.8.2 closure remains historical and is not rewritten. +- [x] Target 0.8.3 matches `.next-version` and no competing milestone exists. + +#### Closure Gate + +- [ ] All requirements and work items are terminal with current validation evidence. +- [ ] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. +- [ ] Exact feature head is green locally and in protected hosted checks with resolved review threads. +- [ ] Development and stable artifacts are independently reconciled and issues/milestone are closed from receipts. +- [ ] `scripts/plan_lifecycle.py check` passes and the plan is archived through the release-owned transition. + +#### Post-Close Delivery + +- Feature implementation closes through a protected squash merge only after exact + reviewed-head checks. Stable publication remains a separate protected release PR + and immutable external reconciliation; readiness is not delivery. +- #139/#140 and milestone `v0.8.3` remain open until stable receipt publication. +- Final handoff must repeat that no local real-LLM/provider qualification was run + or claimed, while identifying the exact deterministic OCR boundary evidence. + +#### Handoff Notes + +- Start at `WQ-01`; do not implement from an uncommitted or compressed substitute. +- The source-level max-tools mismatch is material: reported normalization `50` + and embedded template default `100` are distinct facts. Preserve that distinction + in code, evidence, docs, changelog, issue updates, and future OCR upgrades. From 9a6375dc2d1f8c3460863598e6637551567088fb Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:06:05 +0200 Subject: [PATCH 2/8] Make OCR failure publication authoritative --- .github/workflows/ocr-compatibility.yml | 11 +- PLANS.md | 47 ++++-- changelog.d/139.bugfix.md | 1 + docs/compatibility.md | 2 + scripts/ocr_compat.py | 104 ++++++++++--- tests/test_ocr_compat.py | 195 ++++++++++++++++++++++++ tests/test_operations_docs.py | 4 + 7 files changed, 323 insertions(+), 41 deletions(-) create mode 100644 changelog.d/139.bugfix.md diff --git a/.github/workflows/ocr-compatibility.yml b/.github/workflows/ocr-compatibility.yml index 254ef47..4ed29ef 100644 --- a/.github/workflows/ocr-compatibility.yml +++ b/.github/workflows/ocr-compatibility.yml @@ -108,13 +108,16 @@ jobs: if: ${{ !cancelled() }} env: GITHUB_TOKEN: ${{ github.token }} + QUALIFICATION_OUTCOME: ${{ steps.qualify.outcome }} REPOSITORY: ${{ github.repository }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | - input=(--status /tmp/ocr-compat/status.json) - if [ -f /tmp/ocr-compat/evidence.json ]; then - input=(--evidence /tmp/ocr-compat/evidence.json) - fi + case "${QUALIFICATION_OUTCOME}" in + success) input=(--evidence /tmp/ocr-compat/evidence.json) ;; + failure) input=(--status /tmp/ocr-compat/status.json) ;; + *) echo "unsupported qualification outcome" >&2; exit 1 ;; + esac + test -f "${input[1]}" PYTHONPATH=src python scripts/ocr_compat.py upsert-issue \ "${input[@]}" \ --repository "${REPOSITORY}" \ diff --git a/PLANS.md b/PLANS.md index 3542563..55cacec 100644 --- a/PLANS.md +++ b/PLANS.md @@ -9,7 +9,8 @@ Use this file for active or blocked repository work. Update it before implementa Status: `active` Owner: Codex Plan Origin: `direct_execution` -Release classification: `release-required` +Release classification: `release-deferred` (the product changes require stable +`0.8.3`, but feature merge and publication are explicitly deferred) Target stable version: `0.8.3` Last Updated: 2026-08-25 @@ -109,7 +110,9 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. - Local execution against a real LLM/provider is explicitly waived because this environment has no access; do not claim that evidence. - Continue the established efficient workflow: logical commits, self-review, - one final complete local gate, protected hosted CI, then ordinary release. + one final complete local gate, and protected hosted CI. +- After the final push, keep PR #141 in Draft. Do not merge the feature branch, + publish development/stable artifacts, create a release PR, or release 0.8.3. #### Completed Baseline State @@ -125,11 +128,11 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. | Work item | Status | Scope and commit boundary | | --- | --- | --- | -| `WQ-01` | `in_progress` | Commit this plan, create `codex/v0.8.3-ocr-boundaries`, milestone `v0.8.3`, attach #139/#140, push once, and open a Draft PR with exact scope/non-claims | -| `WQ-02` | `pending` | Implement #139 outcome-authoritative selection, atomic/closed status recovery, manifest-independent status upsert, tests, and `139.bugfix.md` | -| `WQ-03` | `pending` | Implement #140 sentinel default, closed normalization handling, numeric boundary qualification/evidence, enriched/MCP integration coverage, docs, and `140.bugfix.md` | +| `WQ-01` | `done` | Signed planning commit `85b3097`; branch `codex/v0.8.3-ocr-boundaries`; milestone `v0.8.3` #6 with #139/#140; planning push; Draft PR #141 with exact scope/non-claims | +| `WQ-02` | `done` | #139: outcome-authoritative workflow selection; portable atomic evidence/status/issue-body handoffs; closed late-write recovery; manifest-independent status upsert; regression tests; public contract and `139.bugfix.md` | +| `WQ-03` | `in_progress` | Implement #140 sentinel default, closed normalization handling, numeric boundary qualification/evidence, enriched/MCP integration coverage, docs, and `140.bugfix.md` | | `WQ-04` | `pending` | Reconcile development/public docs and all requirements; run final quality, coverage, manifest, Towncrier, Gitleaks, and complete privacy/data-flow self-review; push completed history and resolve hosted findings | -| `WQ-05` | `pending` | Merge exact reviewed feature head, verify development publication, prepare protected `Release v0.8.3`, publish stable once authorized, and independently reconcile artifacts/provenance/tag/receipt/issues/milestone/clean main | +| `WQ-05` | `out_of_scope` | Owner-deferred delivery: keep PR #141 Draft after final push; do not merge, publish TestPyPI/PyPI, prepare a release PR, tag, close issues, or close the milestone in this run | #### Locked Decisions @@ -161,8 +164,9 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. - Final feature head: `scripts/quality.sh check`, coverage floors, `PYTHONPATH=src python scripts/ocr_compat.py validate`, Towncrier draft, `scripts/gitleaks.sh`, `git diff --check`, and clean-tree confirmation. -- Hosted: all protected feature checks, one development TestPyPI publication and - exact artifact/provenance/install readback. Stable release follows `docs/release.md`. +- Hosted: all protected Draft feature checks on the exact final head. Development + publication, stable release, registry/provenance/install readback, and closure + remain deferred under `docs/release.md`. #### Latest Validation Results @@ -174,6 +178,14 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. normalization target `50` is necessarily the effective loop cap. - No implementation, repository metadata, issue, milestone, branch, or PR write preceded this plan materialization. +- Coordination completed after the signed plan commit: milestone `v0.8.3` #6, + #139/#140 assignment, planning head push, and Draft PR #141. The owner then + explicitly deferred merge and release; no publication belongs to this run. +- #139 focused validation: 95 compatibility/workflow tests pass; Ruff and + `git diff --check` pass. Self-review confirms the workflow selects output from + `steps.qualify.outcome`, all file handoffs preserve the old baseline until an + atomic replace, status recovery consumes only its closed schema, raw details + remain private, and the restored red qualification blocks aggregation. #### Risks And Recovery @@ -194,9 +206,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. #### Resume Point -`WQ-01`: run the plan-fidelity check, create the feature branch, self-review and -commit this plan, then create/read back milestone `v0.8.3`, attach #139/#140, -push the planning commit, and open the Draft PR before production changes. +`WQ-03`: add exact closed parsing for the installed OCR max-tools normalization, +move the example default to sentinel `0`, extend numeric qualification evidence, +and prove the enriched/MCP production caller reaches the model boundary without +using a real model. #### Plan Fidelity Check @@ -219,14 +232,16 @@ push the planning commit, and open the Draft PR before production changes. - [ ] All requirements and work items are terminal with current validation evidence. - [ ] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. - [ ] Exact feature head is green locally and in protected hosted checks with resolved review threads. -- [ ] Development and stable artifacts are independently reconciled and issues/milestone are closed from receipts. -- [ ] `scripts/plan_lifecycle.py check` passes and the plan is archived through the release-owned transition. +- [ ] PR #141 remains Draft at the exact pushed head; #139/#140 and milestone `v0.8.3` remain open. +- [ ] `scripts/plan_lifecycle.py check` passes with an exact external-review/release resume point; the active plan is not archived before deferred delivery. #### Post-Close Delivery -- Feature implementation closes through a protected squash merge only after exact - reviewed-head checks. Stable publication remains a separate protected release PR - and immutable external reconciliation; readiness is not delivery. +- This run ends at a pushed green Draft PR. Feature implementation does not close + through merge, and no TestPyPI/PyPI or stable publication is authorized. +- A later owner-authorized continuation must review the exact Draft head, preserve + or amend the plan from current state, then use the protected feature/release + lifecycle. Readiness is not delivery. - #139/#140 and milestone `v0.8.3` remain open until stable receipt publication. - Final handoff must repeat that no local real-LLM/provider qualification was run or claimed, while identifying the exact deterministic OCR boundary evidence. diff --git a/changelog.d/139.bugfix.md b/changelog.d/139.bugfix.md new file mode 100644 index 0000000..3c637d2 --- /dev/null +++ b/changelog.d/139.bugfix.md @@ -0,0 +1 @@ +Made OCR compatibility failure publication authoritative and recoverable. The workflow now selects closed status or evidence from the actual qualification outcome, atomically replaces output handoffs, and can publish a failure status without loading unrelated support metadata. It still publishes the bounded issue and diagnostic artifact before returning a red job, while aggregate promotion remains blocked and raw diagnostics remain private. diff --git a/docs/compatibility.md b/docs/compatibility.md index 32494f8..cbe50a3 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -23,6 +23,8 @@ Classify a pin-only promotion and its machine or human qualification evidence as Each OCR version owns one stable HTML marker and one qualification issue. The workflow performs a single upsert through bounded direct issue listing rather than GitHub's eventually consistent search index. Historical issues closed with the `duplicate` label remain as incident evidence but do not compete for canonical identity. Any other duplicate state fails closed instead of creating another issue; after operators select and reconcile the canonical issue, reruns update it in place. +The qualification step writes evidence, public issue text, and its closed status through same-directory atomic replacements. The step outcome is authoritative: a successful step publishes validated evidence, while a failed step publishes only the closed failure status even if retained evidence exists from work completed before the failure. Status-based recovery validates no support manifest because it does not consume support metadata. The workflow publishes the issue and diagnostic artifact before restoring the failed job outcome; dependent aggregation and promotion jobs therefore remain blocked. Raw exceptions and subprocess output stay in the private job log and never enter the status or issue projection. + Evidence records optional capabilities separately from required compatibility. OCR 1.8.7 and later expose per-run provider/model overrides and additive `llm` result identity; older tested releases remain valid without these fields. OCR 1.9.4 additionally prints its session ID only in human-readable terminal summaries; JSON output and the toolkit-consumed result contract are unchanged. OCR 1.9.5 makes the existing aggregate budget flag visible in `scan` JSON, while the toolkit remains on `review` and separately qualifies that review's partial manifest, completed findings, and budget-attributed coverage. OCR 1.9.8 adds Bedrock as an upstream provider, moves human-audience JSON/SARIF progress to stderr, and changes only upstream skill guidance to consume native severity. The toolkit configures none of the Bedrock credential boundary, invokes review with agent audience, and already consumes the same tolerant structured severity/category result fields. Session identity and scan-only output remain OCR-owned signals rather than toolkit metrics. Profile or telemetry work must require the capability it consumes rather than treating the recommended version alone as proof. ### OCR 1.9.9 — inherited predecessor diff --git a/scripts/ocr_compat.py b/scripts/ocr_compat.py index eb2e885..f3e1d2f 100644 --- a/scripts/ocr_compat.py +++ b/scripts/ocr_compat.py @@ -91,13 +91,17 @@ "artifact-verification-failed", "contract-probe-failed", "evidence-write-failed", + "issue-body-write-failed", + "status-write-failed", "compatible", } QUALIFICATION_FAILURE_REASONS = { "metadata": frozenset({"metadata-invalid", "metadata-request-failed"}), "artifact": frozenset({"artifact-verification-failed"}), "contracts": frozenset({"contract-probe-failed"}), - "evidence": frozenset({"evidence-write-failed"}), + "evidence": frozenset( + {"evidence-write-failed", "issue-body-write-failed", "status-write-failed"} + ), } T = TypeVar("T") @@ -174,6 +178,38 @@ def canonical_json(value: Any) -> bytes: return (json.dumps(value, ensure_ascii=False, indent=2, sort_keys=True) + "\n").encode() +def write_atomic_bytes(path: Path, payload: bytes, *, label: str) -> None: + """Commit one output through a private same-directory temporary file.""" + + temporary: Path | None = None + try: + path.parent.mkdir(parents=True, exist_ok=True) + descriptor, temporary_name = tempfile.mkstemp(prefix=f".{path.name}.", dir=path.parent) + temporary = Path(temporary_name) + with os.fdopen(descriptor, "wb") as output: + written = output.write(payload) + if written != len(payload): + raise OSError("short output write") + output.flush() + os.fsync(output.fileno()) + os.replace(temporary, path) + temporary = None + if os.name != "nt": + directory_descriptor = os.open(path.parent, os.O_RDONLY) + try: + os.fsync(directory_descriptor) + finally: + os.close(directory_descriptor) + except OSError as exc: + raise CompatibilityError(f"cannot write {label}") from exc + finally: + if temporary is not None: + try: + temporary.unlink(missing_ok=True) + except OSError: + pass + + def load_json(path: Path) -> dict[str, Any]: """Load one JSON object from disk.""" @@ -1671,11 +1707,7 @@ def artifacts() -> dict[str, Path]: } def write_evidence() -> None: - try: - output.parent.mkdir(parents=True, exist_ok=True) - output.write_bytes(canonical_json(evidence)) - except OSError as exc: - raise CompatibilityError("cannot write compatibility evidence") from exc + write_atomic_bytes(output, canonical_json(evidence), label="compatibility evidence") _qualification_stage("evidence", "evidence-write-failed", write_evidence) return evidence @@ -2201,7 +2233,27 @@ def main(argv: list[str] | None = None) -> int: upsert_issue.add_argument("--output-number", type=Path, required=True) args = parser.parse_args(argv) manifest: dict[str, Any] | None = None + + def finish_issue_upsert( + *, evidence: dict[str, Any] | None = None, status: dict[str, Any] | None = None + ) -> int: + issue_number = upsert_qualification_issue( + repository=args.repository, + evidence=evidence, + status=status, + run_url=args.run_url, + ) + write_atomic_bytes( + args.output_number, + f"{issue_number}\n".encode(), + label="qualification issue number", + ) + print(f"qualification issue: #{issue_number}") + return 0 + try: + if args.command == "upsert-issue" and args.status is not None: + return finish_issue_upsert(status=load_json(args.status)) manifest = load_json(args.manifest) validate_manifest(manifest, args.manifest.resolve().parents[1]) if args.command == "validate": @@ -2240,17 +2292,8 @@ def main(argv: list[str] | None = None) -> int: print(path.resolve().relative_to(ROOT)) return 0 if args.command == "upsert-issue": - evidence = load_json(args.evidence) if args.evidence is not None else None - status = load_json(args.status) if args.status is not None else None - issue_number = upsert_qualification_issue( - repository=args.repository, - evidence=evidence, - status=status, - run_url=args.run_url, - ) - args.output_number.write_text(f"{issue_number}\n", encoding="utf-8") - print(f"qualification issue: #{issue_number}") - return 0 + assert args.evidence is not None + return finish_issue_upsert(evidence=load_json(args.evidence)) if args.command == "probe-local": version = args.version.removeprefix("v") _version(version) @@ -2279,7 +2322,15 @@ def main(argv: list[str] | None = None) -> int: tested_baseline_version=args.tested_baseline_version, ) if args.issue_body is not None: - args.issue_body.write_text(render_issue(evidence), encoding="utf-8") + _qualification_stage( + "evidence", + "issue-body-write-failed", + lambda: write_atomic_bytes( + args.issue_body, + render_issue(evidence).encode(), + label="qualification issue body", + ), + ) if args.status_output is not None: status = qualification_status( tag=args.tag, @@ -2290,7 +2341,15 @@ def main(argv: list[str] | None = None) -> int: phase="complete", reason="compatible", ) - args.status_output.write_bytes(canonical_json(status)) + _qualification_stage( + "evidence", + "status-write-failed", + lambda: write_atomic_bytes( + args.status_output, + canonical_json(status), + label="compatibility status", + ), + ) print(f"qualified OCR {evidence['version']}: {evidence['classification']}") return 0 except CompatibilityError as exc: @@ -2311,8 +2370,11 @@ def main(argv: list[str] | None = None) -> int: phase=phase, reason=reason, ) - args.status_output.parent.mkdir(parents=True, exist_ok=True) - args.status_output.write_bytes(canonical_json(status)) + write_atomic_bytes( + args.status_output, + canonical_json(status), + label="compatibility status", + ) except (CompatibilityError, OSError) as status_exc: print(f"OCR compatibility status write failed: {status_exc}", file=sys.stderr) print(f"OCR compatibility qualification failed: {exc}", file=sys.stderr) diff --git a/tests/test_ocr_compat.py b/tests/test_ocr_compat.py index dd17082..142e6a1 100644 --- a/tests/test_ocr_compat.py +++ b/tests/test_ocr_compat.py @@ -759,6 +759,201 @@ def test_qualify_cli_retains_status_when_manifest_validation_fails(tmp_path: Pat assert status["reason"] == "metadata-invalid" +def test_qualify_cli_replaces_retained_evidence_with_failed_status_after_late_write( + tmp_path: Path, +) -> None: + """A late public-output failure remains authoritative after evidence exists.""" + + module = load_script() + evidence_output = tmp_path / "evidence.json" + status_output = tmp_path / "status.json" + blocked_parent = tmp_path / "blocked-parent" + blocked_parent.write_text("not a directory\n", encoding="utf-8") + evidence = json.loads( + (PROJECT_ROOT / "compatibility" / "evidence" / "ocr-1.10.0.json").read_text( + encoding="utf-8" + ) + ) + + def retain_evidence( + _release: dict[str, Any], + _manifest: dict[str, Any], + output: Path, + **_kwargs: object, + ) -> dict[str, Any]: + output.write_bytes(module.canonical_json(evidence)) + return evidence + + with ( + patched_attr(module, "_request_json", lambda _url: release("1.10.0")), + patched_attr(module, "qualify_release", retain_evidence), + ): + result = module.main( + [ + "--manifest", + str(MANIFEST), + "qualify", + "--tag", + "v1.10.0", + "--comparison-version", + "1.9.10", + "--tested-baseline-version", + "1.9.10", + "--output", + str(evidence_output), + "--issue-body", + str(blocked_parent / "issue.md"), + "--status-output", + str(status_output), + ] + ) + + status = json.loads(status_output.read_text(encoding="utf-8")) + assert result == 1 + assert evidence_output.exists() + assert status["phase"] == "evidence" + assert status["reason"] == "issue-body-write-failed" + assert status["result"] == "failed" + assert str(blocked_parent) not in status_output.read_text(encoding="utf-8") + + +def test_atomic_output_preserves_baseline_and_cleans_temporary_on_replace_failure( + tmp_path: Path, +) -> None: + """A failed commit cannot replace the baseline or retain private temporary data.""" + + module = load_script() + output = tmp_path / "status.json" + output.write_bytes(b"baseline\n") + + def fail_replace(_source: object, _target: object) -> None: + raise OSError("injected replace failure") + + with patched_attr(module.os, "replace", fail_replace): + with pytest.raises(module.CompatibilityError, match="cannot write compatibility status"): + module.write_atomic_bytes(output, b"replacement\n", label="compatibility status") + + assert output.read_bytes() == b"baseline\n" + assert list(tmp_path.glob(".status.json.*")) == [] + + +def test_qualify_cli_recovers_when_success_status_commit_fails_once(tmp_path: Path) -> None: + """A failed success-status commit is replaced by its closed failure status.""" + + module = load_script() + evidence_output = tmp_path / "evidence.json" + status_output = tmp_path / "status.json" + evidence = json.loads( + (PROJECT_ROOT / "compatibility" / "evidence" / "ocr-1.10.0.json").read_text( + encoding="utf-8" + ) + ) + real_write = module.write_atomic_bytes + status_writes = 0 + + def retain_evidence( + _release: dict[str, Any], + _manifest: dict[str, Any], + output: Path, + **_kwargs: object, + ) -> dict[str, Any]: + output.write_bytes(module.canonical_json(evidence)) + return evidence + + def fail_first_status_write(path: Path, payload: bytes, *, label: str) -> None: + nonlocal status_writes + if path == status_output: + status_writes += 1 + if status_writes == 1: + raise module.CompatibilityError("cannot write compatibility status") + real_write(path, payload, label=label) + + with ( + patched_attr(module, "_request_json", lambda _url: release("1.10.0")), + patched_attr(module, "qualify_release", retain_evidence), + patched_attr(module, "write_atomic_bytes", fail_first_status_write), + ): + result = module.main( + [ + "--manifest", + str(MANIFEST), + "qualify", + "--tag", + "v1.10.0", + "--comparison-version", + "1.9.10", + "--tested-baseline-version", + "1.9.10", + "--output", + str(evidence_output), + "--status-output", + str(status_output), + ] + ) + + status = json.loads(status_output.read_text(encoding="utf-8")) + assert result == 1 + assert status_writes == 2 + assert status["phase"] == "evidence" + assert status["reason"] == "status-write-failed" + assert status["result"] == "failed" + + +def test_status_upsert_does_not_require_a_valid_support_manifest(tmp_path: Path) -> None: + """Publish a closed recovery status without reopening invalid support metadata.""" + + module = load_script() + invalid_manifest = tmp_path / "invalid-manifest.json" + invalid_manifest.write_text("{}\n", encoding="utf-8") + status_path = tmp_path / "status.json" + status_path.write_bytes( + module.canonical_json( + module.qualification_status( + tag="v1.10.0", + comparison_version="1.9.10", + tested_baseline_version="1.9.10", + result="failed", + phase="metadata", + reason="metadata-invalid", + ) + ) + ) + issue_number = tmp_path / "issue-number.txt" + calls: list[tuple[str, str, dict[str, Any] | None]] = [] + + def request( + url: str, *, method: str = "GET", payload: dict[str, Any] | None = None + ) -> dict[str, Any]: + calls.append((url, method, payload)) + return {"number": 140} + + with ( + patched_attr(module, "find_qualification_issue", lambda _repo, _marker: 140), + patched_attr(module, "_issue_api_request", request), + ): + result = module.main( + [ + "--manifest", + str(invalid_manifest), + "upsert-issue", + "--status", + str(status_path), + "--repository", + "synthetic/repository", + "--run-url", + "https://github.com/synthetic/repository/actions/runs/123", + "--output-number", + str(issue_number), + ] + ) + + assert result == 0 + assert issue_number.read_text(encoding="utf-8") == "140\n" + assert calls[0][1] == "PATCH" + assert calls[0][2] is not None + assert "metadata-invalid" in calls[0][2]["body"] + + def test_qualification_requires_exact_supported_asset_matrix() -> None: module = load_script() candidate = release("1.7.18") diff --git a/tests/test_operations_docs.py b/tests/test_operations_docs.py index 31b659c..44ca8d9 100644 --- a/tests/test_operations_docs.py +++ b/tests/test_operations_docs.py @@ -459,6 +459,10 @@ def test_ocr_compatibility_workflow_is_bounded_and_protected() -> None: assert workflow.count("upsert-issue") == 1 assert "continue-on-error: true" in workflow assert "--status-output /tmp/ocr-compat/status.json" in workflow + assert "QUALIFICATION_OUTCOME: ${{ steps.qualify.outcome }}" in workflow + assert 'case "${QUALIFICATION_OUTCOME}" in' in workflow + assert "success) input=(--evidence /tmp/ocr-compat/evidence.json)" in workflow + assert "failure) input=(--status /tmp/ocr-compat/status.json)" in workflow assert "if: ${{ !cancelled() }}" in workflow assert "if: ${{ always() && !cancelled() }}" in workflow assert "if: steps.qualify.outcome == 'failure'" in workflow From 743d8fa38acc0671c9ec8afc6aec481db8cbba0a Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:21:56 +0200 Subject: [PATCH 3/8] Qualify OCR numeric CLI boundaries --- PLANS.md | 21 +- changelog.d/140.bugfix.md | 1 + compatibility/evidence/ocr-1.10.0.json | 73 ++++ compatibility/ocr-support.json | 4 +- docs/compatibility.md | 4 +- docs/configuration.md | 12 +- docs/development.md | 6 + docs/engineering/test_evidence_matrix.md | 3 +- docs/gitlab.md | 2 +- docs/operations.md | 2 +- examples/gitlab/README.md | 2 +- examples/gitlab/ocr-review.gitlab-ci.yml | 6 +- scripts/ocr_compat.py | 433 +++++++++++++++++++++-- src/ocr_toolkit/review_runner.py | 47 ++- tests/test_environment_contract.py | 6 +- tests/test_integration_contracts.py | 4 +- tests/test_ocr_compat.py | 64 ++++ tests/test_operations_docs.py | 17 + tests/test_review_runner.py | 82 ++++- 19 files changed, 710 insertions(+), 79 deletions(-) create mode 100644 changelog.d/140.bugfix.md diff --git a/PLANS.md b/PLANS.md index 55cacec..7085ce2 100644 --- a/PLANS.md +++ b/PLANS.md @@ -130,8 +130,8 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. | --- | --- | --- | | `WQ-01` | `done` | Signed planning commit `85b3097`; branch `codex/v0.8.3-ocr-boundaries`; milestone `v0.8.3` #6 with #139/#140; planning push; Draft PR #141 with exact scope/non-claims | | `WQ-02` | `done` | #139: outcome-authoritative workflow selection; portable atomic evidence/status/issue-body handoffs; closed late-write recovery; manifest-independent status upsert; regression tests; public contract and `139.bugfix.md` | -| `WQ-03` | `in_progress` | Implement #140 sentinel default, closed normalization handling, numeric boundary qualification/evidence, enriched/MCP integration coverage, docs, and `140.bugfix.md` | -| `WQ-04` | `pending` | Reconcile development/public docs and all requirements; run final quality, coverage, manifest, Towncrier, Gitleaks, and complete privacy/data-flow self-review; push completed history and resolve hosted findings | +| `WQ-03` | `done` | #140: example sentinel `0`; exact normalization parser and operator-only notice; behavioral numeric/effective-loop qualification; updated exact OCR evidence/hash; full enriched/MCP preview regression; public/development contracts and `140.bugfix.md` | +| `WQ-04` | `in_progress` | Reconcile development/public docs and all requirements; run final quality, coverage, manifest, Towncrier, Gitleaks, and complete privacy/data-flow self-review; push completed history and resolve hosted findings | | `WQ-05` | `out_of_scope` | Owner-deferred delivery: keep PR #141 Draft after final push; do not merge, publish TestPyPI/PyPI, prepare a release PR, tag, close issues, or close the milestone in this run | #### Locked Decisions @@ -186,6 +186,15 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. `steps.qualify.outcome`, all file handoffs preserve the old baseline until an atomic replace, status recovery consumes only its closed schema, raw details remain private, and the restored red qualification blocks aggregation. +- #140 focused validation: 212 runtime/compatibility/environment/integration/ + documentation tests pass with Ruff, manifest validation, Towncrier draft, and + `git diff --check`. Exact installed OCR 1.10.0 Darwin arm64 no-model probes + pass in isolated temporary homes: CLI minimum/normalization target `50`, + effective template value `100`, and explicit `101` producing 101 rounds. + The enriched production caller performs collection, store/bootstrap, MCP + registration/self-query, and exact preview before its controlled model + boundary; the parsed normalization becomes only a CI notice and leaves result + warnings, DLP, receipt, posting, telemetry, and approval inputs unchanged. #### Risks And Recovery @@ -206,10 +215,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. #### Resume Point -`WQ-03`: add exact closed parsing for the installed OCR max-tools normalization, -move the example default to sentinel `0`, extend numeric qualification evidence, -and prove the enriched/MCP production caller reaches the model boundary without -using a real model. +`WQ-04`: run the one complete local quality/security/manifest/changelog gate, +perform the final requirement/privacy/data-flow diff review, update this plan and +Draft PR handoff to exact-head truth, then make the single final push and resolve +only evidence-driven hosted failures without releasing. #### Plan Fidelity Check diff --git a/changelog.d/140.bugfix.md b/changelog.d/140.bugfix.md new file mode 100644 index 0000000..64c0f0b --- /dev/null +++ b/changelog.d/140.bugfix.md @@ -0,0 +1 @@ +Fixed the GitLab example's stale positive `OCR_MAX_TOOLS` default by delegating with sentinel `0`. Added exact parsing for OCR's supported max-tools normalization notice and behavior-based compatibility evidence for numeric CLI boundaries, including the effective template-owned tool-loop value. The notice remains operator-only: raw OCR stderr, findings, result warnings, receipts, DLP, telemetry, lifecycle commands, and automatic-approval semantics are unchanged. diff --git a/compatibility/evidence/ocr-1.10.0.json b/compatibility/evidence/ocr-1.10.0.json index 8f6735e..ac7c0dc 100644 --- a/compatibility/evidence/ocr-1.10.0.json +++ b/compatibility/evidence/ocr-1.10.0.json @@ -54,6 +54,78 @@ "result": "passed", "wire_field": "max_completion_tokens" }, + "numeric_cli_probe": { + "max_tokens_budget": { + "cases": { + "invalid_below": { + "effective": null, + "input": -1, + "outcome": "rejected" + }, + "minimum": { + "effective": 1, + "input": 1, + "outcome": "accepted" + }, + "omitted": { + "effective": "unlimited", + "input": null, + "outcome": "accepted" + }, + "representative": { + "effective": 30000, + "input": 30000, + "outcome": "accepted" + }, + "sentinel": { + "effective": "unlimited", + "input": 0, + "outcome": "accepted" + } + }, + "maximum": null, + "owner": "ocr-cli" + }, + "max_tools": { + "cases": { + "invalid_below": { + "effective": null, + "input": -1, + "outcome": "rejected" + }, + "minimum": { + "effective": 100, + "input": 50, + "outcome": "accepted" + }, + "minimum_minus_one": { + "effective": 100, + "input": 49, + "outcome": "normalized", + "reported_normalization": 50 + }, + "omitted": { + "effective": 100, + "input": null, + "outcome": "accepted" + }, + "representative": { + "effective": 101, + "input": 101, + "outcome": "accepted" + }, + "sentinel": { + "effective": 100, + "input": 0, + "outcome": "accepted" + } + }, + "maximum": null, + "owner": "ocr-template-or-higher-cli", + "reported_minimum": 50 + }, + "result": "passed" + }, "optional_capabilities": [ "llm_result_identity", "per_run_model_override", @@ -74,6 +146,7 @@ "--format", "--from", "--max-tokens-budget", + "--max-tools", "--preview", "--rule", "--to" diff --git a/compatibility/ocr-support.json b/compatibility/ocr-support.json index a50ccb6..4b009c4 100644 --- a/compatibility/ocr-support.json +++ b/compatibility/ocr-support.json @@ -1181,8 +1181,8 @@ "semantic_grouping" ], "evidence": "compatibility/evidence/ocr-1.10.0.json", - "evidence_sha256": "50f9668b68dbcc4dc83f379a55e81aa58bc7188d1be88d44abb32a509361e761", - "human_conclusion": "Reviewed OCR 1.9.10...1.10.0 source and hosted Linux wire evidence: semantic grouping, path-aware comments, default medium two-round review, private additive group metadata, output-file capability, git diagnostics, and the 16384 completion cap are compatible with toolkit boundaries; toolkit retains exclusive result-path ownership and explicit 4096 cap support.", + "evidence_sha256": "55fe857b240df00d69a7e70c8ac2d0a0bb2c12640b72e6aa9f270fa87063f8eb", + "human_conclusion": "Reviewed OCR 1.9.10...1.10.0 source, hosted Linux wire evidence, and exact Darwin numeric-boundary evidence: semantic grouping, path-aware comments, default medium two-round review, private additive group metadata, output-file capability, git diagnostics, the 16384 completion cap, and template-owned max-tools behavior are compatible with toolkit boundaries; toolkit retains exclusive result-path ownership, explicit 4096 cap support, and closed normalization projection.", "published_at": "2026-08-25T05:45:43Z", "release_url": "https://github.com/alibaba/open-code-review/releases/tag/v1.10.0", "status": "tested", diff --git a/docs/compatibility.md b/docs/compatibility.md index cbe50a3..8fd65cd 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -6,7 +6,7 @@ The versioned support contract lives in [`compatibility/ocr-support.json`](../co The scheduled **OCR compatibility** workflow discovers stable upstream releases newer than the manifest monitoring floor. Drafts, prereleases, non-semantic tags, unexpected asset sets, oversized metadata or downloads, redirects outside the reviewed GitHub origins, and checksum disagreement fail closed. Every binary digest must agree with both GitHub release metadata and the upstream `sha256sum.txt`. -Candidate execution uses the verified Linux amd64 binary on an Ubuntu runner. The harness checks the reported version, the CLI flags consumed by the GitLab integration, range preview behavior, an actual JSON review through a deterministic local gateway, an aggregate-budget review that must preserve completed findings and emit budget-attributed partial coverage, and the additive JSON fields consumed by posting. For OCR 1.9.0 and later it also requires JSON preview without a session-store side effect and proves that additive comment `thinking` is accepted but not published to GitLab. Upstream source review separately verifies how OCR derives that field; the toolkit probe does not claim to reproduce a provider's private reasoning channel. Evidence permits unknown new fields but requires the fields the toolkit reads. Legacy result statuses and the versioned `ocr.run-manifest/v1` outcome are normalized through one shared toolkit contract; manifest coverage sets, failure classifications, terminal state, and budget attribution must agree before a result can be published. +Candidate execution uses the verified Linux amd64 binary on an Ubuntu runner. The harness checks the reported version, the CLI flags consumed by the GitLab integration, range preview behavior, an actual JSON review through a deterministic local gateway, an aggregate-budget review that must preserve completed findings and emit budget-attributed partial coverage, and the additive JSON fields consumed by posting. Toolkit-managed numeric OCR options are also exercised at omitted/default, sentinel, invalid-below-boundary, minimum minus one, minimum, representative, and maximum edges when bounded. The evidence records closed outcomes, recognized diagnostics, normalization, ownership, and effective values observed through real loop behavior; CLI help text is not accepted as runtime evidence. For OCR 1.9.0 and later the harness also requires JSON preview without a session-store side effect and proves that additive comment `thinking` is accepted but not published to GitLab. Upstream source review separately verifies how OCR derives that field; the toolkit probe does not claim to reproduce a provider's private reasoning channel. Evidence permits unknown new fields but requires the fields the toolkit reads. Legacy result statuses and the versioned `ocr.run-manifest/v1` outcome are normalized through one shared toolkit contract; manifest coverage sets, failure classifications, terminal state, and budget attribution must agree before a result can be published. Built-in MCP qualification follows the protocol revisions supported by the recommended OCR release's exact MCP SDK. The current recommended release uses Go MCP SDK v1.6.1 and initiates revision `2025-11-25`; the evidence server also retains `2025-06-18`, `2025-03-26`, and `2024-11-05` for qualified older clients. For an unknown client revision the server follows MCP negotiation semantics by returning its current supported revision, leaving acceptance or termination to the client. Qualification exercises initialize, the initialized notification, ping, tool discovery, and bounded summary/list/get calls through the exact SDK rather than relying only on handcrafted JSON-RPC fixtures. @@ -45,6 +45,8 @@ Group labels are model-produced and group keys contain sorted changed paths. Add OCR 1.10.0 also adds `--output`; `ocr-ci review` rejects `--output`, `--output=...`, `-o`, and attached short forms before preview because the toolkit retains sole ownership of the private result descriptor and atomic validation/cleanup. Improved upstream git errors remain private OCR stderr and do not widen GitLab diagnostics. With no toolkit completion-cap override, the qualified OpenAI wire request now uses `max_completion_tokens=16384`, down from 58,888 in OCR 1.9.10. `OCR_LLM_MAX_COMPLETION_TOKENS=4096` remains an explicit operator workaround for gateways that reserve spending against the requested output cap. +Its max-tools CLI reports a minimum and normalization target of `50`, while the embedded review template owns `100` rounds and the CLI only raises that value. Exact-binary behavioral probes therefore observe effective `100` for omitted, sentinel `0`, `49`, and `50`, and effective `101` for explicit `101`; `--max-tools 49` still prints the `50` normalization target. Toolkit preview accepts only that exact diagnostic shape and emits a closed operator notice. It does not copy raw stderr into the OCR result or derive finding, receipt, DLP, telemetry, lifecycle, or approval authority from the notice. The public example now passes sentinel `0` instead of a stale release-specific positive value. + Deploy toolkit 0.8.2 directly with OCR 1.10.0; do not install OCR 1.9.10 as an intermediate step. The public Linux amd64 example verifies SHA-256 `f8f99ea071bed77dbcaa15fdd2083287bb8ae408d5928b3943ebe0788d191b6b`; the manifest records the complete hosted asset set and qualification evidence. The contiguous 1.9.7 through 1.9.8 promotion combines the repository-owned hosted Linux qualification with adjacent upstream source review. It also repeats the required 1.9.8 contract probes through the checksum-verified Darwin arm64 artifact. Multi-tool MCP initialization, fixed tool discovery and dispatch, and owner-only session persistence remain compatible with the M5 design: the toolkit may expose several fixed tools from its one built-in process, while an isolated `HOME` contains OCR's `.opencodereview/sessions` state for deterministic cleanup. Gemini and Bedrock are optional upstream provider capabilities that the toolkit neither selects nor configures; mirror-aware installation is outside the toolkit-owned download path; native severity retains the structured result fields already consumed; and human-audience progress routing does not affect the toolkit's agent-audience invocation. Therefore this promotion requires no toolkit CI behavior adaptation beyond version and checksum pins. These source observations establish capability only; the M5 production-path tests remain responsible for proving the toolkit's composition and containment. diff --git a/docs/configuration.md b/docs/configuration.md index 2765b42..974f852 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -98,7 +98,7 @@ These names belong to `examples/gitlab/ocr-review.gitlab-ci.yml`; they are shell | **`OCR_TOOLKIT_CHECKSUMS_URL`** | Example pipeline | Yes | Release URL derived from `OCR_TOOLKIT_VERSION` | Toolkit `SHA256SUMS` URL. | | `OCR_TOOLKIT_WHEEL` | Example shell | Computed | `open_code_review_toolkit-${OCR_TOOLKIT_VERSION}-py3-none-any.whl` | Exact wheel filename selected from the release. | | `OCR_TOOLKIT_WHEEL_SHA256` | Example shell | Computed | Matching value from `SHA256SUMS` | Digest checked before installing the toolkit wheel. | -| `OCR_MAX_TOOLS` | Example pipeline / OCR CLI | No | `30` | Positive maximum OCR tool requests per review loop; the example owns and passes this bounded value explicitly. | +| `OCR_MAX_TOOLS` | Example pipeline / OCR CLI | No | `0` | Non-negative tool-call control passed to OCR. `0` delegates to the installed OCR template; a positive value can raise that template-owned loop limit, subject to OCR's runtime validation. | | `OCR_MAX_TOKENS_BUDGET` | Example pipeline / OCR CLI | No | `0` | Non-negative aggregate OCR token ceiling; `0` is unlimited. | ## Dynamic adapter and MCP inputs @@ -164,6 +164,16 @@ The toolkit publishes that run as partial and never treats it as clean or eligib for automatic approval. The cap is approximate because already-running work may finish and OCR accounts the provider-reported input plus output tokens. +`OCR_MAX_TOOLS` is independent of all three token controls. Leave the example +default at `0` so the installed OCR template owns the effective per-file +tool-call limit. For the currently qualified OCR, behavioral qualification +observes a CLI minimum and normalization target of `50`, while omitted, `0`, +and values through `100` all retain the template's effective `100` rounds; a +value of `101` raises the effective limit to `101`. The OCR help text is not the +source of truth for these values. A recognized normalization is emitted only as +a toolkit-authored CI notice; its raw stderr is not added to findings, result +warnings, receipts, DLP inputs, telemetry, or automatic-approval signals. + ## Posting controls `OCR_POST_MODE`, `OCR_STRICT_POSTING`, `OCR_EXIT_CODE`, `OCR_MAX_POST_COMMENTS`, `OCR_MAX_RESULT_BYTES`, `OCR_POST_ERROR_DETAILS`, `OCR_POST_EMOJI`, `OCR_POST_BADGES`, and `OCR_AUTO_APPROVE` control write behavior and bounded error reporting. Human replies to bot-created discussions prevent automated ownership actions on that discussion. diff --git a/docs/development.md b/docs/development.md index 8cd3195..91b3e97 100644 --- a/docs/development.md +++ b/docs/development.md @@ -57,6 +57,12 @@ Select checks from the changed boundary rather than from an ever-growing generic - package or executable-integration changes include clean wheel and sdist validation rather than mocks alone; - public-source changes keep private audit material untracked and run the pinned complete-range Gitleaks wrapper before push; and - release changes run the release authorization, receipt, workflow, artifact, and documentation suites owned by `docs/release.md`. +- OCR pin changes run production-equivalent behavioral probes for every numeric + CLI option the toolkit example passes. Cover omitted/default, each sentinel, + invalid-below-boundary, minimum minus one, minimum, a representative value, + and maximum plus one when a maximum exists. Record only closed exit, + diagnostic, normalization, ownership, and effective-value facts; help text + alone is not compatibility evidence. Safe bounded read-only diagnostics are allowed. A boundary rule prohibits the unsafe acquisition, trust transition, or mutation mechanism, not HTTP, subprocesses, provider APIs, file cleanup, or debugging as whole categories. diff --git a/docs/engineering/test_evidence_matrix.md b/docs/engineering/test_evidence_matrix.md index de59061..14f321d 100644 --- a/docs/engineering/test_evidence_matrix.md +++ b/docs/engineering/test_evidence_matrix.md @@ -99,7 +99,8 @@ The M5 negative suite is an attacker matrix, not merely a replay of observed rev | Requirement or boundary | Production owner and entry point | Required observable result | Evidence | Double boundary and claim limit | State | | --- | --- | --- | --- | --- | --- | -| Exact OCR 1.10.0 review contract | compatibility gateway -> hosted Linux binary -> manifest/evidence promotion | path-aware comments, semantic grouping/filtering, default-medium two rounds, token/tool accounting, partial budget, inherited `16384` and explicit `4096` completion caps | hosted run 32825123658; checksum-pinned `compatibility/evidence/ocr-1.10.0.json`; independent Darwin arm64 no-LLM gateway probe | deterministic gateway proves the consumed wire/result contract, not provider quality or production spending policy | proven for exact qualified OCR wire contract | +| Exact OCR 1.10.0 review contract | compatibility gateway -> hosted Linux binary -> manifest/evidence promotion | path-aware comments, semantic grouping/filtering, default-medium two rounds, token/tool accounting, partial budget, inherited `16384` and explicit `4096` completion caps, and closed numeric CLI/effective-value boundaries | hosted run 32825123658; checksum-pinned `compatibility/evidence/ocr-1.10.0.json`; independent Darwin arm64 no-LLM gateway probes including actual max-tools loop exhaustion | deterministic gateway proves the consumed wire/result/loop contract, not provider quality or production spending policy | proven for exact qualified OCR wire contract | +| Normalized max-tools preview reaches enriched production orchestration safely | `run_evidence_review` collection -> context/store -> MCP composition/self-query -> exact production OCR preview argv | exact recognized `30 -> 50` diagnostic crosses the gate, emits only a closed operator notice, and does not enter result warnings or approval inputs; unknown output remains rejected | parser adversarial matrix plus complete enriched/MCP production-caller regression in `test_review_runner.py` | a subprocess double beyond the production launcher emits the qualified OCR diagnostic and stops at the model-review boundary; the exact binary probe separately proves its grammar and effective rounds | proven for toolkit orchestration without a model-quality claim | | Operator effort stays closed and source-independent | `configure.review_effort` -> generated root `effort` -> OCR CLI precedence | unset/empty becomes `medium`; only `low|medium|high` is accepted; explicit OCR `--effort` remains the per-run override | environment/configuration, runtime-helper, integration, compatibility, and installed wheel/sdist tests | environment and local gateway fixtures prove configuration/round wiring; they do not prescribe a cost-quality choice for every repository | proven | | Group/round diagnostics cannot gain posting or approval authority | private result -> recursive DLP -> canonical projection -> receipt v5 -> approval | safe group data leaves the canonical projection and eligibility unchanged; PII/secrets are privately sanitized; receipt extensions fail closed | `test_review_runner.py`, `test_posting_approval.py`, and BL-017 source-to-signal audit | result fixtures prove deterministic toolkit isolation, not the semantic quality of OCR grouping | proven | | Result-file ownership remains toolkit-only | review argument validation -> pre-opened result descriptor -> atomic finalization/cleanup | caller `--output`, `--output=...`, `-o`, and attached short forms fail before preview | `test_review_runner.py` and installed-artifact tests | subprocess doubles prove rejection timing; the exact OCR binary independently proves the new flag exists | proven | diff --git a/docs/gitlab.md b/docs/gitlab.md index 28b8dfc..4ac1bbb 100644 --- a/docs/gitlab.md +++ b/docs/gitlab.md @@ -18,7 +18,7 @@ The complete variable inventory, owner, requirement, exact default, and behavior The public pipeline stores the OCR binary checksum as the non-secret `OCR_SHA256` pin. Store actual credentials as masked, protected CI variables; do not place their values in YAML, command arguments, repository evidence, or the generated bootstrap. GitLab job tokens are not accepted for posting. -`OCR_REVIEW_LANGUAGE` defaults to `English`; `Russian` is one example of an explicit review language. Toolkit 0.8.2 pins OCR 1.10.0 and sets `OCR_REVIEW_EFFORT=medium`, allowing two review rounds; `low` and `high` explicitly select one or three. OCR may stop early when a round adds no finding. The example passes `OCR_MAX_TOOLS=30` as a separate bound on tool interaction inside a review round; increase it deliberately only when a reviewed repository needs more tool interaction. `OCR_MAX_TOKENS_BUDGET` defaults to `0`, meaning unlimited; a positive budget may stop dispatch and produce an explicitly partial, automatic-approval-ineligible review. `OCR_LLM_MAX_COMPLETION_TOKENS` defaults to unset and separately controls only the provider request's completion/output cap; `4096` is a practical explicit value for gateways that reserve spending against a larger requested maximum. +`OCR_REVIEW_LANGUAGE` defaults to `English`; `Russian` is one example of an explicit review language. The example pins its qualified OCR release and sets `OCR_REVIEW_EFFORT=medium`, allowing two review rounds; `low` and `high` explicitly select one or three. OCR may stop early when a round adds no finding. `OCR_MAX_TOOLS=0` delegates the per-file tool-call limit to the installed OCR template; use a positive value only after checking that release's behaviorally qualified minimum, normalization, and effective value. `OCR_MAX_TOKENS_BUDGET` defaults to `0`, meaning unlimited; a positive budget may stop dispatch and produce an explicitly partial, automatic-approval-ineligible review. `OCR_LLM_MAX_COMPLETION_TOKENS` defaults to unset and separately controls only the provider request's completion/output cap; `4096` is a practical explicit value for gateways that reserve spending against a larger requested maximum. OCR 1.10.0 semantically groups related changed files before review and filters candidates per group. Grouping, filtering, and multiple rounds can increase provider requests, latency, and token cost, while the manifest and aggregate budget continue to report completeness. Group labels and path-derived keys are untrusted private OCR diagnostics; the toolkit does not publish or use them for severity, fingerprints, lifecycle commands, receipts, telemetry, or approval. OCR's separately configured telemetry may export those high-cardinality repository-derived values, so keep `OCR_TELEMETRY_ENABLED=false` unless the exporter and retention policy are approved. diff --git a/docs/operations.md b/docs/operations.md index d38f74a..1b4fb7a 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -36,7 +36,7 @@ suppressed. This aggregate budget is separate from both OCR's prompt/context `max_tokens` ceiling and the provider request's completion/output cap. The toolkit does not add an environment alias for OCR's prompt/context control. `OCR_LLM_MAX_COMPLETION_TOKENS` defaults to unset and, when set, overlays only the protocol-specific output field. The inherited OpenAI value was 58,888 in OCR 1.9.10 and is 16,384 in OCR 1.10.0. A gateway may reserve cost against that requested maximum before generation even when the eventual response would be short. The `/models` capability value does not reveal an account spending limit or reservation policy, so the toolkit never selects the cap from it automatically. Set an explicit cap when a deployment must not change with the qualified OCR version. -`OCR_REVIEW_EFFORT=medium` is the toolkit default for OCR 1.10.0 and permits two review rounds; `low` permits one and `high` permits three. This is a review-depth choice, not one of the three token controls. OCR first groups related changed files and may make group-filter requests; additional rounds can add requests, latency, and cost, but may stop early when they add no finding. The published GitLab example separately passes `OCR_MAX_TOOLS=30` for tool interaction inside each review round. Exhausting that limit, an aggregate budget stop, or incomplete manifest coverage remains explicit and approval-ineligible; increasing either effort or tools is never a way to hide partial coverage. +`OCR_REVIEW_EFFORT=medium` is the toolkit default for the qualified OCR release and permits two review rounds; `low` permits one and `high` permits three. This is a review-depth choice, not one of the three token controls. OCR first groups related changed files and may make group-filter requests; additional rounds can add requests, latency, and cost, but may stop early when they add no finding. The published GitLab example separately passes `OCR_MAX_TOOLS=0`, delegating the effective per-file tool-call limit to the installed OCR template instead of carrying a release-specific minimum. A positive value can raise that limit, but operators should check the release's behavioral qualification because CLI help, normalization text, and effective template value can differ. Exhausting the effective limit, an aggregate budget stop, or incomplete manifest coverage remains explicit and approval-ineligible; increasing either effort or tools is never a way to hide partial coverage. The outcome wording distinguishes skipped, complete, complete-with-warnings, incomplete, token-budget, and failed reviews while preserving the finding state in that same line. A complete clean review is visibly positive; a complete review with findings or only reviewer-suppressed findings is neutral; warning, partial, budget, and failed states never look clean. Findings withheld by the posting limit remain counted even when the limit allows no individual finding note. Recommended focus areas ranks only its copy of already-published findings by the closed severity, category, safe repository location, and stable-identity order before its existing display cap; inline and fallback discussion order, suppression, counts, security focus, and approval policy remain unchanged. OCR 1.8.5 and later manifest failures provide the canonical failed-file receipt; legacy warnings are a bounded fallback, and `summary.files_reviewed` is never treated as proof of successful coverage. Technical details label the aggregate as all OCR tool calls, separately label toolkit-verified MCP-server calls, and show built-in evidence `summary`/`list`/`get` counts only after exact reconciliation; unavailable attribution is not displayed as zero. Zero-valued token counters and configured-but-unused MCP servers are omitted. Token usage renders only validated input/output/cached/reasoning/total/derived-other buckets; malformed or contradictory counters are unavailable and unknown provider keys are not published. Status and aggregate semantic-category emoji are enabled by default and can be disabled together with `OCR_POST_EMOJI=false`; finding labels remain text unless their separate badge mode is enabled. diff --git a/examples/gitlab/README.md b/examples/gitlab/README.md index df4ecf4..9427a2f 100644 --- a/examples/gitlab/README.md +++ b/examples/gitlab/README.md @@ -2,7 +2,7 @@ [`ocr-review.gitlab-ci.yml`](ocr-review.gitlab-ci.yml) is the complete checksum-pinned pipeline. It defaults to identity-only review and contains the install, preflight, configure, review, and post lifecycle. Copy one mode file below into the pipeline's top-level `variables` mapping, or include exactly one file after reviewing its trust boundary. -The pipeline pins OCR 1.10.0 and explicitly sets `OCR_REVIEW_EFFORT=medium`, which permits two review rounds. Use `low` for one round when latency/cost is the priority, or `high` for three only after accepting the additional provider work. Semantic grouping and filtering are OCR behavior shared by every context mode; they do not change which merge-request text a mode admits. `OCR_MAX_TOOLS`, `OCR_MAX_TOKENS_BUDGET`, and the optional completion cap remain independent controls. +The pipeline pins OCR 1.10.0 and explicitly sets `OCR_REVIEW_EFFORT=medium`, which permits two review rounds. Use `low` for one round when latency/cost is the priority, or `high` for three only after accepting the additional provider work. Semantic grouping and filtering are OCR behavior shared by every context mode; they do not change which merge-request text a mode admits. `OCR_MAX_TOOLS=0` delegates the effective per-file tool-call limit to the installed OCR template. `OCR_MAX_TOKENS_BUDGET` and the optional completion cap remain independent controls. | Mode | Recipe | MR text admitted | External access | Automatic approval | | --- | --- | --- | --- | --- | diff --git a/examples/gitlab/ocr-review.gitlab-ci.yml b/examples/gitlab/ocr-review.gitlab-ci.yml index e5a8bc6..8210d92 100644 --- a/examples/gitlab/ocr-review.gitlab-ci.yml +++ b/examples/gitlab/ocr-review.gitlab-ci.yml @@ -22,8 +22,8 @@ variables: OCR_LLM_ALLOWED_MODELS: "" # Two review rounds by default; low and high remain explicit operator choices. OCR_REVIEW_EFFORT: "medium" - # Bounded example cap; raise deliberately for repositories that need more tool interaction. - OCR_MAX_TOOLS: "30" + # Inherit the installed OCR template; use a positive value only to raise its tool-call limit. + OCR_MAX_TOOLS: "0" # Optional aggregate review ceiling; 0 keeps OCR's unlimited default. OCR_MAX_TOKENS_BUDGET: "0" # Reviews default to English. To request another language, for example Russian, set: @@ -78,7 +78,7 @@ open_code_review: - ocr-ci preflight - ocr-ci configure - export OCR_LLM_TOKEN="${OCR_LLM_TOKEN}" - - ocr-ci review --result /tmp/ocr-result.json --stderr /tmp/ocr-stderr.log -- --from "${CI_MERGE_REQUEST_DIFF_BASE_SHA}" --to "${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}" --rule examples/gitlab/rules.json --format json --max-tools "${OCR_MAX_TOOLS:-30}" --max-tokens-budget "${OCR_MAX_TOKENS_BUDGET:-0}" || export OCR_EXIT_CODE=$? + - ocr-ci review --result /tmp/ocr-result.json --stderr /tmp/ocr-stderr.log -- --from "${CI_MERGE_REQUEST_DIFF_BASE_SHA}" --to "${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}" --rule examples/gitlab/rules.json --format json --max-tools "${OCR_MAX_TOOLS:-0}" --max-tokens-budget "${OCR_MAX_TOKENS_BUDGET:-0}" || export OCR_EXIT_CODE=$? - ocr-ci post --result /tmp/ocr-result.json --stderr /tmp/ocr-stderr.log open_code_review_self_test: diff --git a/scripts/ocr_compat.py b/scripts/ocr_compat.py index f3e1d2f..13d8912 100644 --- a/scripts/ocr_compat.py +++ b/scripts/ocr_compat.py @@ -45,6 +45,7 @@ MAX_RELEASE_CHANGES_CHARS = 4_000 MAX_RELEASE_CHANGES_LINES = 50 MAX_QUALIFICATION_CHAIN = 10 +MAX_CLI_PROBE_BYTES = 100_000 DOWNLOAD_ATTEMPTS = 3 VERSION_RE = re.compile(r"^v?(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$") SHA256_RE = re.compile(r"^(?:sha256:)?([0-9a-f]{64})$") @@ -62,11 +63,56 @@ "--background-file", "--format", "--max-tokens-budget", + "--max-tools", "--from", "--preview", "--rule", "--to", } +MAX_TOOLS_NORMALIZATION_RE = re.compile( + r"\[ocr\] --max-tools ([0-9]{1,12}) is below minimum ([0-9]{1,12}), " + r"using ([0-9]{1,12})\n?\Z" +) +MAX_TOOLS_NEGATIVE_RE = re.compile( + r"Error: --max-tools must be a non-negative integer " + r"\(0 means use template default\)\n?\Z" +) +MAX_TOKENS_BUDGET_NEGATIVE_RE = re.compile( + r"Error: --max-tokens-budget must be a non-negative integer " + r"\(0 means unlimited\)\n?\Z" +) +CURRENT_NUMERIC_CLI_CONTRACT: dict[str, object] = { + "max_tokens_budget": { + "cases": { + "invalid_below": {"effective": None, "input": -1, "outcome": "rejected"}, + "minimum": {"effective": 1, "input": 1, "outcome": "accepted"}, + "omitted": {"effective": "unlimited", "input": None, "outcome": "accepted"}, + "representative": {"effective": 30_000, "input": 30_000, "outcome": "accepted"}, + "sentinel": {"effective": "unlimited", "input": 0, "outcome": "accepted"}, + }, + "maximum": None, + "owner": "ocr-cli", + }, + "max_tools": { + "cases": { + "invalid_below": {"effective": None, "input": -1, "outcome": "rejected"}, + "minimum": {"effective": 100, "input": 50, "outcome": "accepted"}, + "minimum_minus_one": { + "effective": 100, + "input": 49, + "outcome": "normalized", + "reported_normalization": 50, + }, + "omitted": {"effective": 100, "input": None, "outcome": "accepted"}, + "representative": {"effective": 101, "input": 101, "outcome": "accepted"}, + "sentinel": {"effective": 100, "input": 0, "outcome": "accepted"}, + }, + "maximum": None, + "owner": "ocr-template-or-higher-cli", + "reported_minimum": 50, + }, + "result": "passed", +} REQUIRED_ASSETS = { "opencodereview-darwin-amd64", "opencodereview-darwin-arm64", @@ -350,6 +396,8 @@ def validate_manifest(manifest: dict[str, Any], root: Path = ROOT) -> None: "wire_field": "max_completion_tokens", }: _fail(f"evidence does not qualify the completion cap for {version}") + if contracts.get("numeric_cli_probe") != CURRENT_NUMERIC_CLI_CONTRACT: + _fail(f"evidence does not qualify numeric CLI boundaries for {version}") evidence_assets = evidence.get("assets") if not isinstance(evidence_assets, list): _fail(f"evidence assets are missing for {version}") @@ -711,6 +759,7 @@ class _StubHandler(http.server.BaseHTTPRequestHandler): tokens_per_request = 2 grouping_tokens_per_request = 2 grouping_mode = "singletons" + main_mode = "findings" completion_caps: list[object] = [] request_stages: list[str] = [] @@ -807,41 +856,57 @@ def do_POST(self) -> None: stage = "filter" else: contents = type(self)._message_contents(messages) - prior_comment = any( - isinstance(message, dict) - and any( - isinstance(call, dict) - and isinstance(call.get("function"), dict) - and call["function"].get("name") == "code_comment" - for call in message.get("tool_calls", []) - ) - for message in messages - ) - later_round = any("### Previously Confirmed Findings" in item for item in contents) - if not prior_comment and not later_round: - path = type(self)._review_path(messages) - if path is None: - self.send_error(400) - return - arguments = json.dumps( - { - "comments": [ - { - "content": "Synthetic compatibility finding.", - "existing_code": " return 2", - "path": path, - "thinking": "Synthetic private compatibility reasoning.", - "category": "maintainability", - "severity": "low", - } - ] + if type(self).main_mode == "exhaust-tools": + if "file_read" in tool_names: + function = { + "name": "file_read", + "arguments": json.dumps( + {"file_path": "example.py", "start_line": 1, "end_line": 1} + ), } - ) - function = {"name": "code_comment", "arguments": arguments} - call_id = "call-comment" + call_id = f"call-read-{type(self).request_count}" + stage = "main" + else: + function = {"name": "task_done", "arguments": "{}"} + call_id = "call-grace-done" + stage = "grace" else: - function = {"name": "task_done", "arguments": "{}"} - call_id = "call-done" + prior_comment = any( + isinstance(message, dict) + and any( + isinstance(call, dict) + and isinstance(call.get("function"), dict) + and call["function"].get("name") == "code_comment" + for call in message.get("tool_calls", []) + ) + for message in messages + ) + later_round = any("### Previously Confirmed Findings" in item for item in contents) + if not prior_comment and not later_round: + path = type(self)._review_path(messages) + if path is None: + self.send_error(400) + return + arguments = json.dumps( + { + "comments": [ + { + "content": "Synthetic compatibility finding.", + "existing_code": " return 2", + "path": path, + "thinking": "Synthetic private compatibility reasoning.", + "category": "maintainability", + "severity": "low", + } + ] + } + ) + function = {"name": "code_comment", "arguments": arguments} + call_id = "call-comment" + else: + function = {"name": "task_done", "arguments": "{}"} + call_id = "call-done" + stage = "main" message = { "role": "assistant", "content": None, @@ -854,7 +919,6 @@ def do_POST(self) -> None: ], } finish_reason = "tool_calls" - stage = "main" type(self).request_stages.append(stage) usage_tokens = ( type(self).grouping_tokens_per_request @@ -892,6 +956,7 @@ def _stub_gateway( tokens_per_request: int = 2, grouping_tokens_per_request: int = 2, grouping_mode: str = "singletons", + main_mode: str = "findings", ) -> Iterator[str]: """Serve deterministic responses with configurable real usage accounting.""" @@ -899,10 +964,13 @@ def _stub_gateway( _fail("stub gateway token usage must be at least two") if grouping_mode not in {"singletons", "combined"}: _fail("stub gateway grouping mode is invalid") + if main_mode not in {"findings", "exhaust-tools"}: + _fail("stub gateway main mode is invalid") _StubHandler.request_count = 0 _StubHandler.tokens_per_request = tokens_per_request _StubHandler.grouping_tokens_per_request = grouping_tokens_per_request _StubHandler.grouping_mode = grouping_mode + _StubHandler.main_mode = main_mode _StubHandler.completion_caps = [] _StubHandler.request_stages = [] server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _StubHandler) @@ -1079,6 +1147,302 @@ def _budget_result_probe(binary: Path, directory: Path) -> dict[str, object]: } +def _run_numeric_preview_case( + binary: Path, + repo: Path, + base: str, + head: str, + directory: Path, + *, + flag: str, + value: int | None, +) -> subprocess.CompletedProcess[str]: + """Run one bounded no-model numeric CLI preview and retain diagnostics privately.""" + + case_name = "omitted" if value is None else str(value).replace("-", "negative-") + env = _isolated_probe_environment(directory / f"numeric-{flag.removeprefix('--')}-{case_name}") + command = [ + str(binary), + "review", + "--from", + base, + "--to", + head, + "--preview", + "--format", + "json", + ] + if value is not None: + command.extend([flag, str(value)]) + try: + completed = subprocess.run( # nosec B603 + command, + cwd=repo, + env=env, + check=False, + capture_output=True, + text=True, + timeout=60, + ) + except (OSError, subprocess.TimeoutExpired) as exc: + raise CompatibilityError(f"numeric CLI preview failed for {flag}") from exc + if ( + len(completed.stdout.encode()) > MAX_CLI_PROBE_BYTES + or len(completed.stderr.encode()) > MAX_CLI_PROBE_BYTES + ): + _fail(f"numeric CLI preview exceeded its output bound for {flag}") + return completed + + +def _accepted_numeric_preview(completed: subprocess.CompletedProcess[str], *, flag: str) -> None: + """Require one clean accepted numeric preview without trusting help text.""" + + if completed.returncode != 0 or completed.stderr: + _fail(f"candidate {flag} preview did not pass without diagnostics") + try: + payload = json.loads(completed.stdout) + except json.JSONDecodeError as exc: + raise CompatibilityError(f"candidate {flag} preview did not emit JSON") from exc + if not isinstance(payload, dict) or not isinstance(payload.get("files"), list): + _fail(f"candidate {flag} preview emitted an invalid file manifest") + + +def _effective_max_tools_probe( + binary: Path, + repo: Path, + base: str, + head: str, + directory: Path, + *, + value: int, + expected_diagnostic: re.Pattern[str] | None, +) -> int: + """Count real OCR main-loop rounds through a deterministic local protocol peer.""" + + env = _isolated_probe_environment(directory / f"effective-max-tools-{value}") + with _stub_gateway(main_mode="exhaust-tools") as gateway_url: + env.update( + { + "OCR_LLM_URL": gateway_url, + "OCR_LLM_TOKEN": "synthetic-token", + "OCR_LLM_MODEL": "synthetic-model", + "OCR_LLM_PROTOCOL": "openai", + "OCR_TELEMETRY_ENABLED": "false", + } + ) + try: + completed = subprocess.run( # nosec B603 + [ + str(binary), + "review", + "--from", + base, + "--to", + head, + "--format", + "json", + "--audience", + "agent", + "--concurrency", + "1", + "--effort", + "low", + "--no-filter", + "--max-tools", + str(value), + ], + cwd=repo, + env=env, + check=False, + capture_output=True, + text=True, + timeout=120, + ) + except (OSError, subprocess.TimeoutExpired) as exc: + raise CompatibilityError("effective max-tools probe failed") from exc + stages = tuple(_StubHandler.request_stages) + if ( + len(completed.stdout.encode()) > MAX_CLI_PROBE_BYTES + or len(completed.stderr.encode()) > MAX_CLI_PROBE_BYTES + ): + _fail("effective max-tools probe exceeded its output bound") + first_diagnostic = completed.stderr.partition("\n")[0] + if expected_diagnostic is None: + if first_diagnostic.startswith("[ocr] --max-tools"): + _fail("effective max-tools probe returned an unexpected normalization") + elif expected_diagnostic.fullmatch(f"{first_diagnostic}\n") is None: + _fail("effective max-tools probe did not retain the qualified normalization") + try: + payload = json.loads(completed.stdout) + except json.JSONDecodeError as exc: + raise CompatibilityError("effective max-tools probe did not emit JSON") from exc + tool_calls = payload.get("tool_calls") if isinstance(payload, dict) else None + by_tool = tool_calls.get("by_tool") if isinstance(tool_calls, dict) else None + observed = stages.count("main") + if ( + completed.returncode not in {0, 1} + or observed <= 0 + or stages.count("grace") != 1 + or len(stages) != observed + 1 + or not isinstance(by_tool, dict) + or by_tool.get("file_read") != observed + or tool_calls.get("total") != observed + ): + _fail("effective max-tools probe returned an inconsistent tool-loop result") + return observed + + +def _numeric_cli_probe( + binary: Path, repo: Path, base: str, head: str, directory: Path +) -> dict[str, object]: + """Qualify closed numeric CLI boundaries and the resulting effective semantics.""" + + max_tools_omitted = _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tools", value=None + ) + max_tools_sentinel = _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tools", value=0 + ) + max_tools_invalid = _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tools", value=-1 + ) + discovery = _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tools", value=1 + ) + for completed in (max_tools_omitted, max_tools_sentinel): + _accepted_numeric_preview(completed, flag="--max-tools") + if ( + max_tools_invalid.returncode == 0 + or MAX_TOOLS_NEGATIVE_RE.fullmatch(max_tools_invalid.stderr) is None + ): + _fail("candidate max-tools negative boundary did not fail closed") + normalization = MAX_TOOLS_NORMALIZATION_RE.fullmatch(discovery.stderr) + if discovery.returncode != 0 or normalization is None: + _fail("candidate max-tools minimum could not be qualified behaviorally") + requested, minimum, normalized = (int(value) for value in normalization.groups()) + if requested != 1 or minimum <= 1 or normalized != minimum: + _fail("candidate max-tools minimum diagnostic is inconsistent") + below = minimum - 1 + below_preview = _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tools", value=below + ) + below_match = MAX_TOOLS_NORMALIZATION_RE.fullmatch(below_preview.stderr) + if ( + below_preview.returncode != 0 + or below_match is None + or tuple(int(value) for value in below_match.groups()) != (below, minimum, minimum) + ): + _fail("candidate max-tools minimum-minus-one boundary is inconsistent") + minimum_preview = _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tools", value=minimum + ) + _accepted_numeric_preview(minimum_preview, flag="--max-tools") + template_default = _effective_max_tools_probe( + binary, + repo, + base, + head, + directory, + value=below, + expected_diagnostic=MAX_TOOLS_NORMALIZATION_RE, + ) + if template_default < minimum: + _fail("candidate max-tools template default is below the reported minimum") + representative = template_default + 1 + representative_preview = _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tools", value=representative + ) + _accepted_numeric_preview(representative_preview, flag="--max-tools") + effective_representative = _effective_max_tools_probe( + binary, + repo, + base, + head, + directory, + value=representative, + expected_diagnostic=None, + ) + if effective_representative != representative: + _fail("candidate max-tools representative value was not applied exactly") + + budget_cases: dict[str, subprocess.CompletedProcess[str]] = { + "omitted": _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tokens-budget", value=None + ), + "sentinel": _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tokens-budget", value=0 + ), + "invalid_below": _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tokens-budget", value=-1 + ), + "minimum": _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tokens-budget", value=1 + ), + "representative": _run_numeric_preview_case( + binary, repo, base, head, directory, flag="--max-tokens-budget", value=30_000 + ), + } + for name in ("omitted", "sentinel", "minimum", "representative"): + _accepted_numeric_preview(budget_cases[name], flag="--max-tokens-budget") + if ( + budget_cases["invalid_below"].returncode == 0 + or MAX_TOKENS_BUDGET_NEGATIVE_RE.fullmatch(budget_cases["invalid_below"].stderr) is None + ): + _fail("candidate max-tokens-budget negative boundary did not fail closed") + return { + "max_tokens_budget": { + "cases": { + "invalid_below": {"effective": None, "input": -1, "outcome": "rejected"}, + "minimum": {"effective": 1, "input": 1, "outcome": "accepted"}, + "omitted": {"effective": "unlimited", "input": None, "outcome": "accepted"}, + "representative": { + "effective": 30_000, + "input": 30_000, + "outcome": "accepted", + }, + "sentinel": {"effective": "unlimited", "input": 0, "outcome": "accepted"}, + }, + "maximum": None, + "owner": "ocr-cli", + }, + "max_tools": { + "cases": { + "invalid_below": {"effective": None, "input": -1, "outcome": "rejected"}, + "minimum": { + "effective": template_default, + "input": minimum, + "outcome": "accepted", + }, + "minimum_minus_one": { + "effective": template_default, + "input": below, + "outcome": "normalized", + "reported_normalization": minimum, + }, + "omitted": { + "effective": template_default, + "input": None, + "outcome": "accepted", + }, + "representative": { + "effective": representative, + "input": representative, + "outcome": "accepted", + }, + "sentinel": { + "effective": template_default, + "input": 0, + "outcome": "accepted", + }, + }, + "maximum": None, + "owner": "ocr-template-or-higher-cli", + "reported_minimum": minimum, + }, + "result": "passed", + } + + def _semantic_grouping_probe(binary: Path, directory: Path) -> dict[str, object]: """Drive one real two-file group through grouping and medium review rounds.""" @@ -1458,6 +1822,7 @@ def run_contracts(binary: Path, version: str, directory: Path) -> dict[str, Any] } contracts: dict[str, Any] = { + "numeric_cli_probe": _numeric_cli_probe(binary, repo, base, head, directory), "optional_capabilities": optional_capabilities, "review_budget_probe": _budget_result_probe(binary, directory), "target_rule_selection_probe": _target_rule_selection_probe(binary, version, directory), diff --git a/src/ocr_toolkit/review_runner.py b/src/ocr_toolkit/review_runner.py index 93cf277..45ae474 100644 --- a/src/ocr_toolkit/review_runner.py +++ b/src/ocr_toolkit/review_runner.py @@ -117,6 +117,10 @@ r"\[ocr\] --background-file content is ([0-9]{1,12}) characters, exceeding the " r"recommended ([0-9]{1,12}) \(continuing but review quality might be impacted\)\n?\Z" ) +MAX_TOOLS_NORMALIZATION_RE = re.compile( + r"\[ocr\] --max-tools ([0-9]{1,12}) is below minimum ([0-9]{1,12}), " + r"using ([0-9]{1,12})\n?\Z" +) BACKGROUND_HARD_CHARACTER_RE = re.compile( r"Error: background content is ([0-9]{1,12}) characters, exceeding the hard limit of " r"([0-9]{1,12}) \(aborting\)\n?\Z" @@ -149,9 +153,10 @@ class ReviewRunnerError(Exception): @dataclass(frozen=True, slots=True) class BackgroundQualification: - """Carry one toolkit-authored warning derived from installed OCR output.""" + """Carry closed toolkit-authored projections of installed OCR diagnostics.""" warning: str | None = None + operator_notices: tuple[str, ...] = () class BackgroundQualificationRejected(ReviewRunnerError): @@ -1160,7 +1165,7 @@ def _read_bounded_artifact(path: Path, *, limit: int, label: str) -> bytes: def _parse_background_preview(*, returncode: int, stderr: bytes) -> BackgroundQualification: - """Classify only exact installed-OCR background warning and rejection forms.""" + """Classify only exact installed-OCR preview diagnostics consumed by the toolkit.""" try: text = stderr.decode("utf-8", errors="strict") @@ -1170,21 +1175,41 @@ def _parse_background_preview(*, returncode: int, stderr: bytes) -> BackgroundQu soft_matches = [ match for line in lines if (match := BACKGROUND_SOFT_WARNING_RE.fullmatch(line)) is not None ] + max_tools_matches = [ + match for line in lines if (match := MAX_TOOLS_NORMALIZATION_RE.fullmatch(line)) is not None + ] hard_character = BACKGROUND_HARD_CHARACTER_RE.fullmatch(text) hard_file = BACKGROUND_HARD_FILE_RE.fullmatch(text) if returncode == 0: - if len(soft_matches) != len(lines) or len(soft_matches) > 1: + if ( + len(soft_matches) + len(max_tools_matches) != len(lines) + or len(soft_matches) > 1 + or len(max_tools_matches) > 1 + ): raise ReviewRunnerError("OCR background preview returned ambiguous diagnostics") - if not soft_matches: - return BackgroundQualification() - actual, limit = (int(value) for value in soft_matches[0].groups()) - if not 0 < limit < actual: - raise ReviewRunnerError("OCR background preview returned invalid thresholds") - return BackgroundQualification( - warning=( + warning: str | None = None + if soft_matches: + actual, limit = (int(value) for value in soft_matches[0].groups()) + if not 0 < limit < actual: + raise ReviewRunnerError("OCR background preview returned invalid thresholds") + warning = ( f"Installed OCR reported a {actual}-character review background above its " f"recommended {limit}-character threshold; review continued." ) + operator_notices: tuple[str, ...] = () + if max_tools_matches: + requested, minimum, normalized = (int(value) for value in max_tools_matches[0].groups()) + if not 0 < requested < minimum or normalized != minimum: + raise ReviewRunnerError("OCR background preview returned invalid tool limits") + operator_notices = ( + "Installed OCR reported " + f"--max-tools {requested} below its CLI minimum and a normalization target " + f"of {normalized}; the installed OCR template remains authoritative for the " + "effective tool-call limit.", + ) + return BackgroundQualification( + warning=warning, + operator_notices=operator_notices, ) if hard_character is not None: actual, limit = (int(value) for value in hard_character.groups()) @@ -1304,6 +1329,8 @@ def _run_background_qualified_review( f"OCR background qualification warning: {qualification.warning}", file=sys.stderr, ) + for notice in qualification.operator_notices: + print(f"OCR argument qualification notice: {notice}", file=sys.stderr) return ( run_review( result_path, diff --git a/tests/test_environment_contract.py b/tests/test_environment_contract.py index 2f7ee41..09dd73d 100644 --- a/tests/test_environment_contract.py +++ b/tests/test_environment_contract.py @@ -75,7 +75,7 @@ "OCR_TOOLKIT_CHECKSUMS_URL": "Release URL derived from `OCR_TOOLKIT_VERSION`", "OCR_TOOLKIT_WHEEL": "open_code_review_toolkit-${OCR_TOOLKIT_VERSION}-py3-none-any.whl", "OCR_TOOLKIT_WHEEL_SHA256": "Matching value from `SHA256SUMS`", - "OCR_MAX_TOOLS": "30", + "OCR_MAX_TOOLS": "0", "OCR_MAX_TOKENS_BUDGET": "0", } @@ -302,7 +302,7 @@ def test_example_local_defaults_match_the_pipeline() -> None: "OCR_VERSION": "v1.10.0", "OCR_SHA256": EXAMPLE_DEFAULTS["OCR_SHA256"], "OCR_TOOLKIT_VERSION": STABLE_TOOLKIT_VERSION, - "OCR_MAX_TOOLS": "30", + "OCR_MAX_TOOLS": "0", "OCR_MAX_TOKENS_BUDGET": "0", "OCR_REVIEW_EFFORT": "medium", }.items(): @@ -312,4 +312,4 @@ def test_example_local_defaults_match_the_pipeline() -> None: in workflow ) assert 'export OCR_TOOLKIT_WHEEL_SHA256="$(awk ' in workflow - assert '--max-tools "${OCR_MAX_TOOLS:-30}"' in workflow + assert '--max-tools "${OCR_MAX_TOOLS:-0}"' in workflow diff --git a/tests/test_integration_contracts.py b/tests/test_integration_contracts.py index da7b620..8eda256 100644 --- a/tests/test_integration_contracts.py +++ b/tests/test_integration_contracts.py @@ -129,8 +129,8 @@ def test_gitlab_example_preserves_review_gating_and_manual_self_test() -> None: assert 'OCR_LLM_VALIDATE_MODEL: "false"' in workflow assert 'OCR_MAX_TOKENS_BUDGET: "0"' in workflow assert 'OCR_REVIEW_EFFORT: "medium"' in workflow - assert 'OCR_MAX_TOOLS: "30"' in workflow - assert '--max-tools "${OCR_MAX_TOOLS:-30}"' in review_job + assert 'OCR_MAX_TOOLS: "0"' in workflow + assert '--max-tools "${OCR_MAX_TOOLS:-0}"' in review_job assert '--max-tokens-budget "${OCR_MAX_TOKENS_BUDGET:-0}"' in review_job assert "lint:\n stage: lint" in workflow assert "open_code_review:" in review_job diff --git a/tests/test_ocr_compat.py b/tests/test_ocr_compat.py index 142e6a1..8c866f2 100644 --- a/tests/test_ocr_compat.py +++ b/tests/test_ocr_compat.py @@ -1068,6 +1068,70 @@ def test_compatibility_gateway_rejects_malformed_messages_over_real_http() -> No assert error.value.code == 400 +def test_numeric_cli_probe_records_closed_boundaries_and_effective_values() -> None: + """Derive effective numeric behavior without retaining raw OCR diagnostics.""" + + module = load_script() + + def preview( + _binary: Path, + _repo: Path, + _base: str, + _head: str, + _directory: Path, + *, + flag: str, + value: int | None, + ) -> Any: + stderr = "" + returncode = 0 + if flag == "--max-tools" and value in {1, 49}: + stderr = f"[ocr] --max-tools {value} is below minimum 50, using 50\n" + elif flag == "--max-tools" and value == -1: + returncode = 1 + stderr = ( + "Error: --max-tools must be a non-negative integer (0 means use template default)\n" + ) + elif flag == "--max-tokens-budget" and value == -1: + returncode = 1 + stderr = ( + "Error: --max-tokens-budget must be a non-negative integer (0 means unlimited)\n" + ) + return module.subprocess.CompletedProcess( + [flag, str(value)], + returncode, + stdout='{"files": []}\n', + stderr=stderr, + ) + + def effective( + _binary: Path, + _repo: Path, + _base: str, + _head: str, + _directory: Path, + *, + value: int, + expected_diagnostic: object, + ) -> int: + return 100 if value == 49 else value + + with ( + patched_attr(module, "_run_numeric_preview_case", preview), + patched_attr(module, "_effective_max_tools_probe", effective), + ): + result = module._numeric_cli_probe( + Path("/synthetic/ocr"), + Path("/synthetic/repo"), + "a" * 40, + "b" * 40, + Path("/synthetic/probe"), + ) + + assert result == module.CURRENT_NUMERIC_CLI_CONTRACT + assert "stderr" not in json.dumps(result) + + def test_asset_download_never_uses_github_api_token(tmp_path: Path) -> None: module = load_script() captured: dict[str, Any] = {} diff --git a/tests/test_operations_docs.py b/tests/test_operations_docs.py index 44ca8d9..7d5eb6b 100644 --- a/tests/test_operations_docs.py +++ b/tests/test_operations_docs.py @@ -495,6 +495,23 @@ def test_ocr_compatibility_workflow_is_bounded_and_protected() -> None: assert "ocr.run-manifest/v1" in policy +def test_numeric_ocr_controls_use_behavioral_qualification_and_template_delegation() -> None: + """Keep help text, normalization, effective values, and authority distinct.""" + + configuration = CONFIGURATION.read_text(encoding="utf-8") + operations = OPERATIONS.read_text(encoding="utf-8") + compatibility = (PROJECT_ROOT / "docs" / "compatibility.md").read_text(encoding="utf-8") + development = (PROJECT_ROOT / "docs" / "development.md").read_text(encoding="utf-8") + + assert "`0` delegates to the installed OCR template" in configuration + assert "template's effective `100` rounds" in configuration + assert "result\nwarnings, receipts, DLP inputs, telemetry" in configuration + assert "`OCR_MAX_TOOLS=0`" in operations + assert "CLI help, normalization text, and effective template value can differ" in operations + assert "effective `100` for omitted, sentinel `0`, `49`, and `50`" in compatibility + assert "help text\n alone is not compatibility evidence" in development + + def test_actions_storage_maintenance_bounds_completed_run_metadata() -> None: workflow = (PROJECT_ROOT / ".github" / "workflows" / "actions-maintenance.yml").read_text( encoding="utf-8" diff --git a/tests/test_review_runner.py b/tests/test_review_runner.py index d00cdf6..9d02dc9 100644 --- a/tests/test_review_runner.py +++ b/tests/test_review_runner.py @@ -2100,14 +2100,31 @@ def test_review_rejects_caller_owned_output(arguments: list[str]) -> None: @pytest.mark.parametrize( - ("returncode", "stderr", "warning", "rejection"), + ("returncode", "stderr", "warning", "notices", "rejection"), [ - (0, b"", None, None), + (0, b"", None, (), None), ( 0, b"[ocr] --background-file content is 2001 characters, exceeding the recommended " b"2000 (continuing but review quality might be impacted)\n", "2001-character review background above its recommended 2000-character threshold", + (), + None, + ), + ( + 0, + b"[ocr] --max-tools 30 is below minimum 50, using 50\n", + None, + ("--max-tools 30", "normalization target of 50", "template remains authoritative"), + None, + ), + ( + 0, + b"[ocr] --max-tools 30 is below minimum 50, using 50\n" + b"[ocr] --background-file content is 2001 characters, exceeding the recommended " + b"2000 (continuing but review quality might be impacted)\n", + "2001-character review background above its recommended 2000-character threshold", + ("--max-tools 30", "normalization target of 50", "template remains authoritative"), None, ), ( @@ -2115,6 +2132,7 @@ def test_review_rejects_caller_owned_output(arguments: list[str]) -> None: b"Error: background content is 8001 characters, exceeding the hard limit of 8000 " b"(aborting)\n", None, + (), ("ocr_background_character_limit", 8_001, 8_000, "characters"), ), ( @@ -2122,6 +2140,7 @@ def test_review_rejects_caller_owned_output(arguments: list[str]) -> None: b'Error: background file "/private/synthetic/background.md" is 1048577 bytes, ' b"exceeding the maximum of 1048576 bytes; please provide a smaller file\n", None, + (), ("ocr_background_file_size_limit", 1_048_577, 1_048_576, "bytes"), ), ], @@ -2130,6 +2149,7 @@ def test_background_preview_parser_accepts_only_closed_installed_ocr_forms( returncode: int, stderr: bytes, warning: str | None, + notices: tuple[str, ...], rejection: tuple[str, int, int, str] | None, ) -> None: """Keep paths and raw OCR diagnostics outside the public qualification result.""" @@ -2139,6 +2159,10 @@ def test_background_preview_parser_accepts_only_closed_installed_ocr_forms( assert warning is None or warning in result.warning # type: ignore[operator] if warning is None: assert result.warning is None + for expected in notices: + assert expected in result.operator_notices[0] + if not notices: + assert result.operator_notices == () return with pytest.raises(review_runner.BackgroundQualificationRejected) as raised: review_runner._parse_background_preview(returncode=returncode, stderr=stderr) @@ -2153,6 +2177,11 @@ def test_background_preview_parser_accepts_only_closed_installed_ocr_forms( [ (1, b"provider token rejected\n"), (0, b"prefix [ocr] --background-file content is 2001 characters\n"), + (0, b"[ocr] --max-tools 30 is below minimum 50, using 50\nunknown\n"), + (0, b"[ocr] --max-tools 30 is below minimum 50, using 50\n" * 2), + (0, b"[ocr] --max-tools 50 is below minimum 50, using 50\n"), + (0, b"[ocr] --max-tools 30 is below minimum 50, using 51\n"), + (0, "[ocr] --max-tools \uff13\uff10 is below minimum 50, using 50\n".encode()), ( 1, b'Error: background file "/private/value\npath" is 9 bytes, exceeding the maximum ' @@ -2383,6 +2412,7 @@ def test_evidence_review_prepares_internal_context_before_ocr( events: list[object] = [] session_homes: list[Path] = [] + real_subprocess_run = subprocess.run original_home = os.environ.get("HOME") artifacts = review_runner.repository_artifacts(tmp_path) review_runner.prepare_artifact_directory(artifacts) @@ -2425,9 +2455,24 @@ def run(result: Path, stderr: Path, args: list[str], **_kwargs: object) -> int: result.write_text(json.dumps({"status": status}), encoding="utf-8") return ocr_exit_code - def qualify(args: list[str], **_kwargs: object) -> review_runner.BackgroundQualification: - events.append(("preview", args)) - return review_runner.BackgroundQualification() + def preview(argv: list[str], **kwargs: object) -> subprocess.CompletedProcess[bytes]: + if "stdout" not in kwargs: + return real_subprocess_run(argv, **kwargs) # type: ignore[return-value] + events.append(("preview", argv)) + kwargs["stdout"].write(b'{"files":[]}\n') # type: ignore[union-attr] + kwargs["stderr"].write( # type: ignore[union-attr] + b"[ocr] --max-tools 30 is below minimum 50, using 50\n" + ) + return subprocess.CompletedProcess(argv, 0) + + finalized: list[dict[str, object]] = [] + + def finalize( + *_args: object, **kwargs: object + ) -> tuple[dict[str, int], bool, dict[str, object]]: + finalized.append(kwargs) + events.append("ocr-usage") + return {"ocr_toolkit_evidence": 1}, False, {"state": "passed"} def write_bootstrap(path: Path, content: str) -> None: events.append(("bootstrap", path, content)) @@ -2476,20 +2521,25 @@ def write_bootstrap(path: Path, content: str) -> None: patched_attr( review_runner, "_finalize_ocr_result", - lambda *_args, **_kwargs: ( - events.append("ocr-usage") or {"ocr_toolkit_evidence": 1}, - False, - {"state": "passed"}, - ), + finalize, ), patched_attr(review_runner, "_resolve_ocr_binary", lambda: "/synthetic/ocr"), - patched_attr(review_runner, "_qualify_review_background", qualify), + patched_attr(review_runner.subprocess, "run", preview), patched_attr(review_runner, "run_review", run), ): result = review_runner.run_evidence_review( tmp_path / "result.json", tmp_path / "stderr.log", - ["--from", "base", "--to", "head", "--format", "json"], + [ + "--from", + "base", + "--to", + "head", + "--format", + "json", + "--max-tools", + "30", + ], preserve_private_artifacts=preserve_private_artifacts, ) @@ -2514,17 +2564,22 @@ def write_bootstrap(path: Path, content: str) -> None: assert events[6] == "self-query" assert events[7][0] == "preview" # type: ignore[index] assert events[7][1] == [ # type: ignore[index] + "/synthetic/ocr", + "review", "--from", "a" * 40, "--to", "b" * 40, "--format", "json", + "--max-tools", + "30", "--background-file", str(artifacts.bootstrap), + "--preview", ] assert events[8][0] == "ocr" # type: ignore[index] - assert events[8][3] == events[7][1] # type: ignore[index] + assert events[8][3] == events[7][1][2:-1] # type: ignore[index] if preserve_private_artifacts: assert "ocr-usage" not in events assert artifacts.store.exists() @@ -2540,6 +2595,7 @@ def write_bootstrap(path: Path, content: str) -> None: else: if ocr_exit_code == 0: assert events[9] == "ocr-usage" + assert finalized[0]["toolkit_warnings"] == () else: assert "ocr-usage" not in events assert not artifacts.store.exists() From e5a889ff2c415adc1dbce706582da5480024cc7d Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:25:17 +0200 Subject: [PATCH 4/8] Record deferred 0.8.3 handoff --- PLANS.md | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/PLANS.md b/PLANS.md index 7085ce2..4b4948d 100644 --- a/PLANS.md +++ b/PLANS.md @@ -131,7 +131,7 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. | `WQ-01` | `done` | Signed planning commit `85b3097`; branch `codex/v0.8.3-ocr-boundaries`; milestone `v0.8.3` #6 with #139/#140; planning push; Draft PR #141 with exact scope/non-claims | | `WQ-02` | `done` | #139: outcome-authoritative workflow selection; portable atomic evidence/status/issue-body handoffs; closed late-write recovery; manifest-independent status upsert; regression tests; public contract and `139.bugfix.md` | | `WQ-03` | `done` | #140: example sentinel `0`; exact normalization parser and operator-only notice; behavioral numeric/effective-loop qualification; updated exact OCR evidence/hash; full enriched/MCP preview regression; public/development contracts and `140.bugfix.md` | -| `WQ-04` | `in_progress` | Reconcile development/public docs and all requirements; run final quality, coverage, manifest, Towncrier, Gitleaks, and complete privacy/data-flow self-review; push completed history and resolve hosted findings | +| `WQ-04` | `done` | Reconciled public/development contracts and requirements; complete quality/coverage, manifest, Towncrier, and privacy/data-flow review are green; final Draft push and hosted readback are handoff actions, not release delivery | | `WQ-05` | `out_of_scope` | Owner-deferred delivery: keep PR #141 Draft after final push; do not merge, publish TestPyPI/PyPI, prepare a release PR, tag, close issues, or close the milestone in this run | #### Locked Decisions @@ -195,6 +195,14 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. registration/self-query, and exact preview before its controlled model boundary; the parsed normalization becomes only a CI notice and leaves result warnings, DLP, receipt, posting, telemetry, and approval inputs unchanged. +- Final local quality gate passes 1,269 tests plus 310 subtests at 86.16% + combined branch coverage; risk groups report 84%, 82%, 85%, and 87% against + floors 80%, 80%, 85%, and 85%. Ruff format/check, strict MyPy, Bandit, + manifest validation, the rendered 0.8.3 Towncrier draft, and complete + requirement/privacy/data-flow self-review pass. The first gate invocation + stopped before tests on four format-only differences; Ruff formatted those + files, signed commit `743d8fa` amended the logical slice, and the complete + gate then passed on the corrected implementation head. #### Risks And Recovery @@ -215,10 +223,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. #### Resume Point -`WQ-04`: run the one complete local quality/security/manifest/changelog gate, -perform the final requirement/privacy/data-flow diff review, update this plan and -Draft PR handoff to exact-head truth, then make the single final push and resolve -only evidence-driven hosted failures without releasing. +At the next owner-authorized continuation, read back Draft PR #141's exact head, +tree, protected hosted checks, and review threads. If they remain green, review +that immutable implementation rather than repeating local development; merge and +release still require a separate explicit decision and the protected lifecycle. #### Plan Fidelity Check @@ -238,11 +246,11 @@ only evidence-driven hosted failures without releasing. #### Closure Gate -- [ ] All requirements and work items are terminal with current validation evidence. -- [ ] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. +- [x] All implementation requirements and in-scope work items are terminal with current local validation evidence. +- [x] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. - [ ] Exact feature head is green locally and in protected hosted checks with resolved review threads. - [ ] PR #141 remains Draft at the exact pushed head; #139/#140 and milestone `v0.8.3` remain open. -- [ ] `scripts/plan_lifecycle.py check` passes with an exact external-review/release resume point; the active plan is not archived before deferred delivery. +- [x] The active plan retains an exact external-review/release resume point and is not archived before deferred delivery; this repository has no separate plan-lifecycle checker. #### Post-Close Delivery From fb8a21444ed10e0b659d521c45fdf17eaeb2c5b4 Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:42:33 +0200 Subject: [PATCH 5/8] Plan OCR tool usage visibility --- PLANS.md | 89 +++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/PLANS.md b/PLANS.md index 4b4948d..c151b9b 100644 --- a/PLANS.md +++ b/PLANS.md @@ -4,11 +4,11 @@ Use this file for active or blocked repository work. Update it before implementa ## Active Work -### Toolkit 0.8.3 — authoritative OCR compatibility status and numeric CLI limits +### Toolkit 0.8.3 — OCR boundaries and review tool-usage visibility Status: `active` Owner: Codex -Plan Origin: `direct_execution` +Plan Origin: `resumed` Release classification: `release-deferred` (the product changes require stable `0.8.3`, but feature merge and publication are explicitly deferred) Target stable version: `0.8.3` @@ -16,12 +16,16 @@ Last Updated: 2026-08-25 #### Goal -Close #139 and #140 as one protected 0.8.3 bug-fix release: make a failed OCR +Close #139 and #140 and add one bounded review-usage feature as one protected +0.8.3 release: make a failed OCR qualification's closed status authoritative even when stale evidence or invalid support metadata exists, and replace the stale example-owned `OCR_MAX_TOOLS=30` contract with installed-OCR delegation plus behaviorally qualified numeric CLI -boundaries. Preserve private diagnostics, aggregate-promotion blocking, the exact -OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. +boundaries. Extend the existing one-line GitLab technical summary so its bounded +numeric breakdown shows the useful non-zero OCR review-tool counters rather than +only the first six. Preserve private diagnostics, aggregate-promotion blocking, +the exact OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval +semantics. #### Requested Scope @@ -44,6 +48,15 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. - Add separate bug-fix changelog fragments for #139 and #140, update canonical development/public contracts, and complete the protected PR and stable-release lifecycle without a local real-LLM/model invocation. +- Keep the current inline `all OCR tool calls` format while publishing every + admitted non-zero counter that helps explain review activity: OCR repository + inspection (`file_read`, `file_read_diff`, `file_find`, `code_search`), review + output/lifecycle (`code_comment`, `task_done`), toolkit context/evidence + (`ocr_toolkit_evidence`, `context_list`, `context_get`), and the already + verified per-server MCP summary. +- Treat tool-call counts as operational activity only. Keep aggregate token + input/output/cache telemetry alongside them, but do not claim or derive + per-tool token consumption because OCR 1.10.0 does not provide that contract. #### Requirement Traceability @@ -57,6 +70,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. | `REQ-140-B` | #140 acceptance 2-4 | Exact enriched/MCP preview accepts valid sentinel and handles known normalization without ambiguous/raw diagnostics | `WQ-03` | Parser negatives plus production-caller integration with controlled OCR peer | | `REQ-140-C` | #140 scope/acceptance 5 | Qualification records behavioral numeric boundaries and effective-value ownership | `WQ-03` | Exact OCR 1.10.0 no-model probe and evidence-schema tests | | `REQ-140-D` | #140 acceptance 6 | Current docs/tests avoid toolkit-release wording where OCR owns the contract | `WQ-03`, `WQ-04` | Documentation contract tests and full-text review | +| `REQ-USAGE-A` | User-approved manager-facing usage summary | Existing inline technical format includes every admitted useful non-zero tool counter, deterministically ordered by count then name | `WQ-06` | Formatter and complete summary regressions with more than six tools | +| `REQ-USAGE-B` | Result/privacy boundary | Only bounded names and integer counters cross into GitLab; arguments, results, paths, IDs, provider data, and unknown/unattributed names remain private | `WQ-06` | Hostile map/name/count, DLP, Markdown, and note-budget regressions | +| `REQ-USAGE-C` | Honest token explanation | Tool activity appears beside aggregate token usage without percentages or per-tool token attribution | `WQ-06` | Rendering and documentation assertions | +| `REQ-USAGE-D` | Existing trust contracts | The expanded diagnostic is not a finding, receipt proof, telemetry source, severity input, or approval signal | `WQ-06`, `WQ-07` | Posting, DLP, and approval regression review | | `REQ-REL` | Repository release contract | Deliver 0.8.3 through reviewed feature and release PRs with external reconciliation | `WQ-01`, `WQ-04`, `WQ-05` | Protected checks, registries, attestations, tag, receipt, issue/milestone closure | #### Explicit Non-Goals @@ -75,6 +92,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. HTTP peer may exercise the OCR process boundary but is not model evidence. - Do not reorganize tests or production modules beyond the cohesive owners directly required by these two fixes. +- Do not change the current one-line technical-summary layout, publish a catalog + of merely available tools, add per-tool token estimates/percentages, or expose + raw tool-call arguments, results, errors, paths, request IDs, or dynamic + external MCP tool names. #### Constraints @@ -99,6 +120,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. subprocess, outcome-consistency, and integration-proof boundaries. - Official OCR `v1.10.0` source in `cmd/opencodereview/shared_flags.go`, `cmd/opencodereview/shared.go`, and embedded `task_template.json`. +- Official OCR `v1.10.0` built-in registry and tools configuration: six native + review tools (`task_done`, `code_comment`, `file_read`, `file_read_diff`, + `file_find`, and `code_search`); toolkit context/evidence tools remain owned by + the exact MCP composition and receipt. - Isolated local OCR 1.10.0 no-model preview evidence: omitted and `0` accepted; negative rejected; `1..49` report normalization to `50`; `50+` accepted; help says `min 10`; embedded template owns `MAX_TOOL_REQUEST_TIMES=100` and applies @@ -113,6 +138,9 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. one final complete local gate, and protected hosted CI. - After the final push, keep PR #141 in Draft. Do not merge the feature branch, publish development/stable artifacts, create a release PR, or release 0.8.3. +- The accepted manager-facing question is “where did the review activity go?”: + preserve the current inline format, show the selected numeric counters, and + explicitly avoid claiming exact per-tool token attribution. #### Completed Baseline State @@ -123,6 +151,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. - #139 reproduces three authority/recovery gaps added by the 0.8.2 failure path. - #140 reproduces before any provider call because successful OCR preview stderr currently accepts only the background-warning grammar. +- The previously handed-off Draft head `e5a889f` is clean and fully green across + protected hosted checks. Its current formatter publishes the aggregate total + but truncates a valid per-tool breakdown to six entries, which is insufficient + for the newly approved activity explanation. #### Current Work Queue @@ -133,6 +165,8 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. | `WQ-03` | `done` | #140: example sentinel `0`; exact normalization parser and operator-only notice; behavioral numeric/effective-loop qualification; updated exact OCR evidence/hash; full enriched/MCP preview regression; public/development contracts and `140.bugfix.md` | | `WQ-04` | `done` | Reconciled public/development contracts and requirements; complete quality/coverage, manifest, Towncrier, and privacy/data-flow review are green; final Draft push and hosted readback are handoff actions, not release delivery | | `WQ-05` | `out_of_scope` | Owner-deferred delivery: keep PR #141 Draft after final push; do not merge, publish TestPyPI/PyPI, prepare a release PR, tag, close issues, or close the milestone in this run | +| `WQ-06` | `in_progress` | Create the tool-usage issue in milestone `v0.8.3`; extend the existing inline formatter to every admitted useful non-zero counter; keep external MCP aggregated by verified server; add focused boundary tests, public operational wording, and a separate feature changelog fragment | +| `WQ-07` | `pending` | Complete focused and final validation, self-review the reporting/privacy/approval flow, update the Draft body and issue checkboxes, make one signed feature commit plus one final push, and wait for exact-head hosted checks while keeping the PR Draft | #### Locked Decisions @@ -149,6 +183,12 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. discovery, qualification, aggregation, and promotion retain manifest validation. - OCR remains external and pinned to 1.10.0. No local model result substitutes for hosted exact-binary or deterministic controlled-peer evidence. +- The existing `all OCR tool calls: N total (...)` line remains the sole OCR + tool-counter format. It lists admitted non-zero counters by descending count + and then name; no grouping, percentages, or token-allocation inference is added. +- Built-in OCR and toolkit-owned context/evidence names may be shown as activity. + External dynamic MCP tools remain represented only through the existing + verified per-server aggregate; unknown raw names never gain public meaning. #### Verification @@ -167,6 +207,10 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. - Hosted: all protected Draft feature checks on the exact final head. Development publication, stable release, registry/provenance/install readback, and closure remain deferred under `docs/release.md`. +- Tool-usage slice: focused formatter/posting/result-DLP/approval tests must prove + more than six useful counters remain visible, deterministic and bounded; raw + call content and unknown names remain absent; aggregate token rendering and + approval decisions are unchanged. #### Latest Validation Results @@ -203,6 +247,13 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. stopped before tests on four format-only differences; Ruff formatted those files, signed commit `743d8fa` amended the logical slice, and the complete gate then passed on the corrected implementation head. +- Protected hosted checks subsequently passed on exact pushed head + `e5a889ff2c415adc1dbce706582da5480024cc7d` and tree + `1b89c1a69a7014baa0199343c457e7eb69419697`; PR #141 remained Draft, #139/#140 + acceptance boxes were checked, and both issues plus milestone stayed open. +- The user then approved an additive numeric tool-activity breakdown in the + existing format. No repository or GitHub write for that new slice preceded + this resumed-plan materialization. #### Risks And Recovery @@ -220,13 +271,21 @@ OCR 1.10.0 pin, DLP, receipt v5, posting, and automatic-approval semantics. - External write or hosted CI failure: preserve local commits and Draft PR, record exact run evidence, fix only the demonstrated boundary, and rerun its focused gate before a new signed commit. +- Tool counts can be mistaken for token allocation, and one tool call can return + very different context volume from another. Recovery: label them only as OCR + tool calls, retain the separate aggregate token line, document the limitation, + and never compute per-tool token shares. +- A hostile or future OCR result can add names or excessive counters. Recovery: + admit only the closed useful set, validate bounded integer counts, sort + deterministically, and omit unknown/raw entries without changing review or + approval state. #### Resume Point -At the next owner-authorized continuation, read back Draft PR #141's exact head, -tree, protected hosted checks, and review threads. If they remain green, review -that immutable implementation rather than repeating local development; merge and -release still require a separate explicit decision and the protected lifecycle. +Continue at `WQ-06`: create the scoped GitHub issue and attach it to milestone +`v0.8.3`, then implement the closed useful-counter projection in the existing +formatter and its focused tests. Do not change the format, push, merge, or start +release delivery before the new logical slice and `WQ-07` gates are complete. #### Plan Fidelity Check @@ -236,6 +295,8 @@ release still require a separate explicit decision and the protected lifecycle. - [x] Rejected scope and overengineering boundaries are explicit. - [x] Each logical commit has focused verification and self-review gates. - [x] External writes, hosted delivery, recovery, and exact resume state are represented. +- [x] The accepted tool list, current-format decision, token-attribution + non-claim, and unknown/external-tool privacy boundary are explicit. #### Reconciliation Check @@ -243,11 +304,13 @@ release still require a separate explicit decision and the protected lifecycle. - [x] #139 and #140 are the complete open issue set and do not conflict with backlog/roadmap scope. - [x] Stable 0.8.2 closure remains historical and is not rewritten. - [x] Target 0.8.3 matches `.next-version` and no competing milestone exists. +- [x] The new reporting slice extends the open Draft and milestone without + changing the completed #139/#140 contracts or deferred-release boundary. #### Closure Gate -- [x] All implementation requirements and in-scope work items are terminal with current local validation evidence. -- [x] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. +- [ ] All implementation requirements and in-scope work items are terminal with current local validation evidence. +- [ ] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. - [ ] Exact feature head is green locally and in protected hosted checks with resolved review threads. - [ ] PR #141 remains Draft at the exact pushed head; #139/#140 and milestone `v0.8.3` remain open. - [x] The active plan retains an exact external-review/release resume point and is not archived before deferred delivery; this repository has no separate plan-lifecycle checker. @@ -262,6 +325,10 @@ release still require a separate explicit decision and the protected lifecycle. - #139/#140 and milestone `v0.8.3` remain open until stable receipt publication. - Final handoff must repeat that no local real-LLM/provider qualification was run or claimed, while identifying the exact deterministic OCR boundary evidence. +- The resumed slice needs one new feature issue and changelog fragment; #139 and + #140 stay checked/open and must not absorb this manager-facing reporting change. +- Final handoff must state that tool-call counts explain review activity only; + exact per-tool token consumption remains unavailable in OCR 1.10.0. #### Handoff Notes From a416f378d421594f208ae6a9a799490df97b2a9a Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:52:37 +0200 Subject: [PATCH 6/8] Expose bounded OCR tool usage --- PLANS.md | 35 +++++---- changelog.d/142.feature.md | 4 + docs/gitlab.md | 2 + docs/operations.md | 2 +- src/ocr_toolkit/ocr_result.py | 17 +++++ src/ocr_toolkit/posting/formatting.py | 97 +++++++++++------------- src/ocr_toolkit/posting/settings.py | 6 -- src/ocr_toolkit/review_runner.py | 67 +++++------------ tests/test_posting_helpers.py | 102 ++++++++++++++++++++++++++ tests/test_review_runner.py | 91 +++++++++++++++++++++-- 10 files changed, 296 insertions(+), 127 deletions(-) create mode 100644 changelog.d/142.feature.md diff --git a/PLANS.md b/PLANS.md index c151b9b..7c23eec 100644 --- a/PLANS.md +++ b/PLANS.md @@ -70,10 +70,10 @@ semantics. | `REQ-140-B` | #140 acceptance 2-4 | Exact enriched/MCP preview accepts valid sentinel and handles known normalization without ambiguous/raw diagnostics | `WQ-03` | Parser negatives plus production-caller integration with controlled OCR peer | | `REQ-140-C` | #140 scope/acceptance 5 | Qualification records behavioral numeric boundaries and effective-value ownership | `WQ-03` | Exact OCR 1.10.0 no-model probe and evidence-schema tests | | `REQ-140-D` | #140 acceptance 6 | Current docs/tests avoid toolkit-release wording where OCR owns the contract | `WQ-03`, `WQ-04` | Documentation contract tests and full-text review | -| `REQ-USAGE-A` | User-approved manager-facing usage summary | Existing inline technical format includes every admitted useful non-zero tool counter, deterministically ordered by count then name | `WQ-06` | Formatter and complete summary regressions with more than six tools | -| `REQ-USAGE-B` | Result/privacy boundary | Only bounded names and integer counters cross into GitLab; arguments, results, paths, IDs, provider data, and unknown/unattributed names remain private | `WQ-06` | Hostile map/name/count, DLP, Markdown, and note-budget regressions | -| `REQ-USAGE-C` | Honest token explanation | Tool activity appears beside aggregate token usage without percentages or per-tool token attribution | `WQ-06` | Rendering and documentation assertions | -| `REQ-USAGE-D` | Existing trust contracts | The expanded diagnostic is not a finding, receipt proof, telemetry source, severity input, or approval signal | `WQ-06`, `WQ-07` | Posting, DLP, and approval regression review | +| `REQ-USAGE-A` | #142 and user-approved manager-facing usage summary | Existing inline technical format includes every admitted useful non-zero tool counter, deterministically ordered by count then name | `WQ-06` | Formatter and complete summary regressions with more than six tools | +| `REQ-USAGE-B` | #142 result/privacy boundary | Only bounded names and integer counters cross into GitLab; arguments, results, paths, IDs, provider data, and unknown/unattributed names remain private | `WQ-06` | Hostile map/name/count, DLP, Markdown, and note-budget regressions | +| `REQ-USAGE-C` | #142 honest token explanation | Tool activity appears beside aggregate token usage without percentages or per-tool token attribution | `WQ-06` | Rendering and documentation assertions | +| `REQ-USAGE-D` | #142 existing trust contracts | The expanded diagnostic is not a finding, receipt proof, telemetry source, severity input, or approval signal | `WQ-06`, `WQ-07` | Posting, DLP, and approval regression review | | `REQ-REL` | Repository release contract | Deliver 0.8.3 through reviewed feature and release PRs with external reconciliation | `WQ-01`, `WQ-04`, `WQ-05` | Protected checks, registries, attestations, tag, receipt, issue/milestone closure | #### Explicit Non-Goals @@ -165,8 +165,8 @@ semantics. | `WQ-03` | `done` | #140: example sentinel `0`; exact normalization parser and operator-only notice; behavioral numeric/effective-loop qualification; updated exact OCR evidence/hash; full enriched/MCP preview regression; public/development contracts and `140.bugfix.md` | | `WQ-04` | `done` | Reconciled public/development contracts and requirements; complete quality/coverage, manifest, Towncrier, and privacy/data-flow review are green; final Draft push and hosted readback are handoff actions, not release delivery | | `WQ-05` | `out_of_scope` | Owner-deferred delivery: keep PR #141 Draft after final push; do not merge, publish TestPyPI/PyPI, prepare a release PR, tag, close issues, or close the milestone in this run | -| `WQ-06` | `in_progress` | Create the tool-usage issue in milestone `v0.8.3`; extend the existing inline formatter to every admitted useful non-zero counter; keep external MCP aggregated by verified server; add focused boundary tests, public operational wording, and a separate feature changelog fragment | -| `WQ-07` | `pending` | Complete focused and final validation, self-review the reporting/privacy/approval flow, update the Draft body and issue checkboxes, make one signed feature commit plus one final push, and wait for exact-head hosted checks while keeping the PR Draft | +| `WQ-06` | `done` | #142: the existing inline formatter now shows every admitted useful non-zero counter; external MCP stays aggregated by verified server; focused hostile-value/DLP/approval tests, public operational wording, and `142.feature.md` define the activity-not-token-attribution contract | +| `WQ-07` | `in_progress` | Complete the one final full validation and overall self-review, record exact results, update the Draft body and issue checkboxes, make one signed handoff commit plus one final push, and wait for exact-head hosted checks while keeping the PR Draft | #### Locked Decisions @@ -254,6 +254,15 @@ semantics. - The user then approved an additive numeric tool-activity breakdown in the existing format. No repository or GitHub write for that new slice preceded this resumed-plan materialization. +- #142 now owns the additive feature under milestone `v0.8.3`. Focused formatter, + posting, result-DLP, and approval validation passes 291 tests plus 133 subtests; + documentation/release validation passes 38 tests and the rendered Towncrier + draft keeps #142 under Features with #139/#140 separately under Bug Fixes. +- Feature self-review confirms only the nine compile-time native/context/evidence + labels and bounded positive integer counts can enter the inline line. Unknown + or dynamic names remain private, static labels cannot create DLP false + positives, filtered projections retain safe native counts, and the aggregate + token line plus receipt/approval authorities are unchanged. #### Risks And Recovery @@ -282,10 +291,10 @@ semantics. #### Resume Point -Continue at `WQ-06`: create the scoped GitHub issue and attach it to milestone -`v0.8.3`, then implement the closed useful-counter projection in the existing -formatter and its focused tests. Do not change the format, push, merge, or start -release delivery before the new logical slice and `WQ-07` gates are complete. +Continue at `WQ-07`: commit the reviewed #142 feature slice, run the one final +full local gate on that exact implementation, record its evidence in this plan, +then make the handoff commit and single final push. Do not change the format, +merge, or start release delivery. #### Plan Fidelity Check @@ -301,7 +310,7 @@ release delivery before the new logical slice and `WQ-07` gates are complete. #### Reconciliation Check - [x] `PLANS.md` was inactive and the worktree was clean at activation. -- [x] #139 and #140 are the complete open issue set and do not conflict with backlog/roadmap scope. +- [x] #139, #140, and #142 are the complete open release issue set and do not conflict with backlog/roadmap scope. - [x] Stable 0.8.2 closure remains historical and is not rewritten. - [x] Target 0.8.3 matches `.next-version` and no competing milestone exists. - [x] The new reporting slice extends the open Draft and milestone without @@ -325,8 +334,8 @@ release delivery before the new logical slice and `WQ-07` gates are complete. - #139/#140 and milestone `v0.8.3` remain open until stable receipt publication. - Final handoff must repeat that no local real-LLM/provider qualification was run or claimed, while identifying the exact deterministic OCR boundary evidence. -- The resumed slice needs one new feature issue and changelog fragment; #139 and - #140 stay checked/open and must not absorb this manager-facing reporting change. +- #142 and its feature fragment separately own the manager-facing reporting + change; #139 and #140 stay checked/open and do not absorb it. - Final handoff must state that tool-call counts explain review activity only; exact per-tool token consumption remains unavailable in OCR 1.10.0. diff --git a/changelog.d/142.feature.md b/changelog.d/142.feature.md new file mode 100644 index 0000000..4b7711b --- /dev/null +++ b/changelog.d/142.feature.md @@ -0,0 +1,4 @@ +GitLab technical details now make OCR review activity easier to explain without implying unsupported token attribution. + +- **Added:** the existing inline `all OCR tool calls` line shows every non-zero counter from the closed OCR inspection, review-output, and toolkit context/evidence set instead of collapsing entries after the six most frequent tools. +- **Unchanged:** aggregate input/output/cache token usage remains separate because OCR does not report per-tool token consumption; raw call arguments, results, dynamic external MCP tool names, receipt v5, DLP, findings, severity, and automatic approval are unchanged. diff --git a/docs/gitlab.md b/docs/gitlab.md index 4ac1bbb..e900aec 100644 --- a/docs/gitlab.md +++ b/docs/gitlab.md @@ -22,6 +22,8 @@ The public pipeline stores the OCR binary checksum as the non-secret `OCR_SHA256 OCR 1.10.0 semantically groups related changed files before review and filters candidates per group. Grouping, filtering, and multiple rounds can increase provider requests, latency, and token cost, while the manifest and aggregate budget continue to report completeness. Group labels and path-derived keys are untrusted private OCR diagnostics; the toolkit does not publish or use them for severity, fingerprints, lifecycle commands, receipts, telemetry, or approval. OCR's separately configured telemetry may export those high-cardinality repository-derived values, so keep `OCR_TELEMETRY_ENABLED=false` unless the exporter and retention policy are approved. +The collapsed technical details keep aggregate input/output/cache token usage separate from OCR tool activity. The existing `all OCR tool calls` line lists every non-zero count for the closed review set (`file_read`, `file_read_diff`, `file_find`, `code_search`, `code_comment`, `task_done`, `ocr_toolkit_evidence`, `context_list`, and `context_get`) and is omitted when that admitted list is empty. These counts help explain whether OCR read context, searched the repository, checked cross-file diffs, consulted toolkit evidence, or emitted review output; they are not per-tool token attribution. Dynamic external MCP tool names remain private and appear only through the existing verified per-server aggregate. + ## Choose one operating mode `OCR_REVIEW_CONTEXT_MODE` selects `off`, `metadata`, or `enriched`; the mode recipes pair that selector with the appropriate approval, adapter, and direct-MCP controls. diff --git a/docs/operations.md b/docs/operations.md index 1b4fb7a..d3b6112 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -38,7 +38,7 @@ This aggregate budget is separate from both OCR's prompt/context `max_tokens` ce `OCR_REVIEW_EFFORT=medium` is the toolkit default for the qualified OCR release and permits two review rounds; `low` permits one and `high` permits three. This is a review-depth choice, not one of the three token controls. OCR first groups related changed files and may make group-filter requests; additional rounds can add requests, latency, and cost, but may stop early when they add no finding. The published GitLab example separately passes `OCR_MAX_TOOLS=0`, delegating the effective per-file tool-call limit to the installed OCR template instead of carrying a release-specific minimum. A positive value can raise that limit, but operators should check the release's behavioral qualification because CLI help, normalization text, and effective template value can differ. Exhausting the effective limit, an aggregate budget stop, or incomplete manifest coverage remains explicit and approval-ineligible; increasing either effort or tools is never a way to hide partial coverage. -The outcome wording distinguishes skipped, complete, complete-with-warnings, incomplete, token-budget, and failed reviews while preserving the finding state in that same line. A complete clean review is visibly positive; a complete review with findings or only reviewer-suppressed findings is neutral; warning, partial, budget, and failed states never look clean. Findings withheld by the posting limit remain counted even when the limit allows no individual finding note. Recommended focus areas ranks only its copy of already-published findings by the closed severity, category, safe repository location, and stable-identity order before its existing display cap; inline and fallback discussion order, suppression, counts, security focus, and approval policy remain unchanged. OCR 1.8.5 and later manifest failures provide the canonical failed-file receipt; legacy warnings are a bounded fallback, and `summary.files_reviewed` is never treated as proof of successful coverage. Technical details label the aggregate as all OCR tool calls, separately label toolkit-verified MCP-server calls, and show built-in evidence `summary`/`list`/`get` counts only after exact reconciliation; unavailable attribution is not displayed as zero. Zero-valued token counters and configured-but-unused MCP servers are omitted. Token usage renders only validated input/output/cached/reasoning/total/derived-other buckets; malformed or contradictory counters are unavailable and unknown provider keys are not published. Status and aggregate semantic-category emoji are enabled by default and can be disabled together with `OCR_POST_EMOJI=false`; finding labels remain text unless their separate badge mode is enabled. +The outcome wording distinguishes skipped, complete, complete-with-warnings, incomplete, token-budget, and failed reviews while preserving the finding state in that same line. A complete clean review is visibly positive; a complete review with findings or only reviewer-suppressed findings is neutral; warning, partial, budget, and failed states never look clean. Findings withheld by the posting limit remain counted even when the limit allows no individual finding note. Recommended focus areas ranks only its copy of already-published findings by the closed severity, category, safe repository location, and stable-identity order before its existing display cap; inline and fallback discussion order, suppression, counts, security focus, and approval policy remain unchanged. OCR 1.8.5 and later manifest failures provide the canonical failed-file receipt; legacy warnings are a bounded fallback, and `summary.files_reviewed` is never treated as proof of successful coverage. Technical details label the aggregate as all OCR tool calls and retain the existing inline format while listing every admitted non-zero count for the closed native/context/evidence review set. An empty admitted list produces no tool-call line. The counts describe review activity, not per-tool token consumption: one read or search can return a different amount of context from another. Dynamic external tool names remain private; toolkit-verified MCP-server calls stay in their separate aggregate, and built-in evidence `summary`/`list`/`get` counts appear only after exact reconciliation. Zero-valued token counters and configured-but-unused MCP servers are omitted. Token usage renders only validated input/output/cached/reasoning/total/derived-other buckets; malformed or contradictory counters are unavailable and unknown provider keys are not published. Status and aggregate semantic-category emoji are enabled by default and can be disabled together with `OCR_POST_EMOJI=false`; finding labels remain text unless their separate badge mode is enabled. ## Automatic approval lifecycle diff --git a/src/ocr_toolkit/ocr_result.py b/src/ocr_toolkit/ocr_result.py index cd3f85d..908852d 100644 --- a/src/ocr_toolkit/ocr_result.py +++ b/src/ocr_toolkit/ocr_result.py @@ -27,6 +27,23 @@ MAX_TOOLKIT_MCP_USAGE_COUNT = 1_000_000_000 TOOLKIT_MCP_SERVER_NAME_RE = re.compile(r"^[A-Za-z0-9_-]{1,64}$") +# Closed names whose numeric call counts may appear in the public review summary. +# Dynamic external MCP tools remain represented by the receipt's verified +# per-server aggregate so provider-controlled names do not cross into GitLab. +PUBLIC_REVIEW_TOOL_CALL_NAMES = frozenset( + { + "code_comment", + "code_search", + "context_get", + "context_list", + "file_find", + "file_read", + "file_read_diff", + "ocr_toolkit_evidence", + "task_done", + } +) + class OcrResultMissing(Exception): """The OCR result artifact is missing or unreadable on disk.""" diff --git a/src/ocr_toolkit/posting/formatting.py b/src/ocr_toolkit/posting/formatting.py index 299cc94..8974271 100644 --- a/src/ocr_toolkit/posting/formatting.py +++ b/src/ocr_toolkit/posting/formatting.py @@ -20,6 +20,7 @@ from ocr_toolkit.ocr_result import ( MAX_TOOLKIT_MCP_USAGE_COUNT, MAX_TOOLKIT_MCP_USAGE_SERVERS, + PUBLIC_REVIEW_TOOL_CALL_NAMES, SUPPORTED_TOOLKIT_RESULT_SCHEMA_VERSIONS, TOOLKIT_MCP_SERVER_NAME_RE, ) @@ -45,8 +46,6 @@ MAX_REVIEWER_GUIDE_LABEL_CHARS, MAX_REVIEWER_GUIDE_LOCATION_CHARS, MAX_REVIEWER_GUIDE_TEXT_CHARS, - MAX_TOOL_CALL_NAME_CHARS, - MAX_TOOL_CALL_SUMMARY_TOOLS, SUGGESTION_HEADER, post_badges, post_emoji, @@ -374,32 +373,25 @@ def nonnegative_int(value: Any) -> int | None: return None -def truncate_tool_call_name(name: str) -> str: - """Return a compact tool name for one-line MR summaries.""" - - if len(name) <= MAX_TOOL_CALL_NAME_CHARS: - return name - - return name[: MAX_TOOL_CALL_NAME_CHARS - 3].rstrip() + "..." - - def tool_call_name(value: Any) -> str: - """Extract a displayable tool name from common OCR tool-call shapes.""" + """Extract one closed public tool name from common OCR call shapes.""" if isinstance(value, str): - return clean_text(value) + name = clean_text(value) + return name if name in PUBLIC_REVIEW_TOOL_CALL_NAMES else "" if not isinstance(value, dict): return "" for key in ("name", "tool", "tool_name"): name = clean_text(value.get(key)) - if name: + if name in PUBLIC_REVIEW_TOOL_CALL_NAMES: return name function_value = value.get("function") if isinstance(function_value, dict): - return clean_text(function_value.get("name")) + name = clean_text(function_value.get("name")) + return name if name in PUBLIC_REVIEW_TOOL_CALL_NAMES else "" return "" @@ -407,14 +399,17 @@ def tool_call_name(value: Any) -> str: def tool_call_counts_from_items( items: list[Any], ) -> tuple[int | None, list[tuple[str, int]]]: - """Summarize a list-style OCR tool_calls payload.""" + """Summarize admitted calls from a legacy list-style OCR payload.""" counts: dict[str, int] = {} for item in items: name = tool_call_name(item) if not name: continue - counts[name] = counts.get(name, 0) + 1 + count = counts.get(name, 0) + 1 + if count > MAX_TOOLKIT_MCP_USAGE_COUNT: + return None, [] + counts[name] = count total = sum(counts.values()) if total == 0 and items: @@ -424,51 +419,52 @@ def tool_call_counts_from_items( def format_tool_calls_summary(tool_calls: Any) -> str: - """Return one bounded MR summary line for OCR tool-call statistics.""" + """Return one bounded MR line for admitted non-zero OCR tool counts.""" entries: list[tuple[str, int]] total: int | None - scalar_total = nonnegative_int(tool_calls) - if scalar_total is not None: - total = scalar_total - entries = [] - elif isinstance(tool_calls, list): + if isinstance(tool_calls, list): total, entries = tool_call_counts_from_items(tool_calls) elif isinstance(tool_calls, dict): by_tool_value = tool_calls.get("by_tool") entries = [] - by_tool_total = 0 - valid_by_tool_count = False + admitted_total = 0 if isinstance(by_tool_value, dict): for raw_name, raw_count in by_tool_value.items(): - count = nonnegative_int(raw_count) - if count is None: + if not isinstance(raw_name, str) or raw_name not in PUBLIC_REVIEW_TOOL_CALL_NAMES: continue - name = clean_text(raw_name) - if not name: + if ( + not isinstance(raw_count, int) + or isinstance(raw_count, bool) + or not 0 < raw_count <= MAX_TOOLKIT_MCP_USAGE_COUNT + ): continue - valid_by_tool_count = True - by_tool_total += count - if count > 0: - entries.append((name, count)) + admitted_total += raw_count + if admitted_total > MAX_TOOLKIT_MCP_USAGE_COUNT: + return "" + entries.append((raw_name, raw_count)) calls_value = tool_calls.get("calls") - if valid_by_tool_count: - list_total = None - elif isinstance(calls_value, list): + if not by_tool_value and isinstance(calls_value, list): list_total, entries = tool_call_counts_from_items(calls_value) else: list_total = None - total = nonnegative_int(tool_calls.get("total")) - if total is None: - if valid_by_tool_count: - total = by_tool_total - elif list_total is not None: + if "total" in tool_calls: + raw_total = tool_calls["total"] + if ( + not isinstance(raw_total, int) + or isinstance(raw_total, bool) + or not 0 < raw_total <= MAX_TOOLKIT_MCP_USAGE_COUNT + ): + return "" + total = raw_total + else: + if list_total is not None: total = list_total - elif by_tool_value == {}: - total = 0 + elif entries: + total = admitted_total else: return "" else: @@ -476,21 +472,14 @@ def format_tool_calls_summary(tool_calls: Any) -> str: if total is None: return "" - if total == 0: + if total == 0 or not entries: + return "" + if sum(count for _name, count in entries) > total: return "" line = f"- all OCR tool calls: {total} total" - if not entries: - return line - entries.sort(key=lambda item: (-item[1], item[0])) - shown_entries = entries[:MAX_TOOL_CALL_SUMMARY_TOOLS] - detail_parts = [ - f"{inline_code(truncate_tool_call_name(name))}: {count}" for name, count in shown_entries - ] - omitted_entries = len(entries) - len(shown_entries) - if omitted_entries > 0: - detail_parts.append(f"+{omitted_entries} more") + detail_parts = [f"{inline_code(name)}: {count}" for name, count in entries] return f"{line} ({', '.join(detail_parts)})" diff --git a/src/ocr_toolkit/posting/settings.py b/src/ocr_toolkit/posting/settings.py index d5423f4..3929955 100644 --- a/src/ocr_toolkit/posting/settings.py +++ b/src/ocr_toolkit/posting/settings.py @@ -53,12 +53,6 @@ MAX_POST_COMMENTS_HARD_LIMIT = 200 -MAX_TOOL_CALL_SUMMARY_TOOLS = 6 - - -MAX_TOOL_CALL_NAME_CHARS = 48 - - MAX_REVIEWER_GUIDE_COMMENTS = 6 diff --git a/src/ocr_toolkit/review_runner.py b/src/ocr_toolkit/review_runner.py index 45ae474..542e05b 100644 --- a/src/ocr_toolkit/review_runner.py +++ b/src/ocr_toolkit/review_runner.py @@ -75,6 +75,7 @@ from ocr_toolkit.evidence.store import EvidenceStore, EvidenceStoreError from ocr_toolkit.ocr_result import ( MAX_TOOLKIT_MCP_USAGE_COUNT, + PUBLIC_REVIEW_TOOL_CALL_NAMES, TOOLKIT_RESULT_KEY, TOOLKIT_RESULT_SCHEMA_VERSION, OcrResultMalformed, @@ -598,22 +599,6 @@ def _publication_sinks(payload: dict[str, object]) -> list[object]: warnings = payload.get("warnings") if warnings is not None: sinks.append(warnings) - tool_calls = payload.get("tool_calls") - if isinstance(tool_calls, list): - for item in tool_calls: - if not isinstance(item, dict): - continue - sinks.extend(item.get(key) for key in ("name", "tool", "tool_name") if key in item) - function = item.get("function") - if isinstance(function, dict) and "name" in function: - sinks.append(function.get("name")) - elif isinstance(tool_calls, dict): - by_tool = tool_calls.get("by_tool") - if isinstance(by_tool, dict): - sinks.extend(by_tool) - calls = tool_calls.get("calls") - if isinstance(calls, list): - sinks.extend(_publication_sinks({"tool_calls": calls})) manifest = payload.get("manifest") coverage = manifest.get("coverage") if isinstance(manifest, dict) else None failed = coverage.get("failed") if isinstance(coverage, dict) else None @@ -638,23 +623,6 @@ def _is_publication_sink_path(path: tuple[object, ...]) -> bool: and path[2] in QUARANTINE_COMMENT_FIELDS ): return True - if len(path) == 3 and path[:2] == ("tool_calls", "by_tool"): - return True - if ( - len(path) == 4 - and path[0] == "tool_calls" - and isinstance(path[1], int) - and path[2] == "function" - and path[3] == "name" - ): - return True - if ( - len(path) == 3 - and path[0] == "tool_calls" - and isinstance(path[1], int) - and path[2] in {"name", "tool", "tool_name"} - ): - return True return bool( len(path) >= 5 and path[:3] == ("manifest", "coverage", "failed") @@ -663,6 +631,16 @@ def _is_publication_sink_path(path: tuple[object, ...]) -> bool: ) +def _is_static_public_tool_key_path(path: tuple[object, ...]) -> bool: + """Return whether a map key is one compile-time public tool label.""" + + return bool( + len(path) == 3 + and path[:2] == ("tool_calls", "by_tool") + and path[2] in PUBLIC_REVIEW_TOOL_CALL_NAMES + ) + + def _sanitize_nonpublication_fields( payload: dict[str, object], *, budgets: TextBudgets, matcher: ForbiddenMatcher ) -> tuple[dict[str, object], Counter[str], int]: @@ -684,7 +662,9 @@ def _sanitize_nonpublication_fields( child_path = (*path, key) if isinstance(source, dict): assert isinstance(key, str) - if not _is_publication_sink_path(child_path): + if not _is_publication_sink_path( + child_path + ) and not _is_static_public_tool_key_path(child_path): checked_key = check_text( key, budgets=budgets, @@ -788,6 +768,7 @@ def _closed_tool_calls(value: object, *, allowed_tools: frozenset[str]) -> dict[ tool: count for tool, count in by_tool.items() if tool in allowed_tools + and tool in PUBLIC_REVIEW_TOOL_CALL_NAMES and isinstance(count, int) and not isinstance(count, bool) and 0 < count <= MAX_TOOLKIT_MCP_USAGE_COUNT @@ -814,17 +795,9 @@ def _canonical_result_projection(payload: dict[str, object]) -> bytes: else comment for comment in comments ] - tool_calls = payload.get("tool_calls") - projected_tool_calls: object - if isinstance(tool_calls, dict): - total = tool_calls.get("total") - by_tool = tool_calls.get("by_tool") - projected_tool_calls = { - "total": total, - "by_tool": (dict(sorted(by_tool.items())) if isinstance(by_tool, dict) else by_tool), - } - else: - projected_tool_calls = tool_calls + projected_tool_calls = _closed_tool_calls( + payload.get("tool_calls"), allowed_tools=PUBLIC_REVIEW_TOOL_CALL_NAMES + ) projection = { "outcome": { "status": outcome.status, @@ -958,9 +931,7 @@ def _finalize_ocr_result( filtered = False publication: dict[str, object] = {"state": "passed"} usage: dict[str, int] = {} - allowed_tools = frozenset( - tool for capability in composition.capabilities for tool in capability.tools - ) + allowed_tools = PUBLIC_REVIEW_TOOL_CALL_NAMES def finalize(payload: dict[str, object]) -> dict[str, object]: nonlocal filtered, publication, usage diff --git a/tests/test_posting_helpers.py b/tests/test_posting_helpers.py index 7f68cf6..c5cc16e 100644 --- a/tests/test_posting_helpers.py +++ b/tests/test_posting_helpers.py @@ -2469,6 +2469,8 @@ def test_post_emoji_defaults_on_and_accepts_explicit_false(self) -> None: self.assertFalse(settings.post_emoji()) def test_tool_calls_prefers_calls_when_by_tool_is_empty(self) -> None: + """Keep the bounded legacy list fallback for admitted tool names.""" + summary = posting_formatting.format_tool_calls_summary( {"by_tool": {}, "calls": [{"name": "file_read"}, {"tool": "code_search"}]} ) @@ -2477,6 +2479,106 @@ def test_tool_calls_prefers_calls_when_by_tool_is_empty(self) -> None: self.assertIn("`file_read`: 1", summary) self.assertIn("`code_search`: 1", summary) + def test_tool_calls_reports_every_useful_nonzero_counter_inline(self) -> None: + """Expose the complete useful activity breakdown without changing its format.""" + + summary = posting_formatting.format_tool_calls_summary( + { + "total": 36, + "by_tool": { + "task_done": 1, + "file_find": 1, + "context_list": 1, + "file_read_diff": 2, + "context_get": 3, + "code_comment": 4, + "ocr_toolkit_evidence": 5, + "code_search": 7, + "file_read": 12, + }, + } + ) + + self.assertEqual( + summary, + "- all OCR tool calls: 36 total " + "(`file_read`: 12, `code_search`: 7, `ocr_toolkit_evidence`: 5, " + "`code_comment`: 4, `context_get`: 3, `file_read_diff`: 2, " + "`context_list`: 1, `file_find`: 1, `task_done`: 1)", + ) + self.assertNotIn("more", summary) + + def test_tool_calls_omits_empty_zero_or_unknown_breakdowns(self) -> None: + """Do not emit a technical line without a positive admitted counter list.""" + + cases = ( + None, + 7, + [], + {"total": 0, "by_tool": {}}, + {"total": 5, "by_tool": {}}, + {"total": 5, "by_tool": {"file_read": 0}}, + {"total": 5, "by_tool": {"dynamic_external_tool": 5}}, + [{"name": "dynamic_external_tool"}], + ) + + for value in cases: + with self.subTest(value=value): + self.assertEqual(posting_formatting.format_tool_calls_summary(value), "") + + def test_tool_calls_ignores_invalid_counts_and_rejects_invalid_totals(self) -> None: + """Admit only bounded integer counters under a consistent positive total.""" + + summary = posting_formatting.format_tool_calls_summary( + { + "total": 2, + "by_tool": { + "file_read": 2, + "code_search": True, + "file_find": -1, + "task_done": "1", + "context_get": 1.0, + "context_list": ocr_result.MAX_TOOLKIT_MCP_USAGE_COUNT + 1, + }, + } + ) + + self.assertEqual(summary, "- all OCR tool calls: 2 total (`file_read`: 2)") + invalid_totals = (False, 0, -1, "2", 2.0, ocr_result.MAX_TOOLKIT_MCP_USAGE_COUNT + 1) + for total in invalid_totals: + with self.subTest(total=total): + self.assertEqual( + posting_formatting.format_tool_calls_summary( + {"total": total, "by_tool": {"file_read": 1}} + ), + "", + ) + self.assertEqual( + posting_formatting.format_tool_calls_summary({"total": 1, "by_tool": {"file_read": 2}}), + "", + ) + + def test_tool_and_token_activity_remain_separate_technical_lines(self) -> None: + """Do not imply that aggregate token usage is attributable to tool counters.""" + + summary = posting_formatting.summarize_result( + total=0, + inline_count=0, + fallback_count=0, + warning_count=0, + outcome_status="success", + outcome_message="No comments generated. Looks good to me.", + tool_calls_summary=posting_formatting.format_tool_calls_summary( + {"total": 2, "by_tool": {"file_read": 2}} + ), + token_usage_summary="- token usage: 48 total (input: 40, output: 8)", + emoji=True, + ) + + self.assertIn("- all OCR tool calls: 2 total (`file_read`: 2)", summary) + self.assertIn("- token usage: 48 total (input: 40, output: 8)", summary) + self.assertNotIn("per-tool", summary) + def test_root_total_is_not_treated_as_token_usage(self) -> None: self.assertEqual(posting_formatting.format_token_usage_summary({"total": 17}), "") self.assertIn( diff --git a/tests/test_review_runner.py b/tests/test_review_runner.py index 9d02dc9..36cc897 100644 --- a/tests/test_review_runner.py +++ b/tests/test_review_runner.py @@ -811,7 +811,7 @@ def test_publication_dlp_retains_only_safe_local_findings_and_closed_receipt( assert persisted["warnings"] == ["Safe bounded warning."] assert persisted["tool_calls"] == { "total": 2, - "by_tool": {"ocr_toolkit_evidence": 1}, + "by_tool": {"ocr_toolkit_evidence": 1, "task_done": 1}, } assert persisted["_ocr_toolkit"]["publication"] == publication assert publication["retained"] == {"comments": 2, "warnings": 1} @@ -1189,10 +1189,6 @@ def test_warning_objects_are_conservatively_publication_relevant() -> None: [ ("message", "Authorization: Bearer synthetic-secret-token"), ("warnings", ["private warning detail"]), - ( - "tool_calls", - {"total": 1, "by_tool": {"private-tool-name": 1}}, - ), ], ) def test_unsafe_displayed_result_units_are_partial(field: str, value: object) -> None: @@ -1217,6 +1213,91 @@ def test_unsafe_displayed_result_units_are_partial(field: str, value: object) -> assert "private-tool-name" not in serialized +def test_unknown_tool_name_is_private_sanitized_without_blocking_publication() -> None: + """Keep unlisted tool names private without changing review or approval inputs.""" + + payload: dict[str, object] = { + "status": "success", + "comments": [], + "warnings": [], + "tool_calls": { + "total": 2, + "by_tool": {"ocr_toolkit_evidence": 1, "private-tool-name": 1}, + }, + } + baseline = review_runner._canonical_result_projection(payload) + + projected, publication, blocked = review_runner._publication_projection( + payload, + forbidden=("private-tool-name",), + allowed_tools=review_runner.PUBLIC_REVIEW_TOOL_CALL_NAMES, + ) + + assert blocked is False + assert publication["state"] == "private-sanitized" + assert projected["status"] == "success" + assert projected["tool_calls"] == { + "total": 2, + "by_tool": {"ocr_toolkit_evidence": 1}, + } + assert "private-tool-name" not in json.dumps(projected) + assert review_runner._canonical_result_projection(projected) == baseline + + +def test_closed_tool_names_do_not_create_dlp_or_approval_false_positives() -> None: + """Treat compile-time tool labels as labels rather than MR-controlled text.""" + + payload: dict[str, object] = { + "status": "success", + "comments": [], + "warnings": [], + "tool_calls": { + "total": 2, + "by_tool": {"file_read": 1, "ocr_toolkit_evidence": 1}, + }, + } + + projected, publication, blocked = review_runner._publication_projection( + payload, + forbidden=("file_read",), + allowed_tools=review_runner.PUBLIC_REVIEW_TOOL_CALL_NAMES, + ) + + assert projected is payload + assert publication == {"state": "passed"} + assert blocked is False + + +def test_invalid_closed_tool_counter_is_private_sanitized() -> None: + """Sanitize a hostile non-numeric counter without publishing or blocking it.""" + + payload: dict[str, object] = { + "status": "success", + "comments": [], + "warnings": [], + "tool_calls": { + "total": 2, + "by_tool": { + "file_read": "Authorization: Bearer synthetic-tool-token", + "ocr_toolkit_evidence": 1, + }, + }, + } + + projected, publication, blocked = review_runner._publication_projection( + payload, + forbidden=(), + allowed_tools=review_runner.PUBLIC_REVIEW_TOOL_CALL_NAMES, + ) + + assert blocked is False + assert publication["state"] == "private-sanitized" + assert "synthetic-tool-token" not in json.dumps(projected) + assert review_runner._canonical_result_projection(projected) == ( + review_runner._canonical_result_projection(payload) + ) + + def test_unsafe_manifest_failure_detail_is_partial_and_destroyed() -> None: payload: dict[str, object] = { "status": "partial", From 62f5bd7bad0343d3d2ec0bedff1d26aff12b6a29 Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:55:56 +0200 Subject: [PATCH 7/8] Record OCR tool usage Draft handoff --- PLANS.md | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/PLANS.md b/PLANS.md index 7c23eec..43b5845 100644 --- a/PLANS.md +++ b/PLANS.md @@ -113,7 +113,7 @@ semantics. #### Inputs And Sources -- GitHub issues #139 and #140, including their complete acceptance criteria. +- GitHub issues #139, #140, and #142, including their complete acceptance criteria. - `.github/workflows/ocr-compatibility.yml`, `scripts/ocr_compat.py`, `src/ocr_toolkit/review_runner.py`, current focused tests and public contracts. - `docs/engineering/project_principles.md` persisted/atomic-state, external-format, @@ -165,8 +165,8 @@ semantics. | `WQ-03` | `done` | #140: example sentinel `0`; exact normalization parser and operator-only notice; behavioral numeric/effective-loop qualification; updated exact OCR evidence/hash; full enriched/MCP preview regression; public/development contracts and `140.bugfix.md` | | `WQ-04` | `done` | Reconciled public/development contracts and requirements; complete quality/coverage, manifest, Towncrier, and privacy/data-flow review are green; final Draft push and hosted readback are handoff actions, not release delivery | | `WQ-05` | `out_of_scope` | Owner-deferred delivery: keep PR #141 Draft after final push; do not merge, publish TestPyPI/PyPI, prepare a release PR, tag, close issues, or close the milestone in this run | -| `WQ-06` | `done` | #142: the existing inline formatter now shows every admitted useful non-zero counter; external MCP stays aggregated by verified server; focused hostile-value/DLP/approval tests, public operational wording, and `142.feature.md` define the activity-not-token-attribution contract | -| `WQ-07` | `in_progress` | Complete the one final full validation and overall self-review, record exact results, update the Draft body and issue checkboxes, make one signed handoff commit plus one final push, and wait for exact-head hosted checks while keeping the PR Draft | +| `WQ-06` | `done` | #142 feature commit `a416f37`: the existing inline formatter now shows every admitted useful non-zero counter; external MCP stays aggregated by verified server; focused hostile-value/DLP/approval tests, public operational wording, and `142.feature.md` define the activity-not-token-attribution contract | +| `WQ-07` | `done` | One final full local gate and overall reporting/privacy/approval self-review are green; this signed handoff commit is followed by one final push, Draft/issue coordination, and exact-head hosted readback as delivery evidence rather than another repository-content change | #### Locked Decisions @@ -254,15 +254,22 @@ semantics. - The user then approved an additive numeric tool-activity breakdown in the existing format. No repository or GitHub write for that new slice preceded this resumed-plan materialization. -- #142 now owns the additive feature under milestone `v0.8.3`. Focused formatter, - posting, result-DLP, and approval validation passes 291 tests plus 133 subtests; - documentation/release validation passes 38 tests and the rendered Towncrier - draft keeps #142 under Features with #139/#140 separately under Bug Fixes. +- #142 now owns the additive feature under milestone `v0.8.3`. The combined + focused formatter, posting, result-DLP, approval, documentation, and release + gate passes 330 tests plus 133 subtests; the rendered Towncrier draft keeps + #142 under Features with #139/#140 separately under Bug Fixes. - Feature self-review confirms only the nine compile-time native/context/evidence labels and bounded positive integer counts can enter the inline line. Unknown or dynamic names remain private, static labels cannot create DLP false positives, filtered projections retain safe native counts, and the aggregate token line plus receipt/approval authorities are unchanged. +- Final local quality passes 1,275 tests plus 324 subtests at 86.35% combined + branch coverage. Risk groups pass at 84%, 82%, 86%, and 87% against floors + 80%, 80%, 85%, and 85%; Ruff format/check, strict MyPy, and Bandit pass. + Lock and OCR-manifest validation, the rendered Towncrier draft, + checksum-verified temporary Gitleaks 8.24.3, `git diff --check`, and clean-tree + confirmation also pass. The global Gitleaks installation and OCR/LLM state + were not changed. #### Risks And Recovery @@ -291,10 +298,11 @@ semantics. #### Resume Point -Continue at `WQ-07`: commit the reviewed #142 feature slice, run the one final -full local gate on that exact implementation, record its evidence in this plan, -then make the handoff commit and single final push. Do not change the format, -merge, or start release delivery. +After this handoff commit, push the accumulated signed history once, update Draft +PR #141 and #142 with the exact remote head/tree, and wait for protected hosted +checks. If green, the next authorized agent starts from review of that immutable +Draft head rather than repeating local development. Do not merge or start release +delivery. #### Plan Fidelity Check @@ -318,10 +326,10 @@ merge, or start release delivery. #### Closure Gate -- [ ] All implementation requirements and in-scope work items are terminal with current local validation evidence. -- [ ] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. +- [x] All implementation requirements and in-scope work items are terminal with current local validation evidence. +- [x] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. - [ ] Exact feature head is green locally and in protected hosted checks with resolved review threads. -- [ ] PR #141 remains Draft at the exact pushed head; #139/#140 and milestone `v0.8.3` remain open. +- [ ] PR #141 remains Draft at the exact pushed head; #139/#140/#142 and milestone `v0.8.3` remain open. - [x] The active plan retains an exact external-review/release resume point and is not archived before deferred delivery; this repository has no separate plan-lifecycle checker. #### Post-Close Delivery @@ -331,7 +339,7 @@ merge, or start release delivery. - A later owner-authorized continuation must review the exact Draft head, preserve or amend the plan from current state, then use the protected feature/release lifecycle. Readiness is not delivery. -- #139/#140 and milestone `v0.8.3` remain open until stable receipt publication. +- #139/#140/#142 and milestone `v0.8.3` remain open until stable receipt publication. - Final handoff must repeat that no local real-LLM/provider qualification was run or claimed, while identifying the exact deterministic OCR boundary evidence. - #142 and its feature fragment separately own the manager-facing reporting From 6520a53db6b4f9d907735456be43e0b17ec296af Mon Sep 17 00:00:00 2001 From: xeonvs <11463419+xeonvs@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:11:35 +0200 Subject: [PATCH 8/8] Make macOS CI checks advisory --- .github/workflows/ci.yml | 11 +++---- PLANS.md | 54 ++++++++++++++++++++++------------ changelog.d/143.maintenance.md | 1 + docs/development.md | 2 +- tests/test_testpypi_preview.py | 7 ++++- 5 files changed, 49 insertions(+), 26 deletions(-) create mode 100644 changelog.d/143.maintenance.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da8c839..f7c1b48 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,15 +14,16 @@ concurrency: jobs: test: name: test-${{ matrix.os }}-py${{ matrix.python }} + continue-on-error: ${{ matrix.advisory }} strategy: fail-fast: false matrix: include: - - { os: ubuntu-latest, python: "3.12", coverage: false } - - { os: ubuntu-latest, python: "3.13", coverage: false } - - { os: ubuntu-latest, python: "3.14", coverage: true } - - { os: macos-latest, python: "3.12", coverage: false } - - { os: macos-latest, python: "3.14", coverage: false } + - { os: ubuntu-latest, python: "3.12", coverage: false, advisory: false } + - { os: ubuntu-latest, python: "3.13", coverage: false, advisory: false } + - { os: ubuntu-latest, python: "3.14", coverage: true, advisory: false } + - { os: macos-latest, python: "3.12", coverage: false, advisory: true } + - { os: macos-latest, python: "3.14", coverage: false, advisory: true } runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/PLANS.md b/PLANS.md index 43b5845..0eacf20 100644 --- a/PLANS.md +++ b/PLANS.md @@ -9,10 +9,9 @@ Use this file for active or blocked repository work. Update it before implementa Status: `active` Owner: Codex Plan Origin: `resumed` -Release classification: `release-deferred` (the product changes require stable -`0.8.3`, but feature merge and publication are explicitly deferred) +Release classification: `release-required` Target stable version: `0.8.3` -Last Updated: 2026-08-25 +Last Updated: 2026-08-26 #### Goal @@ -57,6 +56,9 @@ semantics. - Treat tool-call counts as operational activity only. Keep aggregate token input/output/cache telemetry alongside them, but do not claim or derive per-tool token consumption because OCR 1.10.0 does not provide that contract. +- Keep the Linux Python 3.12-3.14 matrix, quality, security, dependency, build, + and CodeQL checks release-blocking while making the two macOS endpoint jobs + best-effort diagnostics that cannot delay the priority Linux hotfix release. #### Requirement Traceability @@ -74,6 +76,7 @@ semantics. | `REQ-USAGE-B` | #142 result/privacy boundary | Only bounded names and integer counters cross into GitLab; arguments, results, paths, IDs, provider data, and unknown/unattributed names remain private | `WQ-06` | Hostile map/name/count, DLP, Markdown, and note-budget regressions | | `REQ-USAGE-C` | #142 honest token explanation | Tool activity appears beside aggregate token usage without percentages or per-tool token attribution | `WQ-06` | Rendering and documentation assertions | | `REQ-USAGE-D` | #142 existing trust contracts | The expanded diagnostic is not a finding, receipt proof, telemetry source, severity input, or approval signal | `WQ-06`, `WQ-07` | Posting, DLP, and approval regression review | +| `REQ-MACOS-A` | Owner-authorized hotfix release policy | macOS 3.12 and 3.14 jobs continue to run but are explicitly non-blocking; Linux endpoints and all non-platform release gates remain required | `WQ-08` | Workflow contract test plus live ruleset readback | | `REQ-REL` | Repository release contract | Deliver 0.8.3 through reviewed feature and release PRs with external reconciliation | `WQ-01`, `WQ-04`, `WQ-05` | Protected checks, registries, attestations, tag, receipt, issue/milestone closure | #### Explicit Non-Goals @@ -96,6 +99,9 @@ semantics. of merely available tools, add per-tool token estimates/percentages, or expose raw tool-call arguments, results, errors, paths, request IDs, or dynamic external MCP tool names. +- Do not remove macOS support metadata or stop its endpoint jobs from running; + only their merge-blocking status changes. Do not weaken Linux, coverage, + quality, security, dependency, package, or release-publication gates. #### Constraints @@ -136,8 +142,12 @@ semantics. environment has no access; do not claim that evidence. - Continue the established efficient workflow: logical commits, self-review, one final complete local gate, and protected hosted CI. -- After the final push, keep PR #141 in Draft. Do not merge the feature branch, - publish development/stable artifacts, create a release PR, or release 0.8.3. +- Resume and complete the full stable 0.8.3 lifecycle now: update the existing + Draft, make it ready, merge through protected `main`, publish and independently + verify the stable artifacts, then close the tracked issues and milestone. +- macOS endpoint jobs remain useful best-effort compatibility diagnostics but + must not block the priority Linux hotfix release; all Linux and non-platform + release gates stay mandatory. - The accepted manager-facing question is “where did the review activity go?”: preserve the current inline format, show the selected numeric counters, and explicitly avoid claiming exact per-tool token attribution. @@ -164,9 +174,10 @@ semantics. | `WQ-02` | `done` | #139: outcome-authoritative workflow selection; portable atomic evidence/status/issue-body handoffs; closed late-write recovery; manifest-independent status upsert; regression tests; public contract and `139.bugfix.md` | | `WQ-03` | `done` | #140: example sentinel `0`; exact normalization parser and operator-only notice; behavioral numeric/effective-loop qualification; updated exact OCR evidence/hash; full enriched/MCP preview regression; public/development contracts and `140.bugfix.md` | | `WQ-04` | `done` | Reconciled public/development contracts and requirements; complete quality/coverage, manifest, Towncrier, and privacy/data-flow review are green; final Draft push and hosted readback are handoff actions, not release delivery | -| `WQ-05` | `out_of_scope` | Owner-deferred delivery: keep PR #141 Draft after final push; do not merge, publish TestPyPI/PyPI, prepare a release PR, tag, close issues, or close the milestone in this run | +| `WQ-05` | `in_progress` | Owner-authorized stable delivery: exact-head review and feature merge; development publication readback; protected `release/v0.8.3` PR; stable workflow; independent registry/tag/provenance/attestation/install/receipt verification; issue and milestone closure | | `WQ-06` | `done` | #142 feature commit `a416f37`: the existing inline formatter now shows every admitted useful non-zero counter; external MCP stays aggregated by verified server; focused hostile-value/DLP/approval tests, public operational wording, and `142.feature.md` define the activity-not-token-attribution contract | | `WQ-07` | `done` | One final full local gate and overall reporting/privacy/approval self-review are green; this signed handoff commit is followed by one final push, Draft/issue coordination, and exact-head hosted readback as delivery evidence rather than another repository-content change | +| `WQ-08` | `in_progress` | Make macOS 3.12/3.14 CI entries explicitly best-effort, preserve their execution and all mandatory Linux/non-platform gates, update the workflow contract/docs/changelog, and reconcile the live protected-main ruleset before readiness | #### Locked Decisions @@ -189,6 +200,9 @@ semantics. - Built-in OCR and toolkit-owned context/evidence names may be shown as activity. External dynamic MCP tools remain represented only through the existing verified per-server aggregate; unknown raw names never gain public meaning. +- Linux is the release-priority platform. macOS remains supported and exercised + at both Python endpoints, but its two hosted jobs are advisory rather than + protected-main requirements; failures stay visible for follow-up. #### Verification @@ -211,6 +225,9 @@ semantics. more than six useful counters remain visible, deterministic and bounded; raw call content and unknown names remain absent; aggregate token rendering and approval decisions are unchanged. +- macOS CI policy: workflow-source tests prove both endpoint jobs still exist and + are marked non-blocking while every Linux endpoint remains blocking; live + ruleset readback must omit only the two macOS contexts. #### Latest Validation Results @@ -298,11 +315,10 @@ semantics. #### Resume Point -After this handoff commit, push the accumulated signed history once, update Draft -PR #141 and #142 with the exact remote head/tree, and wait for protected hosted -checks. If green, the next authorized agent starts from review of that immutable -Draft head rather than repeating local development. Do not merge or start release -delivery. +Finish `WQ-08`, push the signed feature head, and require the updated hosted +checks plus exact live-ruleset readback. Then make PR #141 ready and complete +`WQ-05` through feature merge, protected release PR, stable publication, +independent external verification, issue receipts, and milestone closure. #### Plan Fidelity Check @@ -329,16 +345,16 @@ delivery. - [x] All implementation requirements and in-scope work items are terminal with current local validation evidence. - [x] Complete diff self-review confirms issue, workflow, subprocess, privacy, DLP, approval, and documentation boundaries. - [ ] Exact feature head is green locally and in protected hosted checks with resolved review threads. -- [ ] PR #141 remains Draft at the exact pushed head; #139/#140/#142 and milestone `v0.8.3` remain open. -- [x] The active plan retains an exact external-review/release resume point and is not archived before deferred delivery; this repository has no separate plan-lifecycle checker. +- [ ] PR #141 is merged from the exact reviewed head and development publication is verified. +- [ ] Release PR is merged from its exact reviewed head; stable artifacts, tag, + provenance, attestations, immutable Release, installs, and receipt are verified. +- [ ] #139/#140/#142 plus the macOS CI policy issue and milestone `v0.8.3` are closed only after stable receipt publication. #### Post-Close Delivery -- This run ends at a pushed green Draft PR. Feature implementation does not close - through merge, and no TestPyPI/PyPI or stable publication is authorized. -- A later owner-authorized continuation must review the exact Draft head, preserve - or amend the plan from current state, then use the protected feature/release - lifecycle. Readiness is not delivery. +- This run is authorized to complete the protected feature and stable-release + lifecycle. Readiness, feature merge, development publication, release merge, + stable publication, and external reconciliation remain separate gates. - #139/#140/#142 and milestone `v0.8.3` remain open until stable receipt publication. - Final handoff must repeat that no local real-LLM/provider qualification was run or claimed, while identifying the exact deterministic OCR boundary evidence. @@ -349,7 +365,7 @@ delivery. #### Handoff Notes -- Start at `WQ-01`; do not implement from an uncommitted or compressed substitute. +- Resume at `WQ-08` on exact Draft head `62f5bd7`; do not repeat completed feature work. - The source-level max-tools mismatch is material: reported normalization `50` and embedded template default `100` are distinct facts. Preserve that distinction in code, evidence, docs, changelog, issue updates, and future OCR upgrades. diff --git a/changelog.d/143.maintenance.md b/changelog.d/143.maintenance.md new file mode 100644 index 0000000..5ae323c --- /dev/null +++ b/changelog.d/143.maintenance.md @@ -0,0 +1 @@ +Keep both macOS endpoint CI jobs as visible best-effort compatibility diagnostics while retaining Linux, coverage, quality, security, dependency, package, and CodeQL checks as release-blocking gates. diff --git a/docs/development.md b/docs/development.md index 91b3e97..4b8902c 100644 --- a/docs/development.md +++ b/docs/development.md @@ -13,7 +13,7 @@ uv run python -m build uv run twine check dist/* ``` -For routine agent and contributor checks, run focused tests for each logical change and `scripts/quality.sh check` once on the completed handoff head. It replaces the selected mode's prior log, captures current output under ignored `.quality-logs/`, and prints only a short status; on failure it prints the last 80 lines. Individual modes are `format`, `lint`, `test`, `coverage`, `types`, and `security`. The `coverage` and `check` modes reuse that single branch-aware test run, then enforce scoped floors for result/preflight and GitLab posting transactions at 80%, plus review/context/DLP/approval and MCP/provider/policy/result contracts at 85%; a high combined result cannot hide a weak risk group. Hosted pull requests still run the complete suite on all five supported OS/Python combinations, while Ubuntu with the newest supported Python is the sole coverage owner. The Bandit gate scans only the supported runtime package at medium-or-higher severity and confidence; tests and synthetic fixtures are intentionally outside that bounded gate. +For routine agent and contributor checks, run focused tests for each logical change and `scripts/quality.sh check` once on the completed handoff head. It replaces the selected mode's prior log, captures current output under ignored `.quality-logs/`, and prints only a short status; on failure it prints the last 80 lines. Individual modes are `format`, `lint`, `test`, `coverage`, `types`, and `security`. The `coverage` and `check` modes reuse that single branch-aware test run, then enforce scoped floors for result/preflight and GitLab posting transactions at 80%, plus review/context/DLP/approval and MCP/provider/policy/result contracts at 85%; a high combined result cannot hide a weak risk group. Hosted pull requests still run the complete suite on all five supported OS/Python combinations. The three Linux jobs are release-blocking, with Ubuntu on the newest supported Python as the sole coverage owner. The two macOS endpoint jobs are best-effort compatibility diagnostics: they continue to run and remain visible for follow-up, but a platform-only failure does not block a Linux-priority release. The Bandit gate scans only the supported runtime package at medium-or-higher severity and confidence; tests and synthetic fixtures are intentionally outside that bounded gate. Runtime code must remain compatible with Python 3.12-3.14 and standard-library-only. Tests must use synthetic data; public examples must use safe placeholder hosts and credentials while describing the real operating behavior rather than labelling the feature itself as synthetic. User-visible changes require a fragment in `changelog.d/`. Repository-only qualification tools and evidence live under `scripts/` and `compatibility/`; they are excluded from both published distributions. Validate the manifest with `PYTHONPATH=src python scripts/ocr_compat.py validate`. diff --git a/tests/test_testpypi_preview.py b/tests/test_testpypi_preview.py index 7ae163d..8aa29c8 100644 --- a/tests/test_testpypi_preview.py +++ b/tests/test_testpypi_preview.py @@ -276,11 +276,16 @@ def test_distribution_build_is_a_bounded_pull_request_gate() -> None: def test_ci_matrix_covers_supported_python_minors_and_os_boundaries() -> None: - """Run all five functional combinations while collecting coverage once.""" + """Keep Linux blocking and macOS visible but advisory at both endpoints.""" workflow = (PROJECT_ROOT / ".github" / "workflows" / "ci.yml").read_text(encoding="utf-8") assert " push:" not in workflow + assert "continue-on-error: ${{ matrix.advisory }}" in workflow + assert workflow.count("advisory: false") == 3 + assert workflow.count("advisory: true") == 2 + assert workflow.count("os: ubuntu-latest") == 3 + assert workflow.count("os: macos-latest") == 2 assert workflow.count('python: "3.12"') == 2 assert workflow.count('python: "3.13"') == 1 assert workflow.count('python: "3.14"') == 2