diff --git a/.gitignore b/.gitignore index 8f060ad..679125f 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ build/ dist/ .review-context/ .quality-logs/ +.tgrep-index/ .idea/ *.log .codex diff --git a/.next-version b/.next-version index 2003b63..78bc1ab 100644 --- a/.next-version +++ b/.next-version @@ -1 +1 @@ -0.9.2 +0.10.0 diff --git a/PLANS.md b/PLANS.md index 116f44f..965d0fe 100644 --- a/PLANS.md +++ b/PLANS.md @@ -5,3 +5,424 @@ before handoff or commit. Completed stable plans are indexed in [the execution-history archive](docs/engineering/execution_history/README.md). ## Active Work + +### Toolkit 0.10.0 — local provider, diagnostics and OCR controls + +- **Status:** active +- **Draft implementation:** resumed for final qualification and stable delivery +- **Plan Origin:** plan_mode_approved +- **Release classification:** release-required; stable delivery authorized +- **Target stable version:** 0.10.0 (raised from 0.9.2 by user decision) +- **Branch:** `codex/v0.10.0-local-review` +- **PR presentation:** before pushing implementation, replace the planning-only + Draft body with the complete implemented scope, considered boundaries, + validation evidence and remaining external qualification. The user requested + a branch rename if feasible; GitHub closes an open PR when its head branch is + renamed. The user authorized replacing #183: first publish and verify the new + branch and fully described Draft, then close #183 with a replacement link. + Preserve the old remote branch until the replacement is verified. + Replacement Draft #184 is now open on the new branch with the full scope and + evidence description; #183 was closed with its replacement link. GitHub + verifies every published feature commit's SSH signature. +- **Final validation authorization:** retain the completed Draft evidence and + run the one final local OCR review only after the final engineering review. + Preserve every resulting private diagnostic artifact until its outcome is + analyzed. A confirmed finding is fixed with deterministic regressions; no + second OCR run follows that remediation. The owner has authorized protected + merge, publication, release reconciliation, issue and milestone closure. + +#### Goal + +Deliver OCR 1.11.6, a standalone local provider with shared review summaries, +truthful local debug diagnostics, explicit reasoning controls and safe progress, +then complete protected merge, artifact publication, minimal independent +readback, and closure of #181, #182 and milestone v0.10.0. + +#### Baseline And Sources + +Main `8ae890b6f78388554281e649dc8201737679968a` reconciles stable 0.9.1; +the old feature branch is complete. #181 qualifies OCR 1.11.6 against 1.11.5 +in hosted run 34127679854. Upstream #1154 is the product focus; other adjacent +changes are compatibility inputs, not separate product projects. The approved +conversation plan and subsequent provider-support/none clarification are binding. +Local delivery is tracked by #182; both issues belong to milestone v0.10.0. +The user subsequently authorized a renamed branch and replacement Draft for +#183; no commit-history rewrite is needed. +Canonical owners: project principles, development/release guides, configuration, +operations, security, review decision flow, strategy and backlog. At the baseline, +local review verified evidence without a GitLab receipt, while preflight assumed GitLab. + +#### Requirement Traceability + +| Requirement | Work | Verification | +| --- | --- | --- | +| R1 Shared truthful summary and independent local output | WQ-02 | GitLab parity and local outcome matrix | +| R2 Explicit local mode with no GitLab dependency | WQ-03 | installed real Git/process/MCP and hostile CI environment | +| R3 Debug observes actual checks without bypass | WQ-04 | normal/debug parity, provenance, bounds, permissions and failures | +| R4 Protocol-correct reasoning controls | WQ-05 | config/preflight conflicts and real OCR wire probes | +| R5 Privacy-independent progress | WQ-05 | on/off parity, stop/signal/sink failure and bounded output | +| R6 Exact qualified OCR and local binary | WQ-06 | checksums, adjacent audit and no-LLM qualification | +| R7 Green signed Draft and external handoff | WQ-07 | local gates, hosted exact-head checks and remote readback | + +#### Current Work Queue + +| Queue | Status | Work | +| --- | --- | --- | +| WQ-01 | done | Signed planning commit 4e50f78 pushed; Draft #183, milestone and #181/#182 recorded | +| WQ-02 | done | Shared reporting package, characterized GitLab delegation and local Markdown adapter; execution-owner facts remain separate | +| WQ-03 | done | Explicit local preflight/review, private Markdown publication and installed end-to-end path | +| WQ-04 | done | Fresh private debug bundle, actual decision journal and installed normal/debug parity | +| WQ-05 | done | Reasoning controls, actual OCR wire probes and bounded progress with installed parity | +| WQ-06 | done | OCR promotion, verified hosted assets and current no-LLM qualification; installed Darwin binary updated without a post-waiver launch | +| WQ-07 | done | Docs/backlog, full self-review, security review with recorded artifact limitation, green implementation checks and external handoff checklist | +| WQ-08 | active | Final holistic review and retained local OCR review complete; deterministic remediation, protected release lifecycle and minimal external artifact readback remain | + +#### Locked Interfaces And Boundaries + +- Local provider is not a fake GitLab API. Share pure outcome, finding, + coverage, warning, tool/token, verified MCP and DLP reporting. Keep GitLab + posting/suppression/approval and envelopes at its adapter; no fake receipt. + Console Markdown has no HTML disclosure or remote badges, and prints every + admitted finding without posting caps. JSON remains in --result; progress + and diagnostic output use stderr. Every failure has an honest summary. + Keep this core reusable by a future GitHub adapter: no GitLab MR identity, + receipt, API, settings or posting imports in shared report contracts/rendering. + Review health and admitted data are common; publication state, discussion + anchors, suppression and approval remain adapter-owned. GitHub implementation, + credentials and a generic forge API framework are outside this release scope. + User clarification: this is the same toolkit/OCR/LLM/tool-use pipeline as CI, + not a separate local engine. Local publication must persist the complete + admitted Markdown report as an artifact as well as the existing JSON result; + console output is an additional view, not the only report delivery. Keep one + execution/finalization path and put filesystem delivery at the local adapter. + Local --report defaults to the --result path plus .md; require a fresh target, + reject collisions with JSON/stderr, publish complete UTF-8 Markdown privately + without replacing existing files, and report delivery failures as nonzero. + Synthetic child processes belong only to integration tests; they must never + replace OCR or model-driven tool use in production. +- Add `preflight --local` and `review --local`, with required result/stderr + paths and existing immutable commit/from/to input. Ignore inherited CI + identity for local runs; never acquire or mutate GitLab. Reject requested + MR context/adapters. Use JSON/agent audience and reject contradictory options. + Preserve mandatory evidence registry/self-query/completed-summary validation; + configured external MCP cannot replace the mandatory server. No working-tree + snapshot, scan, provider profiles or new authentication framework. +- `--debug-dir PATH` requires --local and a fresh owner-only directory. + Retain bounded original OCR result/stderr, safe final result, console summary + and a structured journal. Record actual configuration, identity/refs, evidence, + MCP preflight/use, preview, subprocess, result validation, DLP, cleanup and + reporting outcomes as passed/failed/degraded/not-run, with explicit missing + artifacts and truncation. Capture real filtering reasons/actions, safe + locations, sizes and digests at their owners, not a second approximation scan. + Raw rejected content stays in private files, never automatic console output. + No environment dump or unnecessary retained session/config. Keep normal + checks and cleanup; reject combination with legacy private-artifact retention. + Bundle ownership is separate from review execution: capture at most 20 MB each + for raw/safe JSON and Markdown, 2 MB for raw stderr, and 1 MB for the journal. + Fixed artifact names and at most 1,000 value-free DLP decisions keep storage + bounded. Record prefix digests and explicit truncation/missing/unavailable + status; do not imply a truncated digest covers the complete source. Normal + result/stderr/report paths must remain outside the fresh debug directory. + Observe actual phases and DLP decisions through optional callbacks at their + production owners; debug observation never substitutes for their checks. +- Preserve existing effort/budget behavior. OCR_LLM_REASONING_EFFORT accepts + unset/empty or case-insensitive none|minimal|low|medium|high|xhigh|max. + Unset adds nothing; none is an explicit wire value, not an omission sentinel. + OpenAI uses reasoning_effort; Responses uses reasoning.effort preserving + siblings. Reject nonempty shortcut for Anthropic. Equal extra-body values + are permitted; incompatible types or conflicts fail before inference. +- OCR_REVIEW_PROGRESS is empty/false by default or true (case-insensitive). + Emit only toolkit phases and a 30-second heartbeat, at most 120 messages. + Never read OCR stderr/results/sessions for progress or change audience to + human. No tee/FIFO, no progress in result/summary/receipts/DLP. Stop promptly + on completion, exceptions and signals; progress sink errors cannot change + review outcomes or leave live heartbeat work behind. +- Preserve zero runtime dependencies, supported Python matrix, immutable Git, + DLP, MCP and provider boundaries. Keep exact pins/evidence separate from + version-neutral current documentation. BL-016/018, BL-010 and BL-021 remain + conditional: none of their independent activation criteria is satisfied. + +#### Provider Architecture Decisions + +- Separate provider input acquisition, common review execution/report data and + provider output/actions. A provider is not merely a formatter and is not a + mandatory all-methods base class. Local supplies immutable Git identity and + Markdown artifacts and console output; forge adapters additionally own authenticated API acquisition + and platform mutations. Do not implement dummy discussion or approval methods + for local execution. +- Shared report data describes the admitted review, not an MR/PR or a posting + transaction. Keep immutable repository refs distinct from optional forge + identity/context. No fabricated author, change-request ID, protection state, + discussion history or publication receipt when those inputs do not exist. +- Distinguish unsupported capabilities, an empty successful acquisition, + disabled acquisition and failed acquisition. In particular, local absence of + discussions does not mean that discussions were fetched and no commands found. + Explicit unsupported requests fail before execution; they are not successful + no-ops or inferred from inherited CI variables. +- Commands from discussions require a supported input channel, authenticated + actor/provenance and action authorization. Common parsers and decision rules + may be reusable, but provider identity, permissions and mutation guards must + not be generalized from GitLab assumptions. Local review does not accept + discussion commands from repository text or model output. +- Share pure calculations and wording; adapt delivery separately. Findings + admitted by DLP, findings selected for posting, and findings actually posted + are distinct facts. Publication limits, suppression and failed delivery must + not rewrite core review health or silently remove local findings. MCP usage + is execution evidence; a forge receipt is a separate platform-bound artifact. +- Preserve existing GitLab acquisition/publication lifecycles in this release. + Do not mechanically equate GitLab discussions/approval with future GitHub + threads/reviews or promise identical retry, transaction and race guarantees. + Future GitHub work must characterize those API boundaries before reuse. + Add dependency tests for the shared report boundary and local tests proving + unsupported channels never acquire data or perform provider writes. + +#### Validation And Commit Gates + +Before each logical signed commit: format changed Python, targeted tests, +complete diff/self-review, repository Ruff format check and git diff --check. +Final gate: quality with scoped coverage floors, lock, manifest/evidence, +Towncrier, package/installed wheel and sdist tests, pinned Gitleaks for tree and +complete feature history. Hosted checks bind the exact final head. +Before overall final self-review, run the Codex Security diff-scan skill over +the complete immutable feature range, triage findings, fix confirmed problems +within this scope and verify remediation. Record scan coverage and unresolved +limitations; a scanner score is not a substitute for semantic self-review. +Organize shared runtime and tests by layer, preserving characterized code through +mechanical moves where appropriate. Reconcile all affected public documentation, +decision-flow diagrams, contract schemas, threat model and test-evidence matrix. + +Required matrices: same normalized GitLab/local report data; clean/findings/ +warnings/partial/budget/failure, true DLP filtering and diagnostic-only input; +progress on/off and normal/debug canonical parity; late heartbeats, signals, +broken output, hostile paths/symlinks/permissions, capture/journal limits and +early failures. Installed tests cross real Git, subprocess and stdio MCP, +include external MCP and polluted CI environment, prove no GitLab calls and +failed mandatory evidence rejection. Real checksum-verified OCR uses local +deterministic protocol peers; no live provider/model calls in this cycle. + +#### External Qualification And Closure + +Create separate local-scenario tracking and relate it to #181 and the Draft. +Update public docs, decision flow, evidence matrix, changelog and backlog. +Preserve example stable toolkit pin until the later release PR; external +qualification must install the artifact built from the exact Draft head. + +External owner verifies GitLab, local and debug workflows with configured LLM. +Reasoning acceptance is a named gate for the exact provider/model/protocol: +test unset (absent on wire), explicit none and intended nonempty effort; +check correct wire shape, provider acceptance and documented/server-observable +support. HTTP 200 alone does not prove that a gateway applied the parameter; +mark unprovable application as unverified, never infer it from model prose. +Unsupported none/effort is explicit; unset remains the safe no-overlay option. +No automatic weakening of security/DLP or fallback to a different model/value. + +#### Resume Point + +WQ-02 is implemented with 475 passing targeted tests and 283 subtests, plus two +adapter-parity tests; Ruff, formatting, mypy and diff checks passed. Mechanical +AST comparison preserves 22 extracted definitions apart from their docstrings. +WQ-02 is committed as 9d0b999. WQ-03 CLI/preflight wiring is implemented: +unsupported local context fails before I/O, inherited forge identity is ignored, +and result admission supplies actual MCP/DLP facts to the console adapter. +Legacy unknown coverage is accepted only as execution-owner admission data, not +as a platform publication receipt. The affected runner/reporting/posting matrix +passes 491 tests and 283 subtests; Ruff, formatting, mypy and diff checks pass. +Installed wheel and sdist scenarios now cross real Git/process/MCP with optional +external MCP and polluted CI identity; forged mandatory use is rejected. Local +publication now persists complete Markdown (default --result plus .md) through +the local adapter after common finalization, with a fresh owner-only atomic file +and console parity. The final CLI/runtime/report/artifact/GitLab regression matrix +passed 631 tests and 400 subtests, including both installed distributions with +hostile repository imports; Ruff, mypy, repository formatting and diff checks +passed. WQ-03 self-review is complete, including failure delivery and private +file lifecycle; the logical commit contains these completed results. +WQ-03 is committed as f864be2. WQ-04 now has fresh private storage, actual phase +and DLP observations, bounded raw/safe/summary captures and explicit incomplete +journal handling. The 264-test matrix passes, including installed wheel/sdist +normal/debug parity for clean, finding, warning, partial, budget, filtered, +forged mandatory usage and subprocess failure outcomes. Observer tests prove no +extra DLP checks and unchanged projections. Ruff, mypy and formatting pass. +WQ-04 self-review is complete: failure-summary delivery and empty-projection +attribution were corrected before the commit gate. The complete affected matrix +passes 654 tests and 400 subtests; an additional installed wheel/sdist SIGTERM +scenario confirms equal nonzero outcomes, cleanup and no safe-result admission. +Ruff, formatting, mypy, Bandit and diff checks pass. The logical WQ-04 commit +contains these results. Actual OCR qualification remains WQ-06. +WQ-04 is committed as 5715c8d. Official OpenAI reasoning +documentation confirms protocol-specific effort paths and model-dependent support. +The reasoning shortcut is implemented in the shared provider configuration owner: +unset adds no overlay, explicit none is retained, Responses siblings survive, +conflicting values/types and Anthropic shortcuts fail in configure and preflight. +Its targeted matrix passes 162 tests and 109 subtests; Ruff and mypy pass. +WQ-05 now includes toolkit-only progress, output backpressure/error handling, +timer shutdown and installed on/off parity (including SIGTERM to OCR and the +toolkit parent). The complete affected matrix passes 829 tests and 400 subtests. +Checksum-verified Darwin OCR 1.11.6 independently passes real request capture +for unset, none and high in both OpenAI protocols, preserving Responses siblings; +the local gateway deliberately rejects requests without model execution. +Self-review, Ruff, mypy and formatting pass. Provider acceptance/application +remains explicitly not tested. WQ-06 must incorporate this probe into the live +qualification contract while preserving older evidence, then promote exact pins +and update the local installed binary. Do not equate the private wire receipt +with full candidate qualification or stable release delivery. +WQ-05 is committed as b585bd4. WQ-06 froze the identical recorded 1.11.4 +and 1.11.5 contracts in the historical reader and added mandatory reasoning +wire evidence to the version-neutral live suite. Existing evidence bytes remain +unchanged; live probes will not branch on candidate release numbers. +The updated live suite passed a complete checksum-verified Darwin 1.11.6 run: +16 language/rule selections, default OCaml/Kotlin-script test exclusions and the +new reasoning wire matrix, alongside every existing consumed probe. Historical +1.11.4/1.11.5 contracts remain byte-identical. Promotion now requires current +schema/contract proof regardless of the historical reader cutoff. + +#### OCR 1.11.6 Adjacent Source Review + +The adjacent `v1.11.5...v1.11.6` comparison contains eight commits. Classification: + +- OCaml/ReasonML allowlist and built-in Rules (`4cea5011`), plus Kotlin `.kts` + routing/test exclusions (`e967f3f4`), change consumed review selection. The live + language probe now verifies those additions; classify their delivery as Rules. +- Upstream Action controls/progress (`0a747205`) inform this feature, but the + toolkit does not execute that Action or adopt human-audience stderr streaming. + Existing toolkit effort and aggregate budgets are preserved; explicit reasoning + and bounded private-safe progress are implemented at toolkit-owned boundaries. +- HTTP header timeout wiring (`71375c16`) affects native OpenAI, Responses and + Anthropic transport: request timeout remains authoritative, with a 30-second + header margin. No toolkit timeout API or response-schema adaptation is added. +- Node launcher signal forwarding (`c590b6b6`) and associated editor cancellation + are upstream launcher/editor behavior; qualification uses standalone assets. +- Viewer line numbering (`7f8fa44f`) and Pages documentation (`0ca5668f`, + `04284b5d`) are not toolkit execution/publication contracts. + +No independent provider-profile, scan or telemetry backlog activation follows +from those adjacent changes. Fresh hosted Linux evidence and full asset readback +are still required before pin promotion; the earlier #181 run predates this suite. +The qualification-harness slice passed self-review, 114 maintainer tests, manifest +validation, formatting and Ruff. Its complete local native run passed all current +contracts. Publish this reviewed slice and dispatch exact-tag Linux qualification +from the new Draft branch before applying the resulting pinned update. +Published qualification slice: c38ca40, followed by validation-override commit +a8c214c. Fresh Linux workflow 34218232310 completed successfully on a8c214c +from the replacement Draft branch: discovery, candidate qualification, asset +readback and complete-chain assessment passed. Its evidence is committed with +the reviewed human conclusion; manifest, preflight and example OCR pins now agree +on 1.11.6. Stable toolkit example pins remain unchanged. The verified Darwin +arm64 asset replaced the installed `/opt/homebrew/bin/ocr`; the former binary is +temporarily retained for rollback. No local binary launch or validation followed +the user's waiver. + +The first Draft CI matrix passed 1694 tests on each of five platforms but failed +two exact environment-inventory assertions whose expected defaults omitted the +new reasoning/progress variables. The expectations are synchronized with the +already documented controls, without relaxing the tests. Quality, build, +dependency, secrets, Bandit and CodeQL jobs passed. Final exact-head CI remains +pending. README, strategy, roadmap, backlog, compatibility history, threat model +and evidence matrix now describe the local provider and its non-claims; existing +result/receipt/context schemas remain unchanged, while the private debug journal +is documented separately and cannot authorize publication. +Promotion/documentation commit bff811a is pushed and the Draft body was updated +and read back in full. Its initial test failures are resolved below. The external +qualification +checklist now lives in `docs/local.md`, covering exact Draft artifact identity, +GitLab/local/debug/progress and unset/none/nonempty provider acceptance. +The user authorized the local read-only security configuration preflight and +corrected publication ordering: no further push before security review and +confirmed fixes are complete. Prior publication before the full-range scan was +an execution error, not satisfaction of that gate. Preflight passed without +configuration changes. Scan `0431df4b-1ce4-4d8b-9771-9395c8b45298` reviews immutable +`8ae890b...63387c3`; TAC access could not be verified because the advisory service +is disconnected. Discovery completed with disjoint source-file assignments. +The second hosted matrix exposed the current-pin test's stale 1.11.5 expectation +and the README heading expectation; those are corrected without changing the +frozen historical fixtures. These corrections passed bounded pre-push review. +Overall self-review found a non-security progress-routing defect: CI enabled the +timer but withheld subprocess/finalization phase notifications unless `--local` +was selected. Observation state now reaches both local and enabled-progress paths +while the report consumer stays local-only. The existing orchestrator matrix adds +progress on/off assertions. A separate pre-push static review of that delta found +no authority, receipt, DLP or cleanup changes; its test double proves wiring only. +Hosted verification remains pending under the local-test waiver. + +The Codex Security scan is sealed/completed with zero findings. All 25 source +inventory files and 32 supplemental changed test/config/documentation files were +reviewed. Important artifact limitation: the service retained the intermediate +`final-coverage-reconciliation` deferred row and reports `partial` even after the +final submission supplied all six completed surfaces and an empty deferred list. +The sealed artifact was not edited or replaced; its partial marker is not claimed +as a fully clean automated coverage result. Actual source review and the bounded +post-scan progress/test correction review are complete. Scan token accounting was +unavailable (`scan_thread_unavailable`). Overall semantic self-review is complete; +no confirmed security finding or unresolved implementation defect remains. +Commit 084ccf3's hosted matrix passed all test executions, including 1700 tests on +the Linux coverage owner, 86.73% total coverage and all four scoped floors. Its +quality job requested a formatting-only collapse of one observer argument; that +exact non-semantic correction is applied and reviewed before the next push. +The old remote branch was deleted with an expected-head lease after GitHub proved +its planning commit is an ancestor of the replacement. #182 now records target +0.10.0, Markdown delivery, replacement #184 and the configured external checklist. +The owned probe/download/tree-check directories and local validation logs were +moved to Trash for recovery, including the previous installed OCR binary backup. +The security report bundle and PR handoff text are retained as private delivery +artifacts under ignored `.quality-logs/ocr0100-handoff/`. + +#### Final local OCR and remediation + +The owner-authorized final local OCR review completed once against the complete +Draft range with the checksum-verified OCR 1.11.6 binary, the configured Waibee +provider, and `openai/gpt-5.6-terra`. It completed all selected items and +preserved its private result, stderr, report, and debug journal for diagnosis. +The initial debug-directory attempt was rejected before OCR execution because +the supplied macOS temporary path traversed a symlink; it is retained as a +pre-execution diagnostic, not counted as a second review. + +Review of the retained output identified seven concrete maintenance defects. +The remediation preserves the one-review boundary: no second OCR launch is +authorized or needed. It pins private report and debug parents with +descriptor-relative filesystem operations; detects exact-boundary summary +truncation; keeps independent safe warnings visible with incomplete coverage; +lets interruption and termination propagate without local report delivery; and +makes optional progress fail closed on a full, closed, or conventional blocking +stderr pipe without changing the caller-owned stream. Interactive terminal and +explicitly nonblocking embedding streams retain bounded progress. The GitLab +example and contracts state that the ordinary job-log pipe intentionally does +not carry optional progress, while normal reports and CI logging are unchanged. + +Focused runtime, reporting, installed-artifact, filesystem-race, terminal, +nonblocking-pipe, blocking-pipe, full-pipe, and documentation tests pass +locally. The complete quality matrix passed with 1711 tests, 408 subtests and +86.72% total coverage; Gitleaks, Ruff, mypy, Bandit, `git diff --check`, and a +Towncrier 0.10.0 draft also pass. Before push, perform a fresh narrow security +diff review for only the signed remediation commit. Do not publish raw OCR +diagnostics, provider values, or private artifact locations. After that review, +push the remediation commit to Draft #184 and wait for exact-head hosted checks. + +#### Draft Readiness Receipt + +Implementation head `5a33e0fce8cefad2917c720bb5a734cc862e58d8` has all 13 checks +green. CI run `34221252576` passed the five supported OS/Python combinations, +including 1700 tests and 86.73% total coverage on its Linux coverage owner; +all four scoped floors passed. Build run `34221252503`, security run +`34221252488`, CodeQL run `34221252685` and dependency review run `34221252618` +passed. Gitleaks 8.24.3 scanned all 11 feature commits through that head and found +no leaks. GitHub verifies the signed feature commits. Draft #184 has the complete +scope, boundaries, evidence and explicit security-artifact limitation; #183 is +closed as superseded, and #181/#182 and milestone v0.10.0 remain open. + +This final documentation-only closure does not change the reviewed runtime. +Before handing it off, read back its own exact-head CI, signature, branch and +Draft state; the live Draft checks/body are the authority for that last external +readback. No further source work is queued. Keep this plan active for the deferred +stable lifecycle: the external owner next qualifies the exact Draft artifact +with configured providers using `docs/local.md`, then separately authorizes +merge and the protected release sequence. No merge, publication or issue closure +has occurred in this work. + +#### Closure Gate + +- [x] Scoped implementation and pre-waiver local evidence complete; remaining validation moved to hosted CI. +- [x] Full self-review, security review with explicit artifact limitation, signed commits and green implementation-head checks. +- [x] Published replacement branch and complete Draft description; owned temporary data cleaned recoverably. +- [x] External qualification checklist recorded; issues/milestone remain open and stable delivery deferred. + +Final docs-head green-check and clean-tree readback is required at handoff, as +described above; it is not authorization for any later release action. diff --git a/README.md b/README.md index cc85790..ec0ddd2 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/xeonvs/open-code-review-toolkit/badge)](https://securityscorecards.dev/viewer/?uri=github.com/xeonvs/open-code-review-toolkit) [![CodeQL](https://github.com/xeonvs/open-code-review-toolkit/actions/workflows/codeql.yml/badge.svg?branch=main)](https://github.com/xeonvs/open-code-review-toolkit/actions/workflows/codeql.yml) -Open Code Review Toolkit is an unofficial GitLab CI integration layer for [Alibaba Open Code Review](https://github.com/alibaba/open-code-review). It provides bounded repository evidence, a compact review bootstrap, a built-in read-only MCP server, environment-driven OCR configuration, preflight validation, and safe GitLab merge-request posting. It does **not** bundle or download the `ocr` binary. +Open Code Review Toolkit is an unofficial review control and integration layer for [Alibaba Open Code Review](https://github.com/alibaba/open-code-review), with GitLab CI and standalone local providers. It provides bounded repository evidence, a compact review bootstrap, a built-in read-only MCP server, environment-driven OCR configuration, preflight validation, local Markdown reports, and safe GitLab merge-request posting. It does **not** bundle or download the `ocr` binary. > [!NOTE] > The project is under active development; the public API, CLI, environment contract, and generated schemas may evolve before 1.0. @@ -55,7 +55,30 @@ The toolkit defaults `OCR_REVIEW_EFFORT` to `medium` for two review rounds. `low Stable distributions are published to [PyPI](https://pypi.org/project/open-code-review-toolkit/) and mirrored as checksum-listed, provenance-attested assets in the corresponding [GitHub Release](https://github.com/xeonvs/open-code-review-toolkit/releases). Development snapshots are published only to TestPyPI. -## How reviews evolve +## Standalone local quick start + +With the qualified OCR binary and LLM environment configured, review an immutable +commit without forge credentials: + +```console +ocr-ci preflight --local +ocr-ci review --local --result review.json --stderr review.stderr --report review.md -- --commit HEAD +``` + +Local execution uses the same OCR, mandatory evidence MCP, validation and DLP +pipeline as CI. It publishes every admitted finding to a fresh private Markdown +file and the console, without GitLab acquisition, suppression, approval or a +fabricated publication receipt. See [local review](docs/local.md) for ranges, +failure behavior and optional private `--debug-dir` bundles. + +Optional [reasoning and progress controls](docs/configuration.md#provider-endpoint-and-completion-cap-contract) +do not change review rounds or budgets. Reasoning is unset by default; explicit +`none` is a wire value whose support depends on the provider/model. Progress is +off by default and emits only bounded toolkit phases to an interactive terminal +or an explicitly nonblocking embedding stream; a conventional CI stderr pipe is +intentionally not used because it can block a review. + +## How GitLab reviews evolve On a successful rerun, the toolkit replaces untouched OCR-only notes instead of accumulating stale reviews. A human reply transfers that discussion to the team: the conversation is preserved and a matching finding is suppressed. Reply with `/ocr suppress` or `@ suppress` to keep a discussion open without future repeats; use the corresponding `resolve` command to resolve it after the next successful posting transaction. For example, a bot named `mr.bot` accepts the exact reply `@mr.bot resolve`. diff --git a/ROADMAP.md b/ROADMAP.md index 895c424..8cc1f0c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -40,6 +40,13 @@ flowchart LR ## Ordering notes +- The active 0.10.0 Draft adds standalone local delivery, provider-neutral reporting, + private debug observations and explicit reasoning/progress controls to the same + execution engine. It does not reopen established milestones or activate the + conditional second-forge, model-profile, routing or evidence-pack backlog. + Draft readiness, configured external qualification and stable delivery remain + separate states in `PLANS.md`. + - OCR compatibility and the established common evidence model converge at compact-bootstrap/evidence-MCP integration. - M3 is established from BL-011's real-OCR characterization of the current generic composition boundary. Direct composition is an operator-configured privileged facility, not safe author-triggered reference resolution. BL-012 remains conditional and does not block M3 or M5 when reviewed static credentials or a stdio proxy suffice. - M2 is established through independently verified stable delivery of its framework plugins, template rules, scoped evidence, deltas, and built-in MCP projection. Conditional future ecosystem packs remain in M7 and do not reopen M2. diff --git a/changelog.d/181.maintenance.md b/changelog.d/181.maintenance.md new file mode 100644 index 0000000..f333323 --- /dev/null +++ b/changelog.d/181.maintenance.md @@ -0,0 +1 @@ +Target checksum-verified Open Code Review 1.11.6 after qualifying 1.11.6. diff --git a/changelog.d/181.rules.md b/changelog.d/181.rules.md new file mode 100644 index 0000000..ab59686 --- /dev/null +++ b/changelog.d/181.rules.md @@ -0,0 +1 @@ +Review OCaml and ReasonML files with the recommended OCR release's built-in rules, and route Kotlin script files to Kotlin rules while preserving its default test-file exclusions. diff --git a/changelog.d/182.bugfix.md b/changelog.d/182.bugfix.md new file mode 100644 index 0000000..da029b4 --- /dev/null +++ b/changelog.d/182.bugfix.md @@ -0,0 +1,4 @@ +Keep standalone local review delivery truthful and available: pin private report +and debug parents against replacement races, retain independent warnings beside +incomplete coverage, preserve interruption semantics, and skip optional progress +when its output stream cannot be written safely. diff --git a/changelog.d/182.feature.md b/changelog.d/182.feature.md new file mode 100644 index 0000000..2b02f22 --- /dev/null +++ b/changelog.d/182.feature.md @@ -0,0 +1,8 @@ +Add explicit standalone local preflight and immutable-diff review with shared +private Markdown artifacts and console summaries, every DLP-admitted finding, and verified evidence usage, +independently of inherited CI identity or forge publication. +Add a private local debug bundle with bounded original and finalized artifacts, +actual check outcomes and value-free DLP decisions, without bypassing the shared +review pipeline or retaining its temporary runtime session. +Expose explicit protocol-specific reasoning effort and bounded, opt-in review +progress that remains independent of OCR output and result admission. diff --git a/changelog.d/182.refactor.md b/changelog.d/182.refactor.md new file mode 100644 index 0000000..0f4d492 --- /dev/null +++ b/changelog.d/182.refactor.md @@ -0,0 +1 @@ +Separate common review outcome, coverage, finding metadata, usage and DLP reporting from GitLab publication, preserving existing GitLab summaries and approval behavior. diff --git a/compatibility/evidence/ocr-1.11.6.json b/compatibility/evidence/ocr-1.11.6.json new file mode 100644 index 0000000..494c999 --- /dev/null +++ b/compatibility/evidence/ocr-1.11.6.json @@ -0,0 +1,275 @@ +{ + "assets": [ + { + "name": "opencodereview-darwin-amd64", + "sha256": "311cc54cefcd51263ba07851aa6a32f259edfcc9ec9861039eab9da274657dce", + "size": 57828336 + }, + { + "name": "opencodereview-darwin-arm64", + "sha256": "a60f101490ba9bb88e90becf23b78c1e2fd2d98219ea36384010d488ed32d7b0", + "size": 54861634 + }, + { + "name": "opencodereview-linux-amd64", + "sha256": "09f30595834f8297a592b51bf4707fb24728b2826a65915f1964c5563e4fb3bd", + "size": 56082594 + }, + { + "name": "opencodereview-linux-arm64", + "sha256": "1e4dc70445037fd06c3bbb0a0b09944a696703937c3287083321540d880729a2", + "size": 52822178 + }, + { + "name": "opencodereview-windows-amd64.exe", + "sha256": "c2652bb4f70c8c3af38bf69656ab05f10c13cd9457da77bc374f41ab9c25de25", + "size": 57546240 + }, + { + "name": "opencodereview-windows-arm64.exe", + "sha256": "a5402e6bf927c5bd042ca67a4fad41e4a92c1d3d06f1dbeee147bf66fdb5486c", + "size": 53508608 + }, + { + "name": "sha256sum.txt", + "sha256": "5fe2640d8b521f7ac6ff4880a5227e452226998171c49e728fb23c02d858cd52", + "size": 572 + } + ], + "classification": "human-review-required", + "classification_reasons": [ + "release notes contain a material or ambiguous compatibility signal" + ], + "comparison_version": "1.11.5", + "contracts": { + "comment_arguments_probe": { + "comments_per_batch": 2, + "failure_arguments_preserved": true, + "intact_cases": [ + "array", + "serialized", + "repaired" + ], + "repair_warning": true, + "result": "passed", + "suspect_batch_rejected": true + }, + "comment_thinking_probe": { + "additive_field_preserved": true, + "posting_exposes_thinking": false, + "result": "passed" + }, + "completion_cap_probe": { + "explicit": 4096, + "inherited": 16384, + "result": "passed", + "wire_field": "max_completion_tokens" + }, + "language_rule_probe": { + "default_excluded_paths": [ + "src/test/kotlin/scripts/Example.kts", + "test/parser.ml" + ], + "excluded_extensions": [ + ".svh" + ], + "extensions": [ + ".cjs", + ".cxx", + ".hxx", + ".kts", + ".mjs", + ".ml", + ".mli", + ".mm", + ".pug", + ".re", + ".rei", + ".sv", + ".v", + ".vh", + ".vhd", + ".vhdl" + ], + "m_routing": "matlab_and_objective_c", + "result": "passed", + "rule_source": "system_builtin", + "selected": 16 + }, + "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", + "per_run_provider_override", + "review_effort", + "semantic_grouping" + ], + "preview_probe": { + "format": "json", + "path": "example.py", + "result": "passed", + "session_store_created": false + }, + "reasoning_effort_probe": { + "efforts": [ + "unset", + "none", + "high" + ], + "protocols": [ + "openai", + "openai-responses" + ], + "provider_acceptance": "not-tested", + "responses_siblings_preserved": true, + "result": "passed" + }, + "required_review_flags": [ + "--audience", + "--background-file", + "--effort", + "--format", + "--from", + "--max-tokens-budget", + "--max-tools", + "--preview", + "--rule", + "--to" + ], + "result_contract_probe": { + "additive_fields_allowed": true, + "comment_fields": [ + "category", + "content", + "end_line", + "existing_code", + "path", + "severity", + "start_line", + "thinking" + ], + "manifest_schema": "ocr.run-manifest/v1", + "normalized_outcome": "clean", + "result": "passed" + }, + "review_budget_probe": { + "budget": 30000, + "completed": 2, + "failed_budget": 1, + "grouping_requests": 0, + "grouping_strategy": "per_file", + "partial_findings_preserved": true, + "result": "passed", + "selected": 3 + }, + "semantic_grouping_probe": { + "default_effort": "medium", + "files": 4, + "filter_requests": 1, + "grouping_completion_cap": 16384, + "grouping_requests": 1, + "main_requests": 3, + "prior_finding_semantics": "filter_survivors_as_confirmed", + "recheck_instruction_requests": 3, + "result": "passed", + "review_rounds": 2 + }, + "small_change_grouping_probe": { + "grouping_requests": 0, + "high_churn": "per_file", + "low_churn": "bundle_all", + "result": "passed", + "single_file": "per_file", + "threshold_files": 4 + }, + "target_rule_selection_probe": { + "format": "json", + "from_to_unchanged": true, + "path": "synthetic-template.ocrfixture", + "result": "passed", + "source_exclusion": "unsupported_ext", + "target_selected": true + }, + "version_probe": "passed" + }, + "published_at": "2026-09-07T07:22:48Z", + "release_changes": "## 🚀 Features\n\n- feat(allowlist): add OCaml/ReasonML language support (#942) (#951)\n- feat(action): first-class review controls (effort, max_tokens_budget, llm_reasoning_effort) and live progress (#1154)\n- feat(viewer): number the lines inside the Existing Code block (#1172) (#1180)\n\n## 🐛 Bug Fixes\n\n- fix(launcher): forward SIGINT/SIGTERM signals to child process (#1145)\n- fix(rules): route Kotlin script files to Kotlin review rules (#1091)\n\n## 📖 Documentation\n\n- docs(pages): correct FAQ JSON output shape and plan-threshold cost lever (#1190)\n- docs(pages): correct the developer guide to match the actual project (#1182)\n\n## Other Changes\n\n- Honor timeout_sec for openai-go's ResponseHeaderTimeout (#1161) (#1162)\n\n**Full Changelog**: https://github.com/alibaba/open-code-review/compare/v1.11.5...v1.11.6", + "release_notes_sha256": "ff62a35907aba038dedc23b6746e42f46066046a950001a50591f79194d27ecb", + "result": "compatible", + "schema_version": 3, + "tag": "v1.11.6", + "tested_baseline_version": "1.11.5", + "upstream_repository": "alibaba/open-code-review", + "version": "1.11.6" +} diff --git a/compatibility/ocr-support.json b/compatibility/ocr-support.json index b1a2abe..a848252 100644 --- a/compatibility/ocr-support.json +++ b/compatibility/ocr-support.json @@ -1,6 +1,6 @@ { - "monitoring_floor": "1.11.5", - "recommended_version": "1.11.5", + "monitoring_floor": "1.11.6", + "recommended_version": "1.11.6", "releases": [ { "assets": [ @@ -1611,6 +1611,59 @@ "release_url": "https://github.com/alibaba/open-code-review/releases/tag/v1.11.5", "status": "tested", "version": "1.11.5" + }, + { + "assets": [ + { + "name": "opencodereview-darwin-amd64", + "sha256": "311cc54cefcd51263ba07851aa6a32f259edfcc9ec9861039eab9da274657dce", + "size": 57828336 + }, + { + "name": "opencodereview-darwin-arm64", + "sha256": "a60f101490ba9bb88e90becf23b78c1e2fd2d98219ea36384010d488ed32d7b0", + "size": 54861634 + }, + { + "name": "opencodereview-linux-amd64", + "sha256": "09f30595834f8297a592b51bf4707fb24728b2826a65915f1964c5563e4fb3bd", + "size": 56082594 + }, + { + "name": "opencodereview-linux-arm64", + "sha256": "1e4dc70445037fd06c3bbb0a0b09944a696703937c3287083321540d880729a2", + "size": 52822178 + }, + { + "name": "opencodereview-windows-amd64.exe", + "sha256": "c2652bb4f70c8c3af38bf69656ab05f10c13cd9457da77bc374f41ab9c25de25", + "size": 57546240 + }, + { + "name": "opencodereview-windows-arm64.exe", + "sha256": "a5402e6bf927c5bd042ca67a4fad41e4a92c1d3d06f1dbeee147bf66fdb5486c", + "size": 53508608 + }, + { + "name": "sha256sum.txt", + "sha256": "5fe2640d8b521f7ac6ff4880a5227e452226998171c49e728fb23c02d858cd52", + "size": 572 + } + ], + "capabilities": [ + "llm_result_identity", + "per_run_model_override", + "per_run_provider_override", + "review_effort", + "semantic_grouping" + ], + "evidence": "compatibility/evidence/ocr-1.11.6.json", + "evidence_sha256": "254d58aa1e516ec686a196f59948ad6d92d219cbc8a3fdd1b756a09f4bcb8c57", + "human_conclusion": "Compatible after adjacent eight-commit source review and complete current Linux amd64 qualification in GitHub Actions run 34218232310, with independent Darwin arm64 no-LLM qualification. OCaml/ReasonML and Kotlin script rule selection is consumed and separately classified as Rules. Native timeout changes preserve the toolkit API; Action progress, Node/editor and viewer changes are not executed by the toolkit. Reasoning wire shape is verified for unset, none and high in both OpenAI protocols; provider acceptance and application remain untested. Protected PR and stable release gates remain required.", + "published_at": "2026-09-07T07:22:48Z", + "release_url": "https://github.com/alibaba/open-code-review/releases/tag/v1.11.6", + "status": "tested", + "version": "1.11.6" } ], "schema_version": 1, diff --git a/docs/codex/TASKS_BACKLOG.md b/docs/codex/TASKS_BACKLOG.md index 36c7cb5..00ba99c 100644 --- a/docs/codex/TASKS_BACKLOG.md +++ b/docs/codex/TASKS_BACKLOG.md @@ -133,6 +133,7 @@ Provider/network telemetry remains outside M1 and M5. OCR owns token, cost, budg - **Dependencies:** Stable provider-neutral core contracts and a funded non-GitLab use case. GitLab-first M5 does not depend on it. - **Activation trigger:** A named forge has an owner, synthetic fixtures, and explicit parity requirements for CI orchestration, positioning, deduplication, discussion ownership, and safe publication. - **Upstream overlap:** OCR reusable GitHub Action checkpoint ranges and Action/plugin capabilities cover only upstream execution surfaces. They do not provide toolkit forge acquisition, discussion, publication, or lifecycle parity, so the trigger and acceptance criteria remain unmet. +- **Local-provider distinction:** The 0.10.0 Draft shares pure reporting and supplies immutable local input plus Markdown delivery. It adds no forge authentication, discussion commands, threads, suppression or approval; it is not completion or activation of this second-forge item. - **Goal:** Add one coherent host adapter without leaking forge semantics into evidence or core result handling. - **Scoped deliverables:** The capability matrix covers authentication, diff positions, drafts, discussion acquisition, protected same-revision CI outcomes, provider-declared account classification, thread/reply structure, edit/version identity, anchors, resolved/stale state, pagination/snapshot mutation, ambiguous writes, permissions, and idempotency. - **Acceptance criteria:** Core remains provider-neutral, GitLab behavior does not regress, unsupported host capabilities fail or degrade explicitly rather than emulate unsafe parity, and the new host meets the approved lifecycle and security matrix. diff --git a/docs/compatibility.md b/docs/compatibility.md index ef97b2d..7ce97c1 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -10,6 +10,10 @@ version. Historical evidence before the suite boundary is checked separately by `scripts/ocr_compat_history.py` using frozen expectations; that reader never executes old binaries. Generic promotion tests use a frozen baseline. See [qualification maintenance](development.md#maintaining-ocr-qualification). +Historical readback is not promotion authority: every newly promoted candidate +must provide the current evidence schema and complete live consumed contract, +regardless of the historical archive boundary. The live suite includes explicit +reasoning wire capture and the current built-in language/test-exclusion inventory. The **OCR compatibility** workflow discovers stable upstream releases newer than the manifest monitoring floor. Its daily trigger is scheduled for `07:15 UTC`, after the observed upstream release window; GitHub may delay or omit scheduled delivery, so exact-tag manual dispatch remains the recovery path. 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`. @@ -174,6 +178,29 @@ Linux amd64 SHA-256: `53a4ab7c8ce6dc07d5362c7c4984bf8d98b55e4e8d4c01b9399d488a2a Darwin arm64: `c041b03cc840957b52df28514e8dbb51f798e6cb1259d97555a41a2e3e3ccaf9`. Upstream checksum file: `0519c13b03d69dd6c4aa5470a8eb52727f5e3fd2ce43e5887d8a67cdf107aa1a`. +### OCR 1.11.6 — toolkit 0.10.0 Draft target + +The Draft recommends exact OCR 1.11.6. Hosted qualification +[34218232310](https://github.com/xeonvs/open-code-review-toolkit/actions/runs/34218232310) +verified every platform asset and the upstream checksum file, then passed the +complete current Linux amd64 suite. Independent checksum-verified Darwin arm64 +probes passed the same consumed contracts. The committed evidence adds reasoning +wire checks for unset, explicit `none` and `high` in both OpenAI protocols, +including preserved Responses siblings. The capture peer rejects requests without +model execution: provider acceptance and actual application remain untested. + +The adjacent eight-commit review identifies consumed OCaml/ReasonML allowlist and +built-in Rules additions plus Kotlin script routing and default test exclusions. +They are separately classified as Rules. Native HTTP header timeout changes retain +the request timeout plus upstream margin without a new toolkit API. The toolkit +does not execute the upstream Action, Node/editor launcher or viewer; its reasoning +and progress controls are owned by the shared toolkit execution path. + +Historical 1.11.4/1.11.5 evidence remains unchanged. New promotions require the +complete current contract even when historical evidence is still readable. +This qualification does not establish stable toolkit delivery or configured +GitLab/local/debug model quality; those remain separate gates. + ## Promotion and rollback Promotion changes `recommended_version`, advances `monitoring_floor`, adds the tested release and evidence, and updates every machine-readable or executable version/checksum pin. Never edit only one copy. Human-qualified candidates must record the compatibility conclusion and release-note impact; an automatic-safe candidate may also record a reviewed conclusion when it is delivered with a human-reviewed chain instead of using the generic machine conclusion. Conclusions may name only versions present in that promotion. A reviewed promotion may cross only one adjacent semantic-version boundary at a time: the next patch, the next minor at `.0`, or the next major at `.0.0`; minor and major transitions always require an explicit human conclusion. Automatic-safe preparation remains limited to adjacent patches in the already-tested major/minor line and retains the same protected review boundary even though the patch itself is mechanical. diff --git a/docs/configuration.md b/docs/configuration.md index 26d30ac..12e0ae5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,5 +1,10 @@ # Environment configuration +Provider selection is an explicit CLI choice: [standalone local review](local.md) +uses `preflight --local` and `review --local`, independently of inherited CI +identity. It preserves the configured LLM and optional external MCP inputs, but +rejects unsupported change-request context channels. + Open Code Review Toolkit uses environment variables for CI/runtime configuration. Empty values are generally treated as absent. Exact defaults and safety caps are enforced by the runtime modules. **Bold variable names are required in the command, mode, example, or declaration scope stated in their `Required` cell.** OCR behavior below refers to the exact `recommended_version` in the @@ -20,9 +25,11 @@ These are the complete supported toolkit-owned runtime inputs. `Required` is sco | `OCR_LLM_EXTRA_HEADERS` | Operator / configure and preflight | No | Empty object | JSON object of additional string headers; cannot duplicate the auth header. | | `OCR_LLM_EXTRA_BODY` | Operator / `ocr-ci configure` | No | Unset | JSON object merged into the OCR LLM request configuration; completion-cap field conflicts are checked against the dedicated variable. | | `OCR_LLM_MAX_COMPLETION_TOKENS` | Operator / `ocr-ci configure` | No | Unset (inherits OCR) | Positive decimal integer from `1` through `1000000`; sets the protocol-specific completion/output cap without changing prompt/context or aggregate review budgets. | +| `OCR_LLM_REASONING_EFFORT` | Operator / configure and preflight | No | Unset (no overlay) | Case-insensitive `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, or `max`; explicit `none` is sent, not omitted. Nonempty values require an OpenAI protocol. | | `OCR_ANTHROPIC_DISABLE_THINKING` | Operator / `ocr-ci configure` | No | `false` | With the Anthropic protocol, exact `true` adds `thinking.type=disabled`. | | `OCR_REVIEW_LANGUAGE` | Operator / shared language resolver | No | `English` | Allowed language label or BCP-47 tag used for the review. | | `OCR_REVIEW_EFFORT` | Operator / `ocr-ci configure` | No | `medium` | Closed OCR quality preset: `low`, `medium`, or `high`; maps to one, two, or three review rounds. | +| `OCR_REVIEW_PROGRESS` | Operator / `ocr-ci review` | No | Empty / `false` | Case-insensitive `true` enables toolkit-only phase messages and a 30-second heartbeat on an interactive terminal or explicitly nonblocking embedding stream, capped at 120 messages per run. Conventional CI stderr pipes are deliberately unsupported. | | `OCR_LLM_VALIDATE_MODEL` | Operator / `ocr-ci preflight` | No | `false` | `true` validates through `/models`; `auto` may use the offline allowlist; false values skip validation. | | `OCR_LLM_MODELS_URL` | Operator / `ocr-ci preflight` | No | Derived from `OCR_LLM_URL` | Explicit absolute credential-free HTTPS metadata URL when validation is enabled or inference query parameters make derivation ambiguous. | | `OCR_LLM_ALLOWED_MODELS` | Operator / `ocr-ci preflight` | No | Empty list | Comma-separated exact model identifiers for offline or `auto` validation. | @@ -52,6 +59,24 @@ Since 0.8.0, `OCR_USE_ANTHROPIC` is not a compatibility alias. Any presence fail `OCR_LLM_PROTOCOL` is authoritative; the URL never selects a protocol. `OCR_LLM_URL` accepts an API root or the matching terminal endpoint: `/chat/completions` for `openai`, `/responses` for `openai-responses`, and `/v1/messages` for `anthropic`. Configure and preflight use the same normalized API root, reject a terminal endpoint belonging to another protocol, and reject credentials or fragments embedded in either provider URL. A query is preserved for inference. Because copying it to an auxiliary endpoint is ambiguous, model validation with a queried inference URL requires an explicit `OCR_LLM_MODELS_URL`. +`OCR_LLM_REASONING_EFFORT` is independent of the existing OCR review effort and +token budgets. Unset or empty leaves the request overlay unchanged. A nonempty +value is normalized to lowercase and sets `reasoning_effort` for `openai`, or +`reasoning.effort` for `openai-responses`, preserving other `reasoning` members. +When the shortcut is set, an equal value at the corresponding path in +`OCR_LLM_EXTRA_BODY` is allowed; +different values, incompatible field types or a non-object Responses `reasoning` +value fail before inference. Manual JSON values must already use the exact +lowercase wire spelling. Nonempty shortcuts are rejected for `anthropic`. + +These paths follow the [official OpenAI reasoning contract](https://developers.openai.com/api/docs/guides/reasoning#reasoning-effort). +Supported values and defaults depend on the exact provider/model/protocol; +`none` is an explicit request and is not universally supported. Toolkit parsing +does not prove provider acceptance or application. Qualify unset, explicit `none` +and the intended effort in the deployment; HTTP success alone does not prove +that a gateway applied the setting. The toolkit never silently substitutes an +effort, model or protocol after rejection. + `OCR_LLM_MAX_COMPLETION_TOKENS` is optional and defaults to **unset**, which inherits the qualified OCR version's behavior. It accepts a positive decimal integer from `1` through `1000000` and writes one protocol-specific field: | `OCR_LLM_PROTOCOL` | Generated `llm.extra_body` field | @@ -70,6 +95,23 @@ The inherited value is version-owned and may change with a qualified OCR upgrade `OCR_REVIEW_EFFORT` defaults to `medium` and is written to OCR's root `effort` configuration key. OCR maps `low`, `medium`, and `high` to one, two, and three review rounds and scales its 15-minute subtask base to 15, 30, or 45 minutes. The environment is operator-owned; merge-request text cannot change it. An explicit caller `--effort` passed after `ocr-ci review --` has normal OCR CLI precedence over the generated config, while an unknown environment value fails configuration before preview or model execution. +`OCR_REVIEW_PROGRESS=true` observes the same review in local and CI execution. +Messages contain only closed toolkit phase names; the observer never reads OCR +results, stderr artifacts or sessions, and does not change the agent audience. +Progress is not copied into results, Markdown, receipts or DLP inputs. The timer +stops on completion, exceptions, signals or the 120-message cap. + +For a conventional blocking stderr pipe, including a GitLab runner job-log pipe, +the optional observer disables itself. A short write can still block after a +readiness check, while `dup()` and common `/dev/fd` reopens may share the +caller-owned descriptor's status flags; the toolkit does neither. A full +nonblocking pipe also disables progress rather than blocking the review. This has +no review, admission, output, or exit-status effect and does not suppress normal +final reports or CI logs. The toolkit does not create a tee, FIFO, background +writer, or another process to simulate a logging transport. Values other than +empty, `false` or `true` fail before review execution without echoing their +contents. + OCR may present filter-surviving comments to a later round as previously confirmed, but the toolkit does not accept that wording as validation. Its mandatory background prefix travels with every main request and requires prior/filter-surviving findings to remain unverified until current code, tests, or trusted evidence support them. Survival cannot change severity, suppress or resolve a finding, authorize approval, or enter a receipt as independent validation. Effort controls review depth, not the prompt/context ceiling, per-call completion cap, aggregate token budget, or per-round tool limit. Semantic grouping and filtering can add requests even at `low`; higher effort can add further rounds until OCR stops early, reaches a coverage/budget boundary, or completes the configured depth. diff --git a/docs/development.md b/docs/development.md index 30b7b68..6d0951d 100644 --- a/docs/development.md +++ b/docs/development.md @@ -106,6 +106,29 @@ New runtime modules, classes, and functions need purpose-focused docstrings. Com Apply the [cohesive-module invariant](engineering/project_principles.md#product-and-architecture) during self-review. Prefer an extract-and-delegate refactor that moves already characterized functions or classes intact, preserves the intentional package facade, and reruns the same contract suite before and after each move. Split on distinct responsibility and dependency direction, not an arbitrary line count; do not rewrite a working algorithm merely to make a file shorter. Architecture tests should protect required owners and forbidden upward dependencies without freezing every future helper-module name. +## Review reporting and provider adapters + +`reporting/` owns provider-neutral report data and pure presentation: `model.py` +snapshots admitted results and execution-owner facts, `outcome.py` describes +review health separately from finding delivery, `result.py` normalizes coverage +and warnings, and `metadata.py`, `usage.py`, `dlp.py`, `sections.py`, and `text.py` +own their respective pure calculations. Shared reporting must not import posting, +forge acquisition, forge receipts, environment configuration or provider actions. +Posting facades preserve existing imports while GitLab retains its publication +transactions, discussion anchors, suppression and approval policy. + +`providers/local.py` publishes private Markdown artifacts and console output from that shared report, with every +admitted finding and no posting cap, remote badge or HTML disclosure. It does not +acquire forge data or accept discussion commands. The runner, not the formatter, +must establish immutable identity, mandatory completed evidence use and DLP +admission before constructing a successful report. Passing a model-authored +receipt to a formatter is not evidence of successful execution. + +New shared reporting tests live in `tests/reporting/`; keep GitLab transaction +regressions with their existing owners. Dependency tests enforce the downward +boundary, while outcome and rendering tests prove the pure report contract. +Provider I/O and executable CLI claims require separate production-boundary tests. + ## Extending ecosystem evidence Normalized source adapters live under `src/ocr_toolkit/evidence/ecosystems/`. Shared parser result contracts belong in `ecosystems/contracts.py`; Python, JavaScript, Go, and PHP package metadata each have one adapter module. Ansible keeps Galaxy requirements and topology/inventory analysis as separate modules under `ecosystems/ansible/`. These adapters consume text or already bounded metadata and return normalized facts: they do not own Git or filesystem reads, subprocesses, network access, framework derivation, persistence, or MCP lifecycle. diff --git a/docs/engineering/test_evidence_matrix.md b/docs/engineering/test_evidence_matrix.md index d4d6feb..97101c6 100644 --- a/docs/engineering/test_evidence_matrix.md +++ b/docs/engineering/test_evidence_matrix.md @@ -197,6 +197,36 @@ Every top-level test module is classified below. A module can contain more than | Serialized comment repair and failure arguments | Checksum-verified OCR 1.11.4 and 1.11.5 use a local deterministic gateway; native, serialized and repaired batches preserve fields and anchors, suspect batches fail with original arguments in private failure data | Binary behavior and field preservation, not model judgment | | Forward-only suite and historical evidence | Complete real no-LLM runs before/after refactoring yield identical JSON contracts; frozen promotion-baseline and historical-independence tests reject missing proof before writes | Historical readback does not imply supported execution of old binaries | +## Shared reporting and local output + +| Boundary | Owner and entry point | Evidence | Claim limit | +| --- | --- | --- | --- | +| Provider-neutral reporting | `reporting.model.report_from_result` and shared pure formatters | `tests/reporting/` exercises outcome, invalid admission, coverage recovery, mandatory action counts, snapshot isolation and forbidden forge dependencies; existing posting and approval tests preserve GitLab behavior | synthetic execution facts stand in for the runner's verified output; these tests do not prove OCR or MCP execution | +| Local Markdown delivery | `providers.local.write_local_report` into a real text stream | all admitted findings and code fields, posting-environment independence, fence/control escaping and truthful failure output | proves the output adapter, not standalone CLI acquisition, subprocess execution or debug retention | +| Private report persistence | `providers.local.publish_local_report`; local CLI failure delivery | `tests/providers/test_local_artifact.py` crosses real descriptor-relative exclusive atomic file publication, permissions, hostile targets, output collisions, partial-render failure, destination races and same-user ancestor replacement | proves filesystem delivery, not OCR/model execution | +| Installed standalone execution | installed `ocr-ci review --local` through the common runner and finalizer | `tests/test_installed_policy_e2e.py` invokes `tests/providers/installed_local_review.py` for direct wheel and sdist-derived wheel; real immutable Git, subprocess, built-in stdio MCP, optional external MCP, polluted CI identity, admitted JSON/Markdown parity and forged mandatory-usage rejection | synthetic OCR process is beyond the real toolkit launcher; proves installed toolkit boundaries, not actual OCR protocol compatibility or model judgment; actual OCR evidence is recorded separately below | +| Reasoning configuration and wire shape | shared provider configuration -> actual native OCR -> capture-only HTTP peer | configuration/preflight conflict and protocol tests; checksum-verified OCR 1.11.6 current qualification checks unset, none and high for OpenAI and Responses with preserved siblings, on Linux amd64 and Darwin arm64 | deliberate HTTP rejection prevents model execution; provider acceptance and application remain untested | +| Content-independent progress | `review_progress` owner -> real stderr descriptor and bounded heartbeat lifecycle | on/off installed wheel/sdist parity across normal/debug outcomes, parent and child SIGTERM; closed/full sink, nonblocking and terminal writers, late tick and timer failures | toolkit lifecycle and output isolation, not upstream Action streaming or model progress; conventional blocking CI stderr deliberately disables the optional observer | + +The local debug boundary is exercised by `tests/test_review_debug.py` through +real private files, descriptor-pinned directories, atomic journal publication, +capture bounds and hostile targets. DLP observer tests compare both the complete +projection and the number of production checks, proving no diagnostic rescan. +The installed wheel/sdist scenario compares normal/debug exit codes, JSON and +Markdown for clean, finding, warning, partial, budget, DLP-filtered, forged-use +and subprocess-failure results, including an actual SIGTERM to the synthetic +OCR child. The external OCR collaborator is synthetic in +that matrix; actual OCR qualification and model judgment remain separate gates. +Injected late write failures prove error handling, not operating-system fault +frequency or durability after host failure. + +The v0.10.0 hosted matrix initially passed 1694 tests per platform and identified +two stale environment-inventory expectations for the new controls; their exact +default entries are synchronized without removing the inventory assertions. +Current real-OCR evidence is committed in `compatibility/evidence/ocr-1.11.6.json` +from hosted run `34218232310`; it preserves the older evidence snapshots and +qualifies all current consumed probes, including expanded language selection. + ## Unsafe or nondeterministic external boundaries The suite intentionally does not perform live GitLab comment, discussion, cleanup, or approval writes; live GitHub issue/release mutations; or PyPI publication. Their tests prove closed payloads, ordering, fail-closed decisions, transport serialization, and receipt parsing. Release completion requires independent live readback as defined in `docs/release.md`. diff --git a/docs/engineering/toolkit_strategy.md b/docs/engineering/toolkit_strategy.md index 6732a2c..a3652b3 100644 --- a/docs/engineering/toolkit_strategy.md +++ b/docs/engineering/toolkit_strategy.md @@ -11,9 +11,11 @@ The toolkit does not replace OCR. OCR owns diff review, file selection and bundl ```mermaid flowchart LR CI[GitLab CI today] --> CT[Toolkit control layer] + LOCAL[Explicit local immutable review] --> CT CT -->|validated config and bounded bootstrap| OCR[Alibaba OCR] OCR -->|review findings| CT CT -->|normalized bounded writes| GL[GitLab discussions today] + CT -->|all admitted findings| MD[Private Markdown and console] OCR -->|native repository tools| REPO[Repository] ``` @@ -31,6 +33,16 @@ Repository-maintenance analyzers such as Bandit remain valid quality controls fo ## Implemented architecture +The explicit local provider supplies immutable Git identity and private Markdown +delivery through the same OCR execution, evidence MCP and result finalization as +GitLab. Pure shared reporting owns review facts and wording, not forge acquisition +or actions. Local execution has no discussion-command channel, suppression, +approval or fabricated platform receipt. Private debug observations follow actual +production checks without rescanning or bypassing them; optional progress reads +no review content. Reasoning effort is a protocol-specific configuration overlay, +not a model-profile router. These capabilities do not activate a second forge, +repository scanning, automatic profile routing or another telemetry layer. + The M1 implementation provides a schema-versioned Repository Evidence Engine with bounded immutable base/head reads, typed dependency/runtime/image/guidance records and deltas, redaction-before-storage, a compact bootstrap, and the built-in read-only `ocr_toolkit_evidence` MCP server. MCP configuration registers that server alongside reviewed external stdio or remote servers with explicit tool allowlists. The legacy `context/*` Markdown renderer, its CLI/environment contract, and its parity-only bridge have been removed. The evidence store and compact bootstrap are private artifacts owned by `ocr-ci review`, not separately configured workflows. diff --git a/docs/local.md b/docs/local.md new file mode 100644 index 0000000..f721eb3 --- /dev/null +++ b/docs/local.md @@ -0,0 +1,146 @@ +# Standalone local review + +Use the local provider to review immutable Git changes and read the admitted +findings in a Markdown artifact and the console, without connecting the repository to a forge. +Install the toolkit and its [qualified OCR binary](../compatibility/ocr-support.json), +then configure the LLM environment described in [configuration.md](configuration.md). + +```console +ocr-ci preflight --local +ocr-ci review --local --result review.json --stderr review.stderr --report review.md -- --commit HEAD +``` + +For a range, replace `--commit HEAD` with `--from BASE --to HEAD`. Both ends are +resolved to immutable commits before execution. Working-tree snapshots, repository +scans, resumed sessions and competing repository/background inputs are unsupported. +The toolkit owns JSON output and agent audience; contradictory `--format` or +`--audience` options fail before execution. + +## Inputs and isolation + +Local mode is explicit. It may run in a CI test job, but inherited CI identity does +not select a forge provider, acquire change-request data or authorize publication. +`preflight --local` checks the OCR binary and configured LLM, not forge access. +Set `OCR_REVIEW_CONTEXT_MODE` to `off` or leave it unset; other context modes and +any `OCR_REVIEW_CONTEXT_ADAPTERS_JSON` value are rejected before input acquisition. +Local mode has no discussion, command, suppression or approval channel. + +The mandatory repository-evidence MCP remains enabled. Registry validation and +the toolkit's self-query do not substitute for the review's completed evidence +summary call: the result's claimed usage must reconcile with the real action +record. Operator-configured external MCP entries remain optional and cannot +replace mandatory evidence. Their capabilities and trust boundaries are described +in [MCP composition](configuration.md#mcp-composition-and-trust-boundary). + +## Output and failures + +Standard output contains shared review health, coverage, warnings, tool/token and +verified MCP facts, DLP admission status, and every admitted finding. It has no +posting caps, remote badges, HTML disclosures or platform publication claims. +Finding text and code are rendered as literal fenced content. Diagnostics use +standard error; JSON stays in the private `--result` artifact. + +The same complete Markdown is published to `--report PATH`, defaulting to the +`--result` path plus `.md`. The report target must be fresh and distinct from +JSON/stderr; existing files and symlinks are never overwritten. Publication is +atomic with owner-only (`0600`) permissions. This is the same OCR/LLM/tool-use, +validation and DLP pipeline as CI, with local filesystem delivery instead of +forge API publication. It does not replace OCR with another local review engine. +The private report parent is opened descriptor-relative without following any +pathname symlink, then the temporary and final entries are created through that +pinned directory. A same-user replacement of a parent path cannot redirect +report delivery. + +Successful execution still requires result validation, DLP and cleanup. The +admitted JSON has no fabricated forge receipt. Filtered findings stay absent from +the console and safe JSON; omissions and original coverage remain explicit. +Legacy results without coverage counts remain unknown, not a fabricated complete +coverage manifest. A blocked run returns nonzero and prints a closed failure +summary naming its stage without presenting untrusted findings. An unavailable +output stream cannot guarantee delivery of that summary. Failures after the +report destination has been accepted also publish the closed failure report; +configuration or unsafe-destination failures print only to the console. A failed +artifact or console delivery returns nonzero; a completed artifact remains +available if the console fails afterward. + +`--local` cannot be combined with legacy `--preserve-private-artifacts`, because +that diagnostic path intentionally bypasses ordinary result finalization. The +legacy option without `--local` retains its existing sensitive-artifact semantics. + +## Private debug bundle + +Add `--debug-dir PATH` to retain diagnostics from the same review execution. +The directory must be fresh, must not traverse symlinks, and must not contain +the normal result, stderr or report destinations. It is created with mode `0700`; +its files use `0600`. This option requires `--local` and rejects legacy private +artifact retention. It does not bypass validation, DLP or ordinary session cleanup. +Its parent directory is pinned before the bundle is created, and every later +write is descriptor-relative; replacing a visible ancestor path cannot redirect +diagnostics to a different directory. + +| Artifact | Contents | Maximum retained bytes | +| --- | --- | --- | +| `raw-result.json` | Original OCR output before finalization, when execution reached OCR | 20,000,000 | +| `raw-stderr.log` | Original OCR diagnostic stream | 2,000,000 | +| `safe-result.json` | Successfully finalized JSON, if available | 20,000,000 | +| `summary.md` | Admitted report or closed failure summary | 20,000,000 | +| `journal.json` | Actual phase observations and value-free DLP decisions | 1,000,000 | + +Raw files can contain rejected confidential content. Do not upload the bundle +as a public CI artifact. No environment dump, authentication token, full runtime +configuration or OCR session is copied into it. Configuration observations name +the selected protocol, language, review effort and logging flags; the model +selection is represented by a SHA-256 fingerprint, not provider acceptance. + +The journal records passed, failed, degraded and not-run checks at their execution +owners. Passing a collection phase does not imply complete repository coverage; +coverage remains a separate review fact. DLP entries describe real detection, +omission or redaction branches, with bounded locations, sizes and value digests. +They are not the results of a second DLP scan and do not authorize publication. +At most 1,000 DLP decisions and 128 phase transitions are retained; omitted counts +remain explicit. Unknown field names are fingerprinted rather than copied. + +Capture metadata distinguishes missing, unavailable and not-run artifacts. +Truncation and source changes are explicit. `sha256_captured` covers only the +retained bytes, never an implied complete source. A truncated Markdown/JSON copy +may end inside a UTF-8 character or JSON value; use the normal artifact for the +complete admitted output. + +`complete: true` means the journal's observation lifecycle finished, not that the +review succeeded. Initial snapshots have `complete: false`. An unsafe initial +directory fails before review. Later diagnostic write failures do not replace the +review outcome or suppress cleanup: unavailable captures are marked in the journal, +and a journal write failure emits a bounded stderr warning. An earlier incomplete +snapshot can remain when the final journal cannot be written. + +## Configured external qualification + +Deterministic installed tests and real-OCR no-LLM probes do not establish model +quality, live GitLab behavior or provider reasoning support. Before promoting a +Draft, the external owner should record the exact Draft head, installed artifact +digest, OCR asset identity and provider/model/protocol for these checks. Do not +install the older stable toolkit pin from the GitLab example for this exercise; +use the wheel built by the exact Draft's Build artifacts workflow in an isolated +environment. Keep the example's stable toolkit pin unchanged until release. + +- GitLab: run the configured review and inspect admitted findings, mandatory + evidence use, receipt identity and the intended discussion/approval behavior + in an authorized test project. A green advisory job is not proof of model use. +- Local: review the same immutable change with `--local`, confirm no forge + acquisition/publication, and compare admitted JSON, the complete private + Markdown artifact and console output. Include a clean case and a finding case. +- Debug: repeat with a fresh `--debug-dir`, compare normalized outcomes with + ordinary local execution, and inspect actual journal phases, DLP decisions, + raw/safe separation, truncation and cleanup. Never publish raw private files. +- Progress: compare enabled/disabled outcomes and artifact contents. Only bounded + toolkit phases and heartbeats should be added to stderr. +- Reasoning: test unset, explicit `none` and the intended nonempty effort for + each selected provider/model/protocol. Record correct wire shape, explicit + provider acceptance/rejection and documented or server-observable application. + HTTP 200 and model prose alone do not prove the gateway applied the parameter. + Mark unprovable application as unverified; do not silently substitute a model, + effort value or weaker validation. Unset remains the no-overlay option. + +Record the outcome or explicit limitation for each scenario in the tracking +issues and Draft. This checklist is a handoff, not a claim that those external +checks have run or authorization to merge, release or close issues. diff --git a/docs/operations.md b/docs/operations.md index d1795da..c91afa8 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -1,5 +1,8 @@ # GitLab review operations +For local Markdown artifacts and console output without forge acquisition or publication, use the +[standalone local review](local.md) provider. + This guide is for developers and CI operators who connect Open Code Review Toolkit to a GitLab merge-request pipeline and need to understand what happens after the first review. Installation and the [production bot recipes](gitlab.md#production-bot-configuration) remain in `gitlab.md`; the complete environment contract is in [configuration.md](configuration.md), and enriched acquisition is in [bounded review context](review-context.md). OCR behavior below refers to the exact recommended release in the diff --git a/docs/review-decision-flow.md b/docs/review-decision-flow.md index 04a3e6c..0d061e4 100644 --- a/docs/review-decision-flow.md +++ b/docs/review-decision-flow.md @@ -1,5 +1,13 @@ # Review decision flow +With `review --local --debug-dir`, a private observer records these same checks; +it does not add an execution or admission branch. Raw output is captured before +finalization, safe output only after admission, and the journal records cleanup +and successful or failed report delivery. See [local debug bundles](local.md#private-debug-bundle) +for bounds, missing artifacts and incomplete journal semantics. Local report and +debug delivery pin their private parent directory before creating entries, so a +same-user replacement of a visible ancestor cannot redirect a retained artifact. + This document is the canonical visual map of the toolkit's end-to-end review decisions. It connects configuration and immutable identity, OCR execution, result and action-receipt validation, additive diagnostics, publication DLP, GitLab posting, and optional later actions. @@ -24,6 +32,44 @@ and every affected public contract together before release. The same palette is repeated in every diagram so a terminal state never changes meaning between views. +## Optional progress observer + +`OCR_REVIEW_PROGRESS` is an observer, not an input to OCR or an execution gate. +It can emit only closed toolkit phase labels and never reads review content, +artifacts, or session state. A conventional GitLab stderr job-log pipe remains a +valid review transport but is not a valid optional-progress transport: the +observer stops instead of risking a blocked review or changing the caller-owned +file-descriptor flags. + +```mermaid +flowchart TD + requested[Review starts] --> enabled{Progress explicitly enabled?} + enabled -- No --> absent[No observer] + enabled -- Yes --> stream{Safe output transport?} + stream -- Interactive terminal --> terminal[Open an owned nonblocking terminal writer] + stream -- Explicitly nonblocking --> nonblocking[Use the supplied nonblocking stream] + stream -- Blocking pipe or unavailable --> disabled[Disable optional observer] + terminal --> emit[Emit bounded closed phase and heartbeat messages] + nonblocking --> emit + emit --> writable{Write accepted?} + writable -- Yes --> next[Continue until review ends or message cap] + writable -- Full or closed --> disabled + next --> stopped[Stop and release observer] + disabled --> review[Review continues unchanged] + absent --> review + stopped --> review + + classDef success fill:#d1fae5,stroke:#15803d,color:#14532d,stroke-width:2px; + classDef warning fill:#ffedd5,stroke:#ea580c,color:#7c2d12,stroke-width:2px; + classDef error fill:#fee2e2,stroke:#dc2626,color:#7f1d1d,stroke-width:2px; + classDef auxiliary fill:#f3f4f6,stroke:#6b7280,color:#1f2937; + classDef decision fill:#dbeafe,stroke:#2563eb,color:#1e3a8a; + + class emit,next success; + class disabled,absent,stopped,review,requested,terminal,nonblocking auxiliary; + class enabled,stream,writable decision; +``` + ## End-to-end control flow ```mermaid @@ -52,6 +98,14 @@ flowchart TD partial --> mode mode -- Local diagnostic retention --> local[Owner-only artifacts retained;
no provider receipt or posting authority] mode -- Local ordinary --> local_result[Validated receipt-less local result] + local_result --> local_output{Explicit local provider?} + local_output -- Yes --> local_file[Shared admitted summary and all findings
to fresh private Markdown artifact and console] + local_file --> local_delivery{Artifact and console delivery succeed?} + local_delivery -- Yes --> local_done[Local delivery complete;
no platform actions] + local_delivery -- No --> local_error[Nonzero delivery failure;
preserve any completed artifact] + runtime_error -- Explicit local --> local_failure[Closed failure summary to console;
artifact if destination was accepted] + integrity_error -- Explicit local --> local_failure + local_output -- No --> local_handoff[Private JSON handoff only] mode -- GitLab MR --> receipt[Attach exact receipt v8] receipt --> post{Posting input valid at readback?} post -- No --> posting_error[Publication-policy error;
findings transaction not started] diff --git a/docs/security.md b/docs/security.md index 27b6563..98c27ab 100644 --- a/docs/security.md +++ b/docs/security.md @@ -26,6 +26,27 @@ An operator controls CI configuration, direct MCP servers, adapter commands/endp 8. The complete successful OCR result crosses independent publication DLP, deterministic GitLab publication, suppression, and receipt-v8 later-action policy. Receipt v8 binds exact source/target identity and actual protection state. Additive failed-tool diagnostics can limit a later action but cannot suppress a valid review publication; dynamic detail is console-only. An unprotected receipt can add only the static limitation and is structurally barred from the approval executor. On non-zero OCR exit with a valid bounded `ocr.llm-retry-report/v1`, only a closed provider-neutral reason may cross into a toolkit-authored failure note; one separate local line may contain closed protocol detail, a shared HTTP status, and bounded aggregate counts from the same strict parser. Raw result fields and stderr remain private, normal findings are ignored, and approval is unreachable. Missing or invalid retry diagnostics use the pre-existing generic failure boundary. See the [review decision flow](review-decision-flow.md) for the complete branch ordering. 9. A release candidate crosses protected-base authorization, publication, provenance, and live readback. +### Local delivery and diagnostic observations + +Explicit local execution ignores inherited forge identity and has no provider +acquisition or mutation channel. It still requires the common immutable Git, +mandatory MCP, result admission, DLP and cleanup checks. Shared reporting consumes +admitted review facts; a local Markdown file is not a forge publication receipt. +Fresh private report publication rejects existing targets and output collisions. + +The optional debug bundle is a distinct sensitive retention surface. Raw OCR +result/stderr may contain rejected content; owner-only permissions are not +redaction. Bounded descriptor-pinned captures and value-free observations of +actual DLP decisions do not authorize egress or prove host integrity. No session, +environment dump or full configuration is copied. A journal's completion flag +describes observation completion, not review success. Same-user host compromise, +host failure and operator disclosure of raw artifacts remain residual risks. +See [local debug semantics](local.md#private-debug-bundle) for bounds and failures. + +Progress emits a closed bounded toolkit phase vocabulary without reading raw +results or stderr. Reasoning configuration validates protocol shape and conflicts; +it does not establish model capability or that a gateway applied an accepted value. + ### Security objectives - Repository and provider-controlled content remains data. It cannot promote itself into policy, tools, permissions, suppression, posting, approval, or release authority. diff --git a/examples/gitlab/ocr-review.gitlab-ci.yml b/examples/gitlab/ocr-review.gitlab-ci.yml index 29e7b42..6a660b6 100644 --- a/examples/gitlab/ocr-review.gitlab-ci.yml +++ b/examples/gitlab/ocr-review.gitlab-ci.yml @@ -6,10 +6,10 @@ default: image: python:3.12-slim variables: - OCR_VERSION: "v1.11.5" + OCR_VERSION: "v1.11.6" OCR_TOOLKIT_VERSION: "0.9.1" OCR_TOOLKIT_CHECKSUMS_URL: "https://github.com/xeonvs/open-code-review-toolkit/releases/download/v${OCR_TOOLKIT_VERSION}/SHA256SUMS" - OCR_SHA256: "53a4ab7c8ce6dc07d5362c7c4984bf8d98b55e4e8d4c01b9399d488a2a983d95" + OCR_SHA256: "09f30595834f8297a592b51bf4707fb24728b2826a65915f1964c5563e4fb3bd" OCR_POST_MODE: "draft" OCR_STRICT_POSTING: "true" # Secure default: reject an unprotected GitLab target before OCR. @@ -28,6 +28,8 @@ variables: OCR_MAX_TOOLS: "0" # Optional aggregate review ceiling; 0 keeps OCR's unlimited default. OCR_MAX_TOKENS_BUDGET: "0" + # Do not set OCR_REVIEW_PROGRESS for the standard GitLab stderr job-log pipe: + # optional progress is safely unavailable there rather than risking a blocked review. # Reviews default to English. To request another language, for example Russian, set: # OCR_REVIEW_LANGUAGE: "Russian" diff --git a/scripts/ocr_compat.py b/scripts/ocr_compat.py index 00504f7..33e76b6 100644 --- a/scripts/ocr_compat.py +++ b/scripts/ocr_compat.py @@ -134,7 +134,13 @@ "native/source.cxx": "**/*.{cpp,cc,cxx,hpp,hxx}", "native/header.hxx": "**/*.{cpp,cc,cxx,hpp,hxx}", "native/object.mm": "**/*.mm", + "src/parser.ml": "**/*.{ml,mli}", + "src/parser.mli": "**/*.{ml,mli}", + "src/component.re": "**/*.{re,rei}", + "src/component.rei": "**/*.{re,rei}", + "scripts/setup.kts": "**/*.{kt,kts}", } +CURRENT_DEFAULT_EXCLUDED_PATHS = ("src/test/kotlin/scripts/Example.kts", "test/parser.ml") REQUIRED_ASSETS = { "opencodereview-darwin-amd64", @@ -350,6 +356,7 @@ def _validate_current_contracts(value: object) -> None: }, "language_rule_probe": { "excluded_extensions": [".svh"], + "default_excluded_paths": list(CURRENT_DEFAULT_EXCLUDED_PATHS), "extensions": extensions, "result": "passed", "rule_source": "system_builtin", @@ -362,6 +369,13 @@ def _validate_current_contracts(value: object) -> None: "result": "passed", "wire_field": "max_completion_tokens", }, + "reasoning_effort_probe": { + "result": "passed", + "protocols": ["openai", "openai-responses"], + "efforts": ["unset", "none", "high"], + "responses_siblings_preserved": True, + "provider_acceptance": "not-tested", + }, "comment_arguments_probe": { "result": "passed", "intact_cases": ["array", "serialized", "repaired"], @@ -841,6 +855,9 @@ class _StubHandler(http.server.BaseHTTPRequestHandler): request_stages: list[str] = [] prior_finding_semantics: set[str] = set() recheck_instruction_requests = 0 + capture_reasoning = False + reasoning_requests: list[dict[str, object]] = [] + reasoning_overflow = False @staticmethod def _message_contents(messages: list[Any]) -> list[str]: @@ -880,7 +897,10 @@ def _review_path(cls, messages: list[Any]) -> str | None: return None def do_POST(self) -> None: - if self.path != "/v1/chat/completions": + allowed_paths = {"/v1/chat/completions"} + if type(self).capture_reasoning: + allowed_paths.add("/v1/responses") + if self.path not in allowed_paths: self.send_error(404) return length = int(self.headers.get("Content-Length", "0")) @@ -895,6 +915,27 @@ def do_POST(self) -> None: if not isinstance(request, dict): self.send_error(400) return + if type(self).capture_reasoning: + reasoning = request.get("reasoning") + if len(type(self).reasoning_requests) < 32: + type(self).reasoning_requests.append( + { + "path": self.path, + "root_present": "reasoning_effort" in request, + "root_effort": request.get("reasoning_effort"), + "nested_present": isinstance(reasoning, dict) and "effort" in reasoning, + "nested_effort": reasoning.get("effort") + if isinstance(reasoning, dict) + else None, + "summary": reasoning.get("summary") + if isinstance(reasoning, dict) + else None, + } + ) + else: + type(self).reasoning_overflow = True + self.send_error(400, "Synthetic request capture; no model execution") + return type(self).completion_caps.append(request.get("max_completion_tokens")) messages = request.get("messages") if not isinstance(messages, list): @@ -1053,6 +1094,7 @@ def _stub_gateway( grouping_mode: str = "singletons", main_mode: str = "findings", comment_mode: str = "default", + capture_reasoning: bool = False, ) -> Iterator[str]: """Serve deterministic responses with configurable real usage accounting.""" @@ -1075,6 +1117,9 @@ def _stub_gateway( _StubHandler.request_stages = [] _StubHandler.prior_finding_semantics = set() _StubHandler.recheck_instruction_requests = 0 + _StubHandler.capture_reasoning = capture_reasoning + _StubHandler.reasoning_requests = [] + _StubHandler.reasoning_overflow = False server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _StubHandler) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() @@ -1900,6 +1945,100 @@ def _completion_cap_probe(binary: Path, directory: Path) -> dict[str, object]: } +def _reasoning_effort_probe(binary: Path, directory: Path) -> dict[str, object]: + """Capture real OCR effort fields without claiming provider/model acceptance.""" + + from ocr_toolkit.config_writer import write_ocr_config + from ocr_toolkit.provider_config import request_controls_from_environment + + root = directory / "reasoning-effort-probe" + root.mkdir() + git_env = _isolated_probe_environment(root / "git-home") + repo, base, head = _synthetic_repo(root, git_env) + for protocol in ("openai", "openai-responses"): + for effort in ("", "none", "high"): + label = effort or "unset" + env = _isolated_probe_environment(root / f"{protocol}-{label}-home") + settings = {"OCR_LLM_PROTOCOL": protocol, "OCR_LLM_REASONING_EFFORT": effort} + if protocol == "openai-responses" and effort: + settings["OCR_LLM_EXTRA_BODY"] = '{"reasoning":{"summary":"auto"}}' + controls = request_controls_from_environment(settings) + with _stub_gateway(capture_reasoning=True) as gateway_url: + config: dict[str, object] = { + "auth_token": "synthetic-token", + "model": "synthetic-model", + "protocol": protocol, + "url": gateway_url, + "use_anthropic": False, + } + if controls.extra_body is not None: + config["extra_body"] = controls.extra_body + write_ocr_config( + {"llm": config, "telemetry": {"enabled": False}}, + Path(env["HOME"]) / ".opencodereview" / "config.json", + ) + try: + completed = subprocess.run( # nosec B603 + [ + str(binary), + "review", + "--from", + base, + "--to", + head, + "--format", + "json", + "--audience", + "agent", + "--effort", + "low", + "--concurrency", + "1", + ], + cwd=repo, + env=env, + check=False, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=30, + ) + except (OSError, subprocess.TimeoutExpired) as exc: + raise CompatibilityError( + f"reasoning wire probe did not finish: {protocol}/{label}" + ) from exc + if completed.returncode < 0: + _fail(f"reasoning wire probe was interrupted: {protocol}/{label}") + observed = list(_StubHandler.reasoning_requests) + overflow = _StubHandler.reasoning_overflow + if overflow: + _fail( + f"reasoning wire probe exceeded its request capture limit: {protocol}/{label}" + ) + if not observed: + _fail(f"reasoning wire probe received no request: {protocol}/{label}") + expected_path = "/v1/chat/completions" if protocol == "openai" else "/v1/responses" + prefix = "root" if protocol == "openai" else "nested" + other_prefix = "nested" if protocol == "openai" else "root" + for request in observed: + if ( + request["path"] != expected_path + or request[f"{other_prefix}_present"] is not False + or request[f"{prefix}_present"] is not bool(effort) + or request[f"{prefix}_effort"] != (effort or None) + or (protocol == "openai-responses" and effort and request["summary"] != "auto") + ): + _fail( + f"reasoning wire fields differ from configured controls: {protocol}/{label}" + ) + return { + "result": "passed", + "protocols": ["openai", "openai-responses"], + "efforts": ["unset", "none", "high"], + "responses_siblings_preserved": True, + "provider_acceptance": "not-tested", + } + + def _preview_file_selection(payload: object, path: str) -> tuple[bool, object]: """Return one JSON preview file's selected state and closed exclusion reason.""" @@ -2027,7 +2166,7 @@ def _language_rule_probe(binary: Path, directory: Path) -> dict[str, object]: qualified_rules = set(exact_patterns) supported_paths = tuple(sorted(qualified_rules)) unsupported_path = "rtl/include.svh" - paths = (*supported_paths, unsupported_path) + paths = (*supported_paths, unsupported_path, *CURRENT_DEFAULT_EXCLUDED_PATHS) for path in paths: target = repo / path target.parent.mkdir(parents=True, exist_ok=True) @@ -2070,6 +2209,10 @@ def _language_rule_probe(binary: Path, directory: Path) -> dict[str, object]: unsupported_selected, unsupported_reason = _preview_file_selection(payload, unsupported_path) if unsupported_selected or unsupported_reason != "unsupported_ext": _fail("candidate unexpectedly selected the unqualified .svh extension") + for path in CURRENT_DEFAULT_EXCLUDED_PATHS: + selected_test, reason = _preview_file_selection(payload, path) + if selected_test or not isinstance(reason, str) or not reason: + _fail(f"candidate did not preserve the default test-path exclusion: {path}") for path in supported_paths: output = _run([str(binary), "rules", "check", path], cwd=repo, env=env) if ( @@ -2088,6 +2231,7 @@ def _language_rule_probe(binary: Path, directory: Path) -> dict[str, object]: result: dict[str, object] = { "extensions": expected_extensions, "excluded_extensions": [".svh"], + "default_excluded_paths": list(CURRENT_DEFAULT_EXCLUDED_PATHS), "result": "passed", "rule_source": "system_builtin", "selected": len(supported_paths), @@ -2373,6 +2517,7 @@ def run_contracts(binary: Path, version: str, directory: Path) -> dict[str, Any] contracts["small_change_grouping_probe"] = _small_change_grouping_probe(binary, directory) contracts["language_rule_probe"] = _language_rule_probe(binary, directory) contracts["completion_cap_probe"] = _completion_cap_probe(binary, directory) + contracts["reasoning_effort_probe"] = _reasoning_effort_probe(binary, directory) contracts["comment_arguments_probe"] = _comment_arguments_probe(binary, directory) contracts["comment_thinking_probe"] = thinking_probe _validate_current_contracts(contracts) @@ -2725,10 +2870,9 @@ def prepare_update( _fail("human conclusions may reference only evidence versions in this promotion") for item in evidences: - if _version(str(item["version"])) >= history.HISTORICAL_CUTOFF: - if item.get("schema_version") != 3: - _fail("current candidate requires evidence schema 3") - _validate_current_contracts(item.get("contracts")) + if item.get("schema_version") != 3: + _fail("current candidate requires evidence schema 3") + _validate_current_contracts(item.get("contracts")) version = versions[-1] releases = manifest.get("releases") diff --git a/scripts/ocr_compat_history.py b/scripts/ocr_compat_history.py index 44ebee7..696213b 100644 --- a/scripts/ocr_compat_history.py +++ b/scripts/ocr_compat_history.py @@ -5,7 +5,7 @@ from collections.abc import Callable from typing import Any, NoReturn -HISTORICAL_CUTOFF = (1, 11, 4) +HISTORICAL_CUTOFF = (1, 11, 6) ARCHIVED_NUMERIC_CLI_CONTRACT: dict[str, object] = { "max_tokens_budget": { @@ -41,6 +41,130 @@ } +ARCHIVED_1_11_4_CONTRACTS: dict[str, object] = { + "numeric_cli_probe": ARCHIVED_NUMERIC_CLI_CONTRACT, + "comment_arguments_probe": { + "comments_per_batch": 2, + "failure_arguments_preserved": True, + "intact_cases": ["array", "serialized", "repaired"], + "repair_warning": True, + "result": "passed", + "suspect_batch_rejected": True, + }, + "comment_thinking_probe": { + "additive_field_preserved": True, + "posting_exposes_thinking": False, + "result": "passed", + }, + "completion_cap_probe": { + "explicit": 4096, + "inherited": 16384, + "result": "passed", + "wire_field": "max_completion_tokens", + }, + "language_rule_probe": { + "excluded_extensions": [".svh"], + "extensions": [ + ".cjs", + ".cxx", + ".hxx", + ".mjs", + ".mm", + ".pug", + ".sv", + ".v", + ".vh", + ".vhd", + ".vhdl", + ], + "m_routing": "matlab_and_objective_c", + "result": "passed", + "rule_source": "system_builtin", + "selected": 11, + }, + "optional_capabilities": [ + "llm_result_identity", + "per_run_model_override", + "per_run_provider_override", + "review_effort", + "semantic_grouping", + ], + "preview_probe": { + "format": "json", + "path": "example.py", + "result": "passed", + "session_store_created": False, + }, + "required_review_flags": [ + "--audience", + "--background-file", + "--effort", + "--format", + "--from", + "--max-tokens-budget", + "--max-tools", + "--preview", + "--rule", + "--to", + ], + "result_contract_probe": { + "additive_fields_allowed": True, + "comment_fields": [ + "category", + "content", + "end_line", + "existing_code", + "path", + "severity", + "start_line", + "thinking", + ], + "manifest_schema": "ocr.run-manifest/v1", + "normalized_outcome": "clean", + "result": "passed", + }, + "review_budget_probe": { + "budget": 30000, + "completed": 2, + "failed_budget": 1, + "grouping_requests": 0, + "grouping_strategy": "per_file", + "partial_findings_preserved": True, + "result": "passed", + "selected": 3, + }, + "semantic_grouping_probe": { + "default_effort": "medium", + "files": 4, + "filter_requests": 1, + "grouping_completion_cap": 16384, + "grouping_requests": 1, + "main_requests": 3, + "prior_finding_semantics": "filter_survivors_as_confirmed", + "recheck_instruction_requests": 3, + "result": "passed", + "review_rounds": 2, + }, + "small_change_grouping_probe": { + "grouping_requests": 0, + "high_churn": "per_file", + "low_churn": "bundle_all", + "result": "passed", + "single_file": "per_file", + "threshold_files": 4, + }, + "target_rule_selection_probe": { + "format": "json", + "from_to_unchanged": True, + "path": "synthetic-template.ocrfixture", + "result": "passed", + "source_exclusion": "unsupported_ext", + "target_selected": True, + }, + "version_probe": "passed", +} + + def language_extensions(version_tuple: tuple[int, int, int]) -> list[str]: """Return the language inventory recorded in the historical evidence epochs.""" @@ -60,6 +184,10 @@ def validate_contracts( if version_tuple >= HISTORICAL_CUTOFF: fail("current evidence cannot use historical validation") + if version_tuple >= (1, 11, 4): + if evidence.get("contracts") != ARCHIVED_1_11_4_CONTRACTS: + fail(f"historical qualification contract disagrees for {version}") + return if version_tuple >= (1, 9, 5): contracts = evidence.get("contracts") required_flags = ( diff --git a/src/ocr_toolkit/cli.py b/src/ocr_toolkit/cli.py index db38ad9..0765d43 100644 --- a/src/ocr_toolkit/cli.py +++ b/src/ocr_toolkit/cli.py @@ -24,7 +24,12 @@ def build_parser() -> argparse.ArgumentParser: version=f"%(prog)s {__version__}", ) subparsers = parser.add_subparsers(dest="command", required=True) - subparsers.add_parser("preflight", help="Validate OCR, GitLab, and LLM access.") + preflight_parser = subparsers.add_parser( + "preflight", help="Validate review prerequisites and configured access." + ) + preflight_parser.add_argument( + "--local", action="store_true", help="Validate standalone local review prerequisites." + ) subparsers.add_parser("configure", help="Write the OCR runtime configuration.") subparsers.add_parser("mcp-config", help="Write OCR MCP server configuration.") review_parser = subparsers.add_parser( @@ -32,6 +37,15 @@ def build_parser() -> argparse.ArgumentParser: ) review_parser.add_argument("--result", required=True, help="OCR JSON output path.") review_parser.add_argument("--stderr", required=True, help="Full OCR stderr artifact path.") + review_parser.add_argument( + "--local", action="store_true", help="Review immutable Git refs and publish local Markdown." + ) + review_parser.add_argument( + "--report", help="Fresh local Markdown path (default: --result path plus .md)." + ) + review_parser.add_argument( + "--debug-dir", help="Fresh private diagnostic bundle directory (requires --local)." + ) review_parser.add_argument( "--preserve-private-artifacts", action="store_true", @@ -62,7 +76,7 @@ def main(argv: Sequence[str] | None = None) -> int: args = build_parser().parse_args(argv) if args.command == "preflight": - return preflight.main() + return preflight.main(local=True) if args.local else preflight.main() if args.command == "configure": return configure.main() if args.command == "mcp-config": @@ -75,6 +89,9 @@ def main(argv: Sequence[str] | None = None) -> int: Path(args.stderr), ocr_args, preserve_private_artifacts=args.preserve_private_artifacts, + **({"local": True} if args.local else {}), + **({"report_path": Path(args.report)} if args.report is not None else {}), + **({"debug_dir": Path(args.debug_dir)} if args.debug_dir is not None else {}), ) except review_runner.ReviewRunnerError as exc: print(f"Cannot run Open Code Review: {exc}", file=sys.stderr) diff --git a/src/ocr_toolkit/common/filesystem.py b/src/ocr_toolkit/common/filesystem.py index cf4468f..70d6939 100644 --- a/src/ocr_toolkit/common/filesystem.py +++ b/src/ocr_toolkit/common/filesystem.py @@ -4,6 +4,8 @@ import errno import os +import stat +from pathlib import Path def fsync_directory(descriptor: int) -> None: @@ -17,3 +19,50 @@ def fsync_directory(descriptor: int) -> None: # descriptor/filesystem cases; propagate genuine durability failures. if exc.errno not in {errno.EINVAL, errno.ENOTSUP, errno.EBADF}: raise + + +def open_private_parent_directory(path: Path) -> tuple[int, str]: + """Open a path's parent without following replaceable pathname components. + + The returned descriptor pins the parent directory. Callers must use + descriptor-relative operations and close it themselves; returning an + absolute pathname would reintroduce the ancestor replacement race this + helper avoids. + """ + + absolute = path.absolute() + parts = [part for part in absolute.parts if part != os.path.sep] + if not parts: + raise ValueError("private output path must name a file or directory") + descriptor = os.open(os.path.sep, os.O_RDONLY | os.O_DIRECTORY) + try: + for component in parts[:-1]: + flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW + try: + child = os.open(component, flags, dir_fd=descriptor) + except FileNotFoundError: + try: + os.mkdir(component, mode=0o700, dir_fd=descriptor) + except FileExistsError: + pass + try: + child = os.open(component, flags, dir_fd=descriptor) + except OSError as exc: + raise ValueError("private output parent must not be a symlink") from exc + except OSError as exc: + raise ValueError("private output parent must not be a symlink") from exc + os.close(descriptor) + descriptor = child + parent = os.fstat(descriptor) + if not stat.S_ISDIR(parent.st_mode): + raise OSError("private output parent is not a directory") + return descriptor, parts[-1] + except BaseException: + os.close(descriptor) + raise + + +def same_file_identity(left: os.stat_result, right: os.stat_result) -> bool: + """Compare two pinned filesystem identities without following pathnames.""" + + return left.st_dev == right.st_dev and left.st_ino == right.st_ino diff --git a/src/ocr_toolkit/posting/comments.py b/src/ocr_toolkit/posting/comments.py index 77ae58e..f1a3f93 100644 --- a/src/ocr_toolkit/posting/comments.py +++ b/src/ocr_toolkit/posting/comments.py @@ -1,87 +1,21 @@ -"""Small helpers for OCR comment value normalization.""" - -from __future__ import annotations - -import re -from typing import Any - -from ocr_toolkit.common.markdown import escape_control_chars - -MARKDOWN_INLINE_SPECIAL_RE = re.compile(r"([`*_{}\[\]()+.!|~-])") - - -def clean_text(value: Any) -> str: - """Convert a JSON value to a stripped string.""" - - return "" if value is None else str(value).strip() - - -def compact_text(value: str, max_chars: int) -> str: - """Collapse whitespace and bound text for one-line MR summaries.""" - - if max_chars <= 0: - return "" - - collapsed = " ".join(value.split()) - if len(collapsed) <= max_chars: - return collapsed - - if max_chars <= 3: - return "." * max_chars - - return collapsed[: max_chars - 3].rstrip() + "..." - - -def compact_escaped_text(value: str, max_chars: int) -> str: - """Escape Markdown-sensitive text before compacting MR summary snippets.""" - - escaped = escape_control_chars(value) - escaped = escaped.replace("&", "&").replace("<", "<").replace(">", ">") - escaped = escaped.replace("@", "@") - escaped = MARKDOWN_INLINE_SPECIAL_RE.sub(r"\\\1", escaped) - return compact_text(escaped, max_chars) - - -def compact_control_text(value: str, max_chars: int) -> str: - """Escape controls only before compacting text rendered inside inline code.""" - - return compact_text(escape_control_chars(value), max_chars) - - -def code_text(value: Any) -> str: - """Convert a JSON value to text while preserving code indentation.""" - - if value is None: - return "" - return str(value).rstrip("\n") - - -def line_number(value: Any) -> int: - """Parse a line number, returning zero when invalid.""" - - if isinstance(value, bool): - return 0 - if isinstance(value, int): - parsed = value - elif isinstance(value, str): - text = value.strip() - if not text.isdecimal(): - return 0 - parsed = int(text) - else: - return 0 - return parsed if parsed > 0 else 0 - - -def comment_line(comment: dict[str, Any]) -> int: - """Return the best new-line number for a GitLab inline discussion. - - Anchor on the start of the range so multi-line findings highlight the - first problematic line rather than the end of the span. - """ - - for key in ("start_line", "line", "end_line"): - parsed = line_number(comment.get(key)) - if parsed > 0: - return parsed - return 0 +"""Posting facade for shared report value normalization.""" + +from ocr_toolkit.reporting.text import ( + clean_text, + code_text, + comment_line, + compact_control_text, + compact_escaped_text, + compact_text, + line_number, +) + +__all__ = [ + "clean_text", + "code_text", + "comment_line", + "compact_control_text", + "compact_escaped_text", + "compact_text", + "line_number", +] diff --git a/src/ocr_toolkit/posting/formatting.py b/src/ocr_toolkit/posting/formatting.py index 16226e7..3e85739 100644 --- a/src/ocr_toolkit/posting/formatting.py +++ b/src/ocr_toolkit/posting/formatting.py @@ -16,15 +16,6 @@ from ocr_toolkit.common.markdown import ( inline_code as _inline_code, ) -from ocr_toolkit.common.redaction import redact_sensitive -from ocr_toolkit.evidence.actions import EVIDENCE_ACTIONS -from ocr_toolkit.ocr_result import ( - MAX_TOOLKIT_MCP_USAGE_COUNT, - MAX_TOOLKIT_MCP_USAGE_SERVERS, - PUBLIC_REVIEW_TOOL_CALL_NAMES, - TOOLKIT_MCP_SERVER_NAME_RE, - OcrToolkitAdvisory, -) from ocr_toolkit.posting.approval import ( ApprovalResult, approval_summary_line, @@ -39,7 +30,7 @@ line_number, ) from ocr_toolkit.posting.payloads import truncate_code_text, truncate_note_body -from ocr_toolkit.posting.result import CoverageDiagnostics, ocr_warning_text +from ocr_toolkit.posting.result import CoverageDiagnostics from ocr_toolkit.posting.settings import ( FALLBACK_NOTE_CHUNK_BUDGET, MAX_FALLBACK_CODE_DETAILS_CHARS, @@ -57,49 +48,38 @@ SuggestionState, safe_repository_path, ) -from ocr_toolkit.result_usage import normalize_token_usage -from ocr_toolkit.review_receipt import toolkit_receipt_is_valid - -OCR_FINDING_CATEGORIES = { - "bug", - "security", - "performance", - "maintainability", - "test", - "style", - "documentation", - "other", -} - -OCR_FINDING_SEVERITIES = {"critical", "high", "medium", "low"} -OCR_FINDING_SEVERITY_ORDER = ("critical", "high", "medium", "low") -OCR_FINDING_CATEGORY_ORDER = ( - "security", - "bug", - "performance", - "maintainability", - "test", - "documentation", - "style", - "other", +from ocr_toolkit.reporting.metadata import ( + CATEGORY_EMOJI as CATEGORY_EMOJI, ) - -SEVERITY_EMOJI = { - "critical": "❌", - "high": "🚨", - "medium": "⚠️", - "low": "ℹ️", # noqa: RUF001 - intentional information emoji -} -CATEGORY_EMOJI = { - "bug": "🐛", - "security": "🔒", - "performance": "⚡", - "maintainability": "🛠️", - "test": "🧪", - "style": "🎨", - "documentation": "📚", - "other": "📌", -} +from ocr_toolkit.reporting.metadata import ( + OCR_FINDING_CATEGORIES as OCR_FINDING_CATEGORIES, +) +from ocr_toolkit.reporting.metadata import ( + OCR_FINDING_CATEGORY_ORDER, + OCR_FINDING_SEVERITY_ORDER, + finding_metadata, +) +from ocr_toolkit.reporting.metadata import ( + OCR_FINDING_SEVERITIES as OCR_FINDING_SEVERITIES, +) +from ocr_toolkit.reporting.metadata import ( + SEVERITY_EMOJI as SEVERITY_EMOJI, +) +from ocr_toolkit.reporting.metadata import format_ocr_core_advisory as format_ocr_core_advisory +from ocr_toolkit.reporting.metadata import ( + format_token_usage_summary as format_token_usage_summary, +) +from ocr_toolkit.reporting.metadata import ( + normalized_ocr_metadata as normalized_ocr_metadata, +) +from ocr_toolkit.reporting.outcome import FindingVisibility, review_outcome_line +from ocr_toolkit.reporting.sections import report_sections +from ocr_toolkit.reporting.usage import format_tool_calls_summary as format_tool_calls_summary +from ocr_toolkit.reporting.usage import format_verified_mcp_usage +from ocr_toolkit.reporting.usage import nonnegative_int as nonnegative_int +from ocr_toolkit.reporting.usage import tool_call_counts_from_items as tool_call_counts_from_items +from ocr_toolkit.reporting.usage import tool_call_name as tool_call_name +from ocr_toolkit.review_receipt import toolkit_receipt_is_valid SHIELDS_BADGE_BASE_URL = "https://img.shields.io/badge" SHIELDS_SEVERITY_COLORS = { @@ -121,23 +101,6 @@ def inline_code(value: str) -> str: return _inline_code(value, escape_controls=True) -def normalized_ocr_metadata(value: Any, allowed_values: set[str]) -> str: - """Return a whitelisted OCR metadata value suitable for display.""" - - text = clean_text(value).casefold() - return text if text in allowed_values else "" - - -def finding_metadata(comment: dict[str, Any]) -> tuple[str, str]: - """Return structured OCR category/severity metadata from a finding.""" - - severity = normalized_ocr_metadata( - comment.get("severity"), OCR_FINDING_SEVERITIES - ) or normalized_ocr_metadata(comment.get("priority"), OCR_FINDING_SEVERITIES) - category = normalized_ocr_metadata(comment.get("category"), OCR_FINDING_CATEGORIES) - return severity, category - - def _finding_badge_label(*, severity: str, category: str) -> str: """Return a compact label built only from normalized closed enums.""" @@ -355,204 +318,16 @@ def format_metadata_counts( return ", ".join(parts) -def nonnegative_int(value: Any) -> int | None: - """Parse a non-negative integer from OCR JSON, ignoring malformed values.""" - - if isinstance(value, bool) or value is None: - return None - - if isinstance(value, int): - return value if value >= 0 else None - - if isinstance(value, float): - if value.is_integer() and value >= 0: - return int(value) - return None - - if isinstance(value, str): - try: - parsed = int(value.strip()) - except ValueError: - return None - return parsed if parsed >= 0 else None - - return None - - -def tool_call_name(value: Any) -> str: - """Extract one closed public tool name from common OCR call shapes.""" - - if isinstance(value, str): - 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 in PUBLIC_REVIEW_TOOL_CALL_NAMES: - return name - - function_value = value.get("function") - if isinstance(function_value, dict): - name = clean_text(function_value.get("name")) - return name if name in PUBLIC_REVIEW_TOOL_CALL_NAMES else "" - - return "" - - -def tool_call_counts_from_items( - items: list[Any], -) -> tuple[int | None, list[tuple[str, int]]]: - """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 - 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: - return None, [] - - return total, list(counts.items()) - - -def format_tool_calls_summary(tool_calls: Any) -> str: - """Return one bounded MR line for admitted non-zero OCR tool counts.""" - - entries: list[tuple[str, int]] - total: int | None - 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 = [] - admitted_total = 0 - - if isinstance(by_tool_value, dict): - for raw_name, raw_count in by_tool_value.items(): - if not isinstance(raw_name, str) or raw_name not in PUBLIC_REVIEW_TOOL_CALL_NAMES: - continue - if ( - not isinstance(raw_count, int) - or isinstance(raw_count, bool) - or not 0 < raw_count <= MAX_TOOLKIT_MCP_USAGE_COUNT - ): - continue - 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 not by_tool_value and isinstance(calls_value, list): - list_total, entries = tool_call_counts_from_items(calls_value) - else: - list_total = 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 entries: - total = admitted_total - else: - return "" - else: - return "" - - if total is None: - return "" - 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" - entries.sort(key=lambda item: (-item[1], item[0])) - detail_parts = [f"{inline_code(name)}: {count}" for name, count in entries] - - return f"{line} ({', '.join(detail_parts)})" - - def format_mcp_usage_summary(toolkit_metadata: Any) -> str: """Report MCP servers from the safe receipt produced by `ocr-ci review`.""" if not toolkit_receipt_is_valid(toolkit_metadata): return "" mcp = toolkit_metadata.get("mcp") - mcp_usage = mcp.get("usage") if isinstance(mcp, dict) else None - if ( - not isinstance(mcp_usage, dict) - or len(mcp_usage) > MAX_TOOLKIT_MCP_USAGE_SERVERS - or any( - not isinstance(server, str) - or TOOLKIT_MCP_SERVER_NAME_RE.fullmatch(server) is None - or not isinstance(count, int) - or isinstance(count, bool) - or not 0 < count <= MAX_TOOLKIT_MCP_USAGE_COUNT - for server, count in mcp_usage.items() - ) - ): - return "" - used = sorted(mcp_usage.items()) - if not used: - return "" - details = ", ".join(f"{inline_code(server)}: {count}" for server, count in used) - lines = [f"- reconciled MCP attempts: {len(used)} server(s) ({details})"] - evidence = toolkit_metadata.get("evidence") - actions = evidence.get("actions") if isinstance(evidence, dict) else None - if isinstance(actions, dict) and set(actions) == {"state", "attempted", "completed"}: - attempted = actions.get("attempted") - completed = actions.get("completed") - evidence_calls = evidence.get("calls") if isinstance(evidence, dict) else None - mandatory = evidence.get("mandatory") if isinstance(evidence, dict) else None - evidence_used = evidence.get("used") if isinstance(evidence, dict) else None - if not ( - actions.get("state") == "verified" - and isinstance(attempted, dict) - and set(attempted) == {*EVIDENCE_ACTIONS, "unattributed"} - and isinstance(completed, dict) - and set(completed) == set(EVIDENCE_ACTIONS) - and all( - isinstance(count, int) - and not isinstance(count, bool) - and 0 <= count <= MAX_TOOLKIT_MCP_USAGE_COUNT - for count in (*attempted.values(), *completed.values()) - ) - and isinstance(evidence_calls, int) - and not isinstance(evidence_calls, bool) - and 0 <= evidence_calls <= MAX_TOOLKIT_MCP_USAGE_COUNT - and isinstance(mandatory, bool) - and isinstance(evidence_used, bool) - and all(completed[action] <= attempted[action] for action in EVIDENCE_ACTIONS) - and sum(attempted.values()) == evidence_calls - and evidence_used is (sum(completed.values()) > 0) - and (not mandatory or completed["summary"] >= 1) - ): - return "\n".join(lines) - positive = [action for action in EVIDENCE_ACTIONS if completed[action] > 0] - if positive: - lines.append( - "- completed built-in evidence actions: " - + ", ".join(f"{action}: {completed[action]}" for action in positive) - ) - return "\n".join(lines) + return format_verified_mcp_usage( + mcp_usage=mcp.get("usage") if isinstance(mcp, dict) else None, + evidence=toolkit_metadata.get("evidence"), + ) def publication_dlp_signal( @@ -639,27 +414,6 @@ def format_publication_dlp_details(signal: dict[str, Any] | None) -> str: ) -def format_token_usage_summary(result: dict[str, Any]) -> str: - """Return one bounded MR summary line for structured OCR token usage.""" - - usage = normalize_token_usage(result) - if usage is None: - return "" - - total = usage.get("total") - details: list[str] = [] - for bucket in ("input", "output", "cached", "reasoning", "other"): - if (count := usage.get(bucket)) is not None and count > 0: - details.append(f"{bucket}: {count}") - - if total is None: - return f"- token usage: {', '.join(details)}" if details else "" - line = f"- token usage: {total} total" - if details: - line += f" ({', '.join(details)})" - return line - - SECURITY_SIGNAL_RE = re.compile( r"(?i)\b(" r"security|credential|secret|token|password|private[_ -]?token|" @@ -890,75 +644,15 @@ def _review_outcome_line( ) -> str: """Combine review health and finding publication into one visible status.""" - budget_stop = outcome_status == "budget_exceeded" or ( - outcome_status == "partial" and "budget" in outcome_message.casefold() - ) - partial_result = outcome_status in {"partial", "completed_with_errors", "budget_exceeded"} - has_finding_state = total > 0 or omitted_count > 0 or suppressed_count > 0 - if outcome_status == "skipped": - marker, status_text = "ℹ️", "Review skipped" # noqa: RUF001 - result_text = "no supported files changed" - elif outcome_status == "failed": - marker, status_text = "❌", "Review failed" - result_text = "no reliable review result was produced" - else: - if budget_stop: - marker, status_text = "⚠️", "Review stopped at token budget" - elif partial_result: - marker, status_text = "⚠️", "Review incomplete" - elif outcome_status == "publication-filtered": - marker, status_text = "⚠️", "Review complete with publication filtering" - elif outcome_status in {"warning", "completed_with_warnings"} or warning_count: - marker, status_text = "⚠️", "Review complete with warnings" - elif has_finding_state: - marker, status_text = "🔎", "Review complete" - else: - marker, status_text = "✅", "Review complete" - - if total: - noun = "finding" if total == 1 else "findings" - result_text = f"{total} {noun} published" - if partial_result: - result_text += " from reviewed files" - elif omitted_count: - result_text = ( - "no findings published from reviewed files" - if partial_result - else "no findings published" - ) - elif suppressed_count: - result_text = ( - "no new findings published from reviewed files" - if partial_result - else "no new findings published" - ) - elif partial_result: - result_text = "no findings in reviewed files" - else: - result_text = "no findings" - - if omitted_count: - noun = "finding" if omitted_count == 1 else "findings" - result_text += f"; {omitted_count} {noun} omitted by posting limit" - if suppressed_count: - noun = "finding" if suppressed_count == 1 else "findings" - result_text += f"; {suppressed_count} {noun} matched prior reviewer decisions" - if partial_result and diagnostics.file_count is not None: - noun = "file" if diagnostics.file_count == 1 else "files" - result_text += f"; {diagnostics.file_count} {noun} not reviewed" - - prefix = f"{marker} " if emoji else "" - return f"{prefix}**{status_text} — {result_text}**" - - -def format_ocr_core_advisory(advisory: OcrToolkitAdvisory | None) -> str: - """Render one validated numeric OCR advisory for Technical details only.""" - - if advisory is None: - return "" - return ( - f"- OCR core advisory: background {advisory.actual} characters; recommended " - f"{advisory.recommended} characters; accepted by OCR core" + return review_outcome_line( + findings=FindingVisibility( + count=total, published=True, omitted=omitted_count, suppressed=suppressed_count + ), + warning_count=warning_count, + outcome_status=outcome_status, + outcome_message=outcome_message, + unreviewed_file_count=diagnostics.file_count, + emoji=emoji, ) @@ -1014,51 +708,7 @@ def summarize_result( ] ) - severity_counts: dict[str, int] = {} - category_counts: dict[str, int] = {} - for comment in comments: - severity, category = finding_metadata(comment) - if severity: - severity_counts[severity] = severity_counts.get(severity, 0) + 1 - if category: - category_counts[category] = category_counts.get(category, 0) + 1 - if severity_counts or category_counts: - lines.extend(["", "### Findings", ""]) - for value in OCR_FINDING_SEVERITY_ORDER: - count = severity_counts.get(value, 0) - if count: - icon = f"{SEVERITY_EMOJI[value]} " if use_emoji else "" - lines.append(f"- {icon}{inline_code(value)}: {count}") - for value in OCR_FINDING_CATEGORY_ORDER: - count = category_counts.get(value, 0) - if count: - icon = f"{CATEGORY_EMOJI[value]} " if use_emoji else "" - lines.append(f"- {icon}{inline_code(value)}: {count}") - - if diagnostics.records or diagnostics.invalid or diagnostics.omitted: - lines.extend(["", "### Incomplete coverage", ""]) - for diagnostic in diagnostics.records: - detail = f" — {diagnostic.detail}" if diagnostic.detail else "" - lines.append(f"- {inline_code(diagnostic.path)} — {diagnostic.reason}{detail}") - if diagnostics.invalid: - lines.append( - f"- {diagnostics.invalid} failed item(s) had no safe repository-relative path" - ) - if diagnostics.omitted: - lines.append(f"- ... and {diagnostics.omitted} more failed file record(s)") - - safe_warnings = [] - for warning in warnings[:10]: - safe = compact_escaped_text( - neutralize_quick_actions(redact_sensitive(ocr_warning_text(warning))), 500 - ) - if safe: - safe_warnings.append(safe) - if safe_warnings and not diagnostics.records: - lines.extend(["", "### Review warnings", ""]) - lines.extend(f"- {warning}" for warning in safe_warnings) - if len(warnings) > len(safe_warnings): - lines.append(f"- ... and {len(warnings) - len(safe_warnings)} more warning(s)") + lines.extend(report_sections(comments, diagnostics, warnings, use_emoji=use_emoji)) if reviewer_guide: lines.extend(["", reviewer_guide.strip()]) diff --git a/src/ocr_toolkit/posting/result.py b/src/ocr_toolkit/posting/result.py index 25886a0..34b78f4 100644 --- a/src/ocr_toolkit/posting/result.py +++ b/src/ocr_toolkit/posting/result.py @@ -1,26 +1,20 @@ -"""OCR result artifact loading and provider failure classification.""" +"""Posting facade for shared OCR result diagnostics.""" -from __future__ import annotations - -import re -from collections.abc import Sequence -from dataclasses import dataclass -from pathlib import PurePosixPath -from typing import Any - -from ocr_toolkit.common.markdown import neutralize_quick_actions -from ocr_toolkit.common.redaction import redact_sensitive -from ocr_toolkit.ocr_result import ( +from ocr_toolkit.reporting.result import ( + CoverageDiagnostic, + CoverageDiagnostics, OcrResultMalformed, OcrResultMissing, OcrResultTooLarge, + llm_billing_failure_reason, load_ocr_result, + normalize_coverage_diagnostics, + ocr_warning_text, ) -from ocr_toolkit.posting.comments import clean_text, compact_escaped_text -from ocr_toolkit.provider_failure import ProviderFailureReason -from ocr_toolkit.result_contract import ReviewOutcome __all__ = [ + "CoverageDiagnostic", + "CoverageDiagnostics", "OcrResultMalformed", "OcrResultMissing", "OcrResultTooLarge", @@ -29,187 +23,3 @@ "normalize_coverage_diagnostics", "ocr_warning_text", ] - -LLM_BILLING_FAILURE_RE = re.compile( - r"(?i)\b(" - r"(?:http\s*)?status(?:[_\s]*code)?[\"']?\s*[:=]\s*[\"']?402|" - r"code[\"']?\s*[:=]\s*[\"']?402|payment required|insufficient[_ -]?funds|insufficient user balance|" - r"insufficient balance|insufficient[_ -]?quota|quota[_ -]?exceeded|" - r"out of credits|credit balance" - r")\b" -) - -MAX_COVERAGE_DIAGNOSTICS = 10 -MAX_COVERAGE_DETAIL_CHARS = 240 -FAILURE_REASON_LABELS = { - "timeout": "review timed out", - "provider": "provider request failed", - "cancelled": "review cancelled", - "configuration": "configuration failed", - "input": "input could not be reviewed", - "budget": "token limit reached", - "panic": "unknown subtask failure", - "unknown": "unknown subtask failure", -} - - -@dataclass(frozen=True, slots=True) -class CoverageDiagnostic: - """Hold one safe actionable failed-file receipt.""" - - path: str - reason: str - detail: str = "" - - -@dataclass(frozen=True, slots=True) -class CoverageDiagnostics: - """Hold bounded diagnostics and explicit information loss counters.""" - - records: tuple[CoverageDiagnostic, ...] - omitted: int - invalid: int - failed_total: int - unique_file_count: int - - @property - def file_count(self) -> int | None: - """Return a safe unique-file count only when every failure has a path.""" - - return self.unique_file_count if self.unique_file_count > 0 and self.invalid == 0 else None - - -def _safe_repository_path(value: object) -> str: - """Return one normalized repository-relative path or an empty value.""" - - if not isinstance(value, str) or not value or len(value) > 1_024 or "\\" in value: - return "" - parts = value.split("/") - path = PurePosixPath(value) - if path.is_absolute() or any(part in {"", ".", ".."} for part in parts): - return "" - if any(character == "\x7f" or ord(character) < 32 for character in value): - return "" - return path.as_posix() - - -def _legacy_reason(warning: Any) -> str: - """Map legacy warning shapes into a closed reviewer-facing vocabulary.""" - - warning_type = clean_text(warning.get("type")) if isinstance(warning, dict) else "" - text = f"{warning_type}\n{ocr_warning_text(warning)}".casefold() - if "timeout" in text or "timed out" in text: - return "review timed out" - if "budget" in text or "token limit" in text: - return "token limit reached" - if "provider" in text or "request" in text: - return "provider request failed" - if "tool" in text and "loop" in text: - return "tool loop failed" - if "parse" in text or "invalid result" in text: - return "result could not be parsed" - return "unknown subtask failure" - - -def _safe_detail(value: object, reason: str) -> str: - """Return optional redacted Markdown-neutral detail within a small budget.""" - - text = compact_escaped_text( - neutralize_quick_actions(redact_sensitive(clean_text(value))), - MAX_COVERAGE_DETAIL_CHARS, - ) - return "" if not text or text.casefold() == reason.casefold() else text - - -def normalize_coverage_diagnostics( - outcome: ReviewOutcome, - warnings: Sequence[Any], - *, - legacy_warning_fallback: bool = True, -) -> CoverageDiagnostics: - """Normalize manifest failures or legacy warnings once at the posting boundary.""" - - candidates: list[tuple[object, str, object]] = [] - if outcome.manifest_present: - candidates.extend( - ( - item.path, - FAILURE_REASON_LABELS.get(item.classification, "unknown subtask failure"), - item.reason, - ) - for item in outcome.failed_items - ) - elif outcome.kind == "partial" and legacy_warning_fallback: - for warning in warnings: - path = warning.get("file") or warning.get("path") if isinstance(warning, dict) else None - candidates.append((path, _legacy_reason(warning), ocr_warning_text(warning))) - - records: list[CoverageDiagnostic] = [] - seen: set[tuple[str, str]] = set() - invalid = 0 - for raw_path, reason, raw_detail in candidates: - path = _safe_repository_path(raw_path) - if not path: - invalid += 1 - continue - key = (path, reason) - if key in seen: - continue - seen.add(key) - records.append(CoverageDiagnostic(path, reason, _safe_detail(raw_detail, reason))) - records.sort(key=lambda item: (item.path, item.reason, item.detail)) - omitted = max(0, len(records) - MAX_COVERAGE_DIAGNOSTICS) - unique_file_count = len({record.path for record in records}) - return CoverageDiagnostics( - records=tuple(records[:MAX_COVERAGE_DIAGNOSTICS]), - omitted=omitted, - invalid=invalid, - failed_total=len(seen) + invalid, - unique_file_count=unique_file_count, - ) - - -def ocr_warning_text(warning: Any, *, _seen: set[int] | None = None) -> str: - """Return warning text relevant for provider failure classification.""" - - if _seen is None: - _seen = set() - if isinstance(warning, (dict, list)): - marker = id(warning) - if marker in _seen: - return "" - _seen.add(marker) - if isinstance(warning, dict): - parts: list[str] = [] - for key in ("type", "message", "code", "status", "status_code", "detail"): - text = clean_text(warning.get(key)) - if text: - parts.append(f"{key}: {text}" if key in {"code", "status", "status_code"} else text) - for key in ("error", "details"): - nested = warning.get(key) - if isinstance(nested, dict): - text = ocr_warning_text(nested, _seen=_seen) - if text: - parts.append(text) - else: - text = clean_text(nested) - if text: - parts.append(text) - return "\n".join(parts)[:4000] - if isinstance(warning, list): - return "\n".join( - text for value in warning[:40] if (text := ocr_warning_text(value, _seen=_seen)) - )[:4000] - return clean_text(warning) - - -def llm_billing_failure_reason( - warnings: Sequence[Any], -) -> ProviderFailureReason | None: - """Map a legacy OCR billing warning to the shared safe provider reason.""" - - for warning in warnings: - text = ocr_warning_text(warning) - if text and LLM_BILLING_FAILURE_RE.search(text): - return ProviderFailureReason.RATE_OR_SPENDING_LIMIT - return None diff --git a/src/ocr_toolkit/preflight.py b/src/ocr_toolkit/preflight.py index 5d732b8..d163fb1 100644 --- a/src/ocr_toolkit/preflight.py +++ b/src/ocr_toolkit/preflight.py @@ -20,6 +20,7 @@ ProviderConfigError, provider_config_from_environment, ) +from ocr_toolkit.providers.local_config import validate_local_context HTTP_TIMEOUT_SECONDS = 30 MAX_RESPONSE_BODY_BYTES = 2_000_000 @@ -28,7 +29,7 @@ "Accept": "application/json", "User-Agent": "open-code-review-ci-preflight/1.0", } -EXPECTED_OCR_VERSION = "1.11.5" +EXPECTED_OCR_VERSION = "1.11.6" class PreflightError(Exception): @@ -331,12 +332,18 @@ def validate_llm_model() -> None: print(f"LLM model validated: {model_id} context_length={context_length}") -def main() -> int: +def main(*, local: bool = False) -> int: """Run all fail-fast checks.""" try: + if local: + try: + validate_local_context(os.environ) + except ValueError as exc: + raise PreflightError(str(exc)) from exc validate_ocr_binary() - validate_gitlab_access() + if not local: + validate_gitlab_access() validate_llm_model() except PreflightError as exc: print(f"OCR preflight failed: {redact_sensitive(str(exc))}", file=sys.stderr) diff --git a/src/ocr_toolkit/provider_config.py b/src/ocr_toolkit/provider_config.py index 0b3eaa6..133491b 100644 --- a/src/ocr_toolkit/provider_config.py +++ b/src/ocr_toolkit/provider_config.py @@ -13,6 +13,7 @@ HEADER_NAME_RE = re.compile(r"^[!#$%&'*+.^_`|~0-9A-Za-z-]+$") POSITIVE_DECIMAL_RE = re.compile(r"^[1-9][0-9]*$") LLM_PROTOCOLS = frozenset({"anthropic", "openai", "openai-responses"}) +REASONING_EFFORTS = frozenset({"none", "minimal", "low", "medium", "high", "xhigh", "max"}) MAX_COMPLETION_TOKENS_LIMIT = 1_000_000 COMPLETION_TOKEN_FIELDS = { "anthropic": "max_tokens", @@ -230,6 +231,35 @@ def _canonical_provider_urls( return inference, api_root, explicit_models or derived_models +def _apply_reasoning_effort(extra_body: dict[str, Any], *, protocol: str, value: str) -> None: + """Overlay one explicit reasoning value without replacing operator siblings.""" + + effort = value.lower() + if not effort: + return + if effort not in REASONING_EFFORTS: + raise ProviderConfigError( + "OCR_LLM_REASONING_EFFORT must be none, minimal, low, medium, high, xhigh, or max" + ) + if protocol == "anthropic": + raise ProviderConfigError("OCR_LLM_REASONING_EFFORT is not supported with anthropic") + target = extra_body + key = "reasoning_effort" + if protocol == "openai-responses": + if "reasoning" in extra_body and not isinstance(extra_body["reasoning"], dict): + raise ProviderConfigError( + "OCR_LLM_EXTRA_BODY.reasoning must be an object when setting effort" + ) + target = extra_body.setdefault("reasoning", {}) + key = "effort" + if key in target and target[key] != effort: + raise ProviderConfigError( + "OCR_LLM_REASONING_EFFORT conflicts with OCR_LLM_EXTRA_BODY; " + "remove one setting or make the effort values equal" + ) + target[key] = effort + + def request_controls_from_environment( environment: Mapping[str, str] | None = None, ) -> ProviderRequestControls: @@ -246,6 +276,9 @@ def request_controls_from_environment( raise ProviderConfigError("OCR_LLM_EXTRA_HEADERS must not duplicate OCR_LLM_AUTH_HEADER") extra_body, explicit_body = _parse_extra_body(_env(values, "OCR_LLM_EXTRA_BODY")) + _apply_reasoning_effort( + extra_body, protocol=protocol, value=_env(values, "OCR_LLM_REASONING_EFFORT") + ) completion_cap = _parse_completion_cap(_env(values, "OCR_LLM_MAX_COMPLETION_TOKENS")) if completion_cap is not None: field = COMPLETION_TOKEN_FIELDS[protocol] diff --git a/src/ocr_toolkit/providers/local.py b/src/ocr_toolkit/providers/local.py new file mode 100644 index 0000000..c596846 --- /dev/null +++ b/src/ocr_toolkit/providers/local.py @@ -0,0 +1,164 @@ +"""Standalone local Markdown output; no forge acquisition, actions or posting policy.""" + +from __future__ import annotations + +import os +import secrets +from collections.abc import Iterator +from pathlib import Path +from typing import TextIO + +from ocr_toolkit.common.filesystem import ( + fsync_directory, + open_private_parent_directory, + same_file_identity, +) +from ocr_toolkit.common.markdown import inline_code, markdown_code_block +from ocr_toolkit.reporting.dlp import admission_dlp_state, format_dlp_admission +from ocr_toolkit.reporting.metadata import finding_metadata, format_ocr_core_advisory +from ocr_toolkit.reporting.model import ReviewReport +from ocr_toolkit.reporting.outcome import FindingVisibility, review_outcome_line +from ocr_toolkit.reporting.sections import report_sections +from ocr_toolkit.reporting.text import clean_text, code_text, comment_line + + +def local_summary(report: ReviewReport) -> str: + """Render common facts, with neither forge publication claims nor remote badges.""" + + lines = [ + "## Open Code Review", + "", + review_outcome_line( + findings=FindingVisibility(count=len(report.comments)), + warning_count=len(report.warnings), + outcome_status=( + "admission-filtered" + if admission_dlp_state(report.publication) == "publication-filtered" + and report.outcome.kind in {"clean", "warning"} + else report.outcome.status + ), + outcome_message=report.outcome_message, + unreviewed_file_count=report.diagnostics.file_count, + ), + ] + if report.failure_stage is not None: + lines.extend( + [ + "", + f"Stopped at: {inline_code(report.failure_stage, escape_controls=True)}.", + "Findings, coverage and execution usage are unavailable or untrusted. " + "See the diagnostic output for the blocking check.", + ] + ) + lines.extend(report_sections(report.comments, report.diagnostics, report.warnings)) + lines.extend(["", "### Technical details", "", "- Output: local Markdown"]) + if report.reviewed_sha: + lines.append(f"- Reviewed commit: {inline_code(report.reviewed_sha, escape_controls=True)}") + if report.outcome.coverage_summary: + lines.append(f"- {report.outcome.coverage_summary}") + for detail in ( + report.mcp_usage_summary, + report.tool_calls_summary, + report.token_usage_summary, + format_ocr_core_advisory(report.advisory), + format_dlp_admission(report.publication), + ): + if detail: + lines.append(detail) + return "\n".join(lines) + + +def local_report_parts(report: ReviewReport) -> Iterator[str]: + """Yield every admitted finding without posting limits or executable markup.""" + + yield local_summary(report) + for ordinal, comment in enumerate(report.comments, 1): + location = clean_text(comment.get("path")) or "unknown" + line = comment_line(comment) + if line: + location += f":L{line}" + tags = ", ".join(part for part in finding_metadata(comment) if part) + header = f"### Finding {ordinal}: {inline_code(location, escape_controls=True)}" + if tags: + header += f" — {inline_code(tags)}" + yield ( + "\n\n" + + header + + "\n\n" + + markdown_code_block( + "", clean_text(comment.get("content")) or "Open Code Review reported an issue here." + ) + ) + for field, label in ( + ("existing_code", "Existing code"), + ("suggestion_code", "Suggested code"), + ): + content = code_text(comment.get(field)) + if content: + yield f"\n\n#### {label}\n\n" + markdown_code_block("", content) + yield "\n" + + +def write_local_report(report: ReviewReport, stream: TextIO) -> None: + """Write admitted output incrementally; the runner owns output failure handling.""" + + for part in local_report_parts(report): + stream.write(part) + stream.flush() + + +def prepare_local_report_path(path: Path, *, other_outputs: tuple[Path, ...]) -> None: + """Validate a fresh report destination without overwriting caller-owned data.""" + + if path.resolve() in {output.resolve() for output in other_outputs}: + raise ValueError("local report path must differ from result and stderr") + if path.is_symlink() or path.exists(): + raise ValueError("local report path must be fresh") + + +def publish_local_report(report: ReviewReport, path: Path) -> None: + """Publish complete private Markdown atomically without replacing another file.""" + + prepare_local_report_path(path, other_outputs=()) + parent_descriptor, destination = open_private_parent_directory(path) + temporary = ".ocr-report-" + secrets.token_hex(12) + descriptor = -1 + temporary_identity: os.stat_result | None = None + try: + descriptor = os.open( + temporary, + os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, + 0o600, + dir_fd=parent_descriptor, + ) + os.fchmod(descriptor, 0o600) + temporary_identity = os.fstat(descriptor) + with os.fdopen(descriptor, "w", encoding="utf-8", newline="\n") as stream: + descriptor = -1 + write_local_report(report, stream) + os.fsync(stream.fileno()) + # Unlike replace(), link() rejects a destination created since preflight. + current = os.stat(temporary, dir_fd=parent_descriptor, follow_symlinks=False) + if not same_file_identity(temporary_identity, current) or current.st_nlink != 1: + raise OSError("private report temporary artifact changed") + os.link( + temporary, + destination, + src_dir_fd=parent_descriptor, + dst_dir_fd=parent_descriptor, + follow_symlinks=False, + ) + os.unlink(temporary, dir_fd=parent_descriptor) + temporary_identity = None + fsync_directory(parent_descriptor) + finally: + if descriptor >= 0: + os.close(descriptor) + if temporary_identity is not None: + try: + current = os.stat(temporary, dir_fd=parent_descriptor, follow_symlinks=False) + if same_file_identity(temporary_identity, current): + os.unlink(temporary, dir_fd=parent_descriptor) + except FileNotFoundError: + pass + os.close(parent_descriptor) diff --git a/src/ocr_toolkit/providers/local_config.py b/src/ocr_toolkit/providers/local_config.py new file mode 100644 index 0000000..358959b --- /dev/null +++ b/src/ocr_toolkit/providers/local_config.py @@ -0,0 +1,14 @@ +"""Validate the input capabilities supported by standalone review.""" + +from collections.abc import Mapping + +from ocr_toolkit.evidence.review_context import parse_review_context_mode + + +def validate_local_context(environment: Mapping[str, str]) -> None: + """Reject explicit unsupported channels before acquiring inputs or credentials.""" + + if parse_review_context_mode(environment.get("OCR_REVIEW_CONTEXT_MODE")) != "off": + raise ValueError("local review does not support change-request context") + if environment.get("OCR_REVIEW_CONTEXT_ADAPTERS_JSON") is not None: + raise ValueError("local review does not support context adapters") diff --git a/src/ocr_toolkit/reporting/__init__.py b/src/ocr_toolkit/reporting/__init__.py new file mode 100644 index 0000000..bd75484 --- /dev/null +++ b/src/ocr_toolkit/reporting/__init__.py @@ -0,0 +1 @@ +"""Provider-neutral review data and pure presentation; no provider I/O or actions.""" diff --git a/src/ocr_toolkit/reporting/dlp.py b/src/ocr_toolkit/reporting/dlp.py new file mode 100644 index 0000000..c7efe08 --- /dev/null +++ b/src/ocr_toolkit/reporting/dlp.py @@ -0,0 +1,197 @@ +"""Provider-neutral validation of DLP admission and original coverage facts.""" + +from __future__ import annotations + +from typing import Any + +from ocr_toolkit.ocr_result import MAX_TOOLKIT_MCP_USAGE_COUNT +from ocr_toolkit.result_contract import OcrResultContractError, ReviewOutcome + + +def format_dlp_admission(publication: Any) -> str: + """Render only closed DLP facts, without implying a provider mutation.""" + + state = admission_dlp_state(publication) + if state is None: + return "- DLP admission: unavailable" + if state == "passed": + return "- DLP admission: passed" + reasons = ", ".join( + f"{reason}: {count}" + for reason, count in sorted(publication["reason_counts"].items()) + if count > 0 + ) + if state == "private-sanitized": + return ( + f"- DLP admission: private fields sanitized ({publication['sanitized_fields']}); " + f"admitted findings unchanged; reasons: {reasons}" + ) + retained, omitted = publication["retained"], publication["omitted"] + return ( + f"- DLP admission: filtered; retained {retained['comments']} finding(s), " + f"{retained['warnings']} warning(s); omitted {omitted['comments']} finding(s), " + f"{omitted['warnings']} warning(s), {omitted['fields']} field(s); reasons: {reasons}" + ) + + +def publication_dlp_state(value: Any) -> str | None: + """Validate the exact current publication-policy receipt.""" + + return _dlp_state(value, allow_unknown_coverage=False) + + +def admission_dlp_state(value: Any) -> str | None: + """Validate execution-owner admission facts, including unknown legacy coverage. + + This does not validate or authorize a platform publication receipt. + """ + + return _dlp_state(value, allow_unknown_coverage=True) + + +def _dlp_state(value: Any, *, allow_unknown_coverage: bool) -> str | None: + """Validate closed DLP facts with an explicit coverage-knowledge boundary.""" + + if value == {"state": "passed"}: + return "passed" + if not isinstance(value, dict): + return None + if value.get("state") == "private-sanitized": + if set(value) != {"state", "reason_counts", "sanitized_fields"}: + return None + reason_counts = value.get("reason_counts") + sanitized_fields = value.get("sanitized_fields") + if ( + not _valid_dlp_reason_counts(reason_counts) + or not any(reason_counts.values()) + or not isinstance(sanitized_fields, int) + or isinstance(sanitized_fields, bool) + or not 0 < sanitized_fields <= MAX_TOOLKIT_MCP_USAGE_COUNT + ): + return None + return "private-sanitized" + if not isinstance(value, dict) or set(value) != { + "state", + "reason_counts", + "retained", + "omitted", + "original", + }: + return None + if value.get("state") != "publication-filtered": + return None + reason_counts = value.get("reason_counts") + retained = value.get("retained") + omitted = value.get("omitted") + original = value.get("original") + if ( + not _valid_dlp_reason_counts(reason_counts) + or not any(reason_counts.values()) + or not isinstance(retained, dict) + or set(retained) != {"comments", "warnings"} + or not isinstance(omitted, dict) + or set(omitted) != {"comments", "warnings", "fields"} + or any( + not isinstance(count, int) + or isinstance(count, bool) + or not 0 <= count <= MAX_TOOLKIT_MCP_USAGE_COUNT + for counts in (retained, omitted) + for count in counts.values() + ) + or not isinstance(original, dict) + or set(original) != {"outcome", "selected", "completed", "reused", "failed", "waived"} + or original.get("outcome") not in {"clean", "warning", "partial", "failed", "skipped"} + or any( + not isinstance(original.get(field), int) + or isinstance(original.get(field), bool) + or not 0 <= original[field] <= MAX_TOOLKIT_MCP_USAGE_COUNT + for field in ("selected", "completed", "reused", "failed", "waived") + ) + ): + return None + selected = original["selected"] + completed = original["completed"] + reused = original["reused"] + failed = original["failed"] + waived = original["waived"] + outcome = original["outcome"] + derived_outcomes = {"failed"} | ( + {"skipped"} + if selected == 0 + else {"clean", "warning"} + if failed == 0 + else {"failed"} + if failed == selected + else {"partial"} + ) + if allow_unknown_coverage and selected == completed == reused == failed == waived == 0: + derived_outcomes |= {"clean", "warning", "partial"} + if selected != completed + reused + failed + waived or outcome not in derived_outcomes: + return None + return "publication-filtered" + + +def _valid_dlp_reason_counts(value: Any) -> bool: + return bool( + isinstance(value, dict) + and set(value) == {"forbidden", "invalid_text", "laundering", "limit", "pii", "secret"} + and all( + isinstance(count, int) + and not isinstance(count, bool) + and 0 <= count <= MAX_TOOLKIT_MCP_USAGE_COUNT + for count in value.values() + ) + ) + + +def publication_outcome_for_summary(outcome: ReviewOutcome, publication: Any) -> ReviewOutcome: + """Recover only validated original coverage facts from a filtered receipt.""" + + if publication_dlp_state(publication) != "publication-filtered": + return outcome + return _original_outcome(outcome, publication) + + +def admitted_outcome_for_summary(outcome: ReviewOutcome, publication: Any) -> ReviewOutcome: + """Recover owner-observed outcome without inventing legacy coverage counts.""" + + if admission_dlp_state(publication) != "publication-filtered": + return outcome + return _original_outcome(outcome, publication) + + +def _original_outcome(outcome: ReviewOutcome, publication: Any) -> ReviewOutcome: + """Recover original outcome only after the caller validates its DLP facts.""" + + if outcome.kind != "partial" or outcome.manifest_present: + raise OcrResultContractError( + "publication-filtered receipt is not bound to a safe result projection" + ) + original = publication["original"] + kind = original["outcome"] + if outcome.budget_exceeded and kind != "partial": + raise OcrResultContractError( + "publication-filtered receipt contradicts the result budget state" + ) + counts = { + field: original[field] for field in ("selected", "completed", "reused", "failed", "waived") + } + manifest_present = any(counts.values()) + status = { + "clean": "complete" if manifest_present else "success", + "warning": "completed_with_warnings", + "partial": "budget_exceeded" if outcome.budget_exceeded else "completed_with_errors", + "failed": "failed", + "skipped": "skipped", + }[kind] + return ReviewOutcome( + status=status, + kind=kind, + budget_exceeded=outcome.budget_exceeded and kind == "partial", + manifest_present=manifest_present, + selected_count=counts["selected"], + completed_count=counts["completed"], + reused_count=counts["reused"], + failed_count=counts["failed"], + waived_count=counts["waived"], + ) diff --git a/src/ocr_toolkit/reporting/metadata.py b/src/ocr_toolkit/reporting/metadata.py new file mode 100644 index 0000000..f9436cc --- /dev/null +++ b/src/ocr_toolkit/reporting/metadata.py @@ -0,0 +1,90 @@ +"""Closed finding metadata and token presentation shared by all adapters.""" + +from __future__ import annotations + +from typing import Any + +from ocr_toolkit.ocr_result import OcrToolkitAdvisory +from ocr_toolkit.reporting.text import clean_text +from ocr_toolkit.result_usage import normalize_token_usage + +OCR_FINDING_SEVERITY_ORDER = ("critical", "high", "medium", "low") +OCR_FINDING_CATEGORY_ORDER = ( + "security", + "bug", + "performance", + "maintainability", + "test", + "documentation", + "style", + "other", +) +OCR_FINDING_SEVERITIES = set(OCR_FINDING_SEVERITY_ORDER) +OCR_FINDING_CATEGORIES = set(OCR_FINDING_CATEGORY_ORDER) + + +def normalized_ocr_metadata(value: Any, allowed_values: set[str]) -> str: + """Return a whitelisted OCR metadata value suitable for display.""" + + text = clean_text(value).casefold() + return text if text in allowed_values else "" + + +def finding_metadata(comment: dict[str, Any]) -> tuple[str, str]: + """Return structured OCR category/severity metadata from a finding.""" + + severity = normalized_ocr_metadata( + comment.get("severity"), OCR_FINDING_SEVERITIES + ) or normalized_ocr_metadata(comment.get("priority"), OCR_FINDING_SEVERITIES) + category = normalized_ocr_metadata(comment.get("category"), OCR_FINDING_CATEGORIES) + return severity, category + + +def format_token_usage_summary(result: dict[str, Any]) -> str: + """Return one bounded summary line for structured OCR token usage.""" + + usage = normalize_token_usage(result) + if usage is None: + return "" + + total = usage.get("total") + details: list[str] = [] + for bucket in ("input", "output", "cached", "reasoning", "other"): + if (count := usage.get(bucket)) is not None and count > 0: + details.append(f"{bucket}: {count}") + + if total is None: + return f"- token usage: {', '.join(details)}" if details else "" + line = f"- token usage: {total} total" + if details: + line += f" ({', '.join(details)})" + return line + + +def format_ocr_core_advisory(advisory: OcrToolkitAdvisory | None) -> str: + """Render one validated numeric OCR advisory for Technical details only.""" + + if advisory is None: + return "" + return ( + f"- OCR core advisory: background {advisory.actual} characters; recommended " + f"{advisory.recommended} characters; accepted by OCR core" + ) + + +SEVERITY_EMOJI = { + "critical": "❌", + "high": "🚨", + "medium": "⚠️", + "low": "ℹ️", # noqa: RUF001 - intentional information emoji +} +CATEGORY_EMOJI = { + "bug": "🐛", + "security": "🔒", + "performance": "⚡", + "maintainability": "🛠️", + "test": "🧪", + "style": "🎨", + "documentation": "📚", + "other": "📌", +} diff --git a/src/ocr_toolkit/reporting/model.py b/src/ocr_toolkit/reporting/model.py new file mode 100644 index 0000000..cb06140 --- /dev/null +++ b/src/ocr_toolkit/reporting/model.py @@ -0,0 +1,127 @@ +"""Compose admitted review data independently of provider acquisition and delivery.""" + +from __future__ import annotations + +import re +from copy import deepcopy +from dataclasses import dataclass +from typing import Any, Literal, get_args + +from ocr_toolkit.ocr_result import OcrToolkitAdvisory +from ocr_toolkit.reporting.dlp import admission_dlp_state, admitted_outcome_for_summary +from ocr_toolkit.reporting.metadata import format_token_usage_summary +from ocr_toolkit.reporting.result import CoverageDiagnostics, normalize_coverage_diagnostics +from ocr_toolkit.reporting.usage import ( + format_tool_calls_summary, + format_verified_mcp_usage, + validated_completed_actions, +) +from ocr_toolkit.result_contract import OcrResultContractError, ReviewOutcome, parse_result_outcome + +FailureStage = Literal[ + "configuration", + "identity", + "evidence", + "mcp-preflight", + "preview", + "subprocess", + "result-validation", + "mcp-use", + "dlp", + "cleanup", + "reporting", +] + + +@dataclass(frozen=True, slots=True) +class ExecutionFacts: + """Carry execution-owner facts separately from model output or forge receipts. + + Only the runner's successful evidence reconciliation and DLP finalization may + supply these values. Rendering does not confer authority on input claims. + """ + + mcp_usage: dict[str, int] + evidence: dict[str, Any] + publication: dict[str, Any] + advisory: OcrToolkitAdvisory | None = None + + +@dataclass(frozen=True, slots=True) +class ReviewReport: + """Snapshot common review facts before an adapter applies delivery policy.""" + + outcome: ReviewOutcome + comments: tuple[dict[str, Any], ...] + warnings: tuple[Any, ...] + diagnostics: CoverageDiagnostics + reviewed_sha: str = "" + outcome_message: str = "" + tool_calls_summary: str = "" + token_usage_summary: str = "" + mcp_usage_summary: str = "" + publication: dict[str, Any] | None = None + advisory: OcrToolkitAdvisory | None = None + failure_stage: FailureStage | None = None + + +def report_from_result( + result: dict[str, Any], *, execution: ExecutionFacts, reviewed_sha: str = "" +) -> ReviewReport: + """Snapshot an already admitted result and separately verified execution facts.""" + + if reviewed_sha and re.fullmatch(r"[0-9a-f]{40}", reviewed_sha) is None: + raise OcrResultContractError("report reviewed commit is not an immutable SHA") + comments = result.get("comments", []) + warnings = result.get("warnings", []) + if not isinstance(comments, list) or any(not isinstance(item, dict) for item in comments): + raise OcrResultContractError("report comments must be a list of objects") + if not isinstance(warnings, list): + raise OcrResultContractError("report warnings must be a list") + state = admission_dlp_state(execution.publication) + if state is None: + raise OcrResultContractError("report DLP admission facts are unavailable") + outcome = admitted_outcome_for_summary(parse_result_outcome(result), execution.publication) + mcp_summary = format_verified_mcp_usage( + mcp_usage=execution.mcp_usage, evidence=execution.evidence + ) + completed = validated_completed_actions(execution.evidence) + if outcome.requires_evidence_mcp and ( + not mcp_summary + or completed is None + or completed["summary"] < 1 + or execution.evidence.get("mandatory") is not True + ): + raise OcrResultContractError("report verified evidence usage is unavailable") + message = result.get("message") + return ReviewReport( + outcome=outcome, + comments=tuple(deepcopy(comments)), + warnings=tuple(deepcopy(warnings)), + diagnostics=normalize_coverage_diagnostics( + outcome, warnings, legacy_warning_fallback=state != "publication-filtered" + ), + reviewed_sha=reviewed_sha, + outcome_message=message if isinstance(message, str) else "", + tool_calls_summary=format_tool_calls_summary(result.get("tool_calls")), + token_usage_summary=format_token_usage_summary(result), + mcp_usage_summary=mcp_summary, + publication=deepcopy(execution.publication), + advisory=execution.advisory, + ) + + +def failed_report(stage: FailureStage, *, reviewed_sha: str = "") -> ReviewReport: + """Represent unavailable review data without rendering raw exceptions or output.""" + + if stage not in get_args(FailureStage): + raise OcrResultContractError("report failure stage is unsupported") + safe_sha = reviewed_sha if re.fullmatch(r"[0-9a-f]{40}", reviewed_sha) else "" + return ReviewReport( + outcome=ReviewOutcome(status="failed", kind="failed", budget_exceeded=False), + comments=(), + warnings=(), + diagnostics=CoverageDiagnostics((), 0, 0, 0, 0), + reviewed_sha=safe_sha, + failure_stage=stage, + ) diff --git a/src/ocr_toolkit/reporting/outcome.py b/src/ocr_toolkit/reporting/outcome.py new file mode 100644 index 0000000..8c87f52 --- /dev/null +++ b/src/ocr_toolkit/reporting/outcome.py @@ -0,0 +1,86 @@ +"""Pure review health and finding-visibility presentation for output adapters.""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True, slots=True) +class FindingVisibility: + """Describe adapter-owned delivery without changing review health.""" + + count: int + published: bool = False + omitted: int = 0 + suppressed: int = 0 + + +def review_outcome_line( + *, + findings: FindingVisibility, + warning_count: int, + outcome_status: str, + outcome_message: str, + unreviewed_file_count: int | None = None, + emoji: bool = False, +) -> str: + """Render one shared decision line; publication facts belong to the adapter.""" + + budget_stop = outcome_status == "budget_exceeded" or ( + outcome_status == "partial" and "budget" in outcome_message.casefold() + ) + partial_result = outcome_status in {"partial", "completed_with_errors", "budget_exceeded"} + has_finding_state = findings.count > 0 or findings.omitted > 0 or findings.suppressed > 0 + if outcome_status == "skipped": + marker, status_text = "ℹ️", "Review skipped" # noqa: RUF001 + result_text = "no supported files changed" + elif outcome_status == "failed": + marker, status_text = "❌", "Review failed" + result_text = "no reliable review result was produced" + else: + if budget_stop: + marker, status_text = "⚠️", "Review stopped at token budget" + elif partial_result: + marker, status_text = "⚠️", "Review incomplete" + elif outcome_status == "publication-filtered": + marker, status_text = "⚠️", "Review complete with publication filtering" + elif outcome_status == "admission-filtered": + marker, status_text = "⚠️", "Review complete with DLP filtering" + elif outcome_status in {"warning", "completed_with_warnings"} or warning_count: + marker, status_text = "⚠️", "Review complete with warnings" + elif has_finding_state: + marker, status_text = "🔎", "Review complete" + else: + marker, status_text = "✅", "Review complete" + + delivered = " published" if findings.published else "" + if findings.count: + noun = "finding" if findings.count == 1 else "findings" + result_text = f"{findings.count} {noun}{delivered}" + if partial_result: + result_text += " from reviewed files" + elif findings.omitted: + result_text = f"no findings{delivered}" + if partial_result: + result_text += " from reviewed files" + elif findings.suppressed: + result_text = f"no new findings{delivered}" + if partial_result: + result_text += " from reviewed files" + elif partial_result: + result_text = "no findings in reviewed files" + else: + result_text = "no findings" + + if findings.omitted: + noun = "finding" if findings.omitted == 1 else "findings" + result_text += f"; {findings.omitted} {noun} omitted by posting limit" + if findings.suppressed: + noun = "finding" if findings.suppressed == 1 else "findings" + result_text += f"; {findings.suppressed} {noun} matched prior reviewer decisions" + if partial_result and unreviewed_file_count is not None: + noun = "file" if unreviewed_file_count == 1 else "files" + result_text += f"; {unreviewed_file_count} {noun} not reviewed" + + prefix = f"{marker} " if emoji else "" + return f"{prefix}**{status_text} — {result_text}**" diff --git a/src/ocr_toolkit/reporting/result.py b/src/ocr_toolkit/reporting/result.py new file mode 100644 index 0000000..2185961 --- /dev/null +++ b/src/ocr_toolkit/reporting/result.py @@ -0,0 +1,215 @@ +"""OCR result artifact loading and provider failure classification.""" + +from __future__ import annotations + +import re +from collections.abc import Sequence +from dataclasses import dataclass +from pathlib import PurePosixPath +from typing import Any + +from ocr_toolkit.common.markdown import neutralize_quick_actions +from ocr_toolkit.common.redaction import redact_sensitive +from ocr_toolkit.ocr_result import ( + OcrResultMalformed, + OcrResultMissing, + OcrResultTooLarge, + load_ocr_result, +) +from ocr_toolkit.provider_failure import ProviderFailureReason +from ocr_toolkit.reporting.text import clean_text, compact_escaped_text +from ocr_toolkit.result_contract import ReviewOutcome + +__all__ = [ + "OcrResultMalformed", + "OcrResultMissing", + "OcrResultTooLarge", + "llm_billing_failure_reason", + "load_ocr_result", + "normalize_coverage_diagnostics", + "ocr_warning_text", +] + +LLM_BILLING_FAILURE_RE = re.compile( + r"(?i)\b(" + r"(?:http\s*)?status(?:[_\s]*code)?[\"']?\s*[:=]\s*[\"']?402|" + r"code[\"']?\s*[:=]\s*[\"']?402|payment required|insufficient[_ -]?funds|insufficient user balance|" + r"insufficient balance|insufficient[_ -]?quota|quota[_ -]?exceeded|" + r"out of credits|credit balance" + r")\b" +) + +MAX_COVERAGE_DIAGNOSTICS = 10 +MAX_COVERAGE_DETAIL_CHARS = 240 +FAILURE_REASON_LABELS = { + "timeout": "review timed out", + "provider": "provider request failed", + "cancelled": "review cancelled", + "configuration": "configuration failed", + "input": "input could not be reviewed", + "budget": "token limit reached", + "panic": "unknown subtask failure", + "unknown": "unknown subtask failure", +} + + +@dataclass(frozen=True, slots=True) +class CoverageDiagnostic: + """Hold one safe actionable failed-file receipt.""" + + path: str + reason: str + detail: str = "" + + +@dataclass(frozen=True, slots=True) +class CoverageDiagnostics: + """Hold bounded diagnostics and explicit information loss counters.""" + + records: tuple[CoverageDiagnostic, ...] + omitted: int + invalid: int + failed_total: int + unique_file_count: int + + @property + def file_count(self) -> int | None: + """Return a safe unique-file count only when every failure has a path.""" + + return self.unique_file_count if self.unique_file_count > 0 and self.invalid == 0 else None + + +def _safe_repository_path(value: object) -> str: + """Return one normalized repository-relative path or an empty value.""" + + if not isinstance(value, str) or not value or len(value) > 1_024 or "\\" in value: + return "" + parts = value.split("/") + path = PurePosixPath(value) + if path.is_absolute() or any(part in {"", ".", ".."} for part in parts): + return "" + if any(character == "\x7f" or ord(character) < 32 for character in value): + return "" + return path.as_posix() + + +def _legacy_reason(warning: Any) -> str: + """Map legacy warning shapes into a closed reviewer-facing vocabulary.""" + + warning_type = clean_text(warning.get("type")) if isinstance(warning, dict) else "" + text = f"{warning_type}\n{ocr_warning_text(warning)}".casefold() + if "timeout" in text or "timed out" in text: + return "review timed out" + if "budget" in text or "token limit" in text: + return "token limit reached" + if "provider" in text or "request" in text: + return "provider request failed" + if "tool" in text and "loop" in text: + return "tool loop failed" + if "parse" in text or "invalid result" in text: + return "result could not be parsed" + return "unknown subtask failure" + + +def _safe_detail(value: object, reason: str) -> str: + """Return optional redacted Markdown-neutral detail within a small budget.""" + + text = compact_escaped_text( + neutralize_quick_actions(redact_sensitive(clean_text(value))), + MAX_COVERAGE_DETAIL_CHARS, + ) + return "" if not text or text.casefold() == reason.casefold() else text + + +def normalize_coverage_diagnostics( + outcome: ReviewOutcome, + warnings: Sequence[Any], + *, + legacy_warning_fallback: bool = True, +) -> CoverageDiagnostics: + """Normalize manifest failures or legacy warnings at the reporting boundary.""" + + candidates: list[tuple[object, str, object]] = [] + if outcome.manifest_present: + candidates.extend( + ( + item.path, + FAILURE_REASON_LABELS.get(item.classification, "unknown subtask failure"), + item.reason, + ) + for item in outcome.failed_items + ) + elif outcome.kind == "partial" and legacy_warning_fallback: + for warning in warnings: + path = warning.get("file") or warning.get("path") if isinstance(warning, dict) else None + candidates.append((path, _legacy_reason(warning), ocr_warning_text(warning))) + + records: list[CoverageDiagnostic] = [] + seen: set[tuple[str, str]] = set() + invalid = 0 + for raw_path, reason, raw_detail in candidates: + path = _safe_repository_path(raw_path) + if not path: + invalid += 1 + continue + key = (path, reason) + if key in seen: + continue + seen.add(key) + records.append(CoverageDiagnostic(path, reason, _safe_detail(raw_detail, reason))) + records.sort(key=lambda item: (item.path, item.reason, item.detail)) + omitted = max(0, len(records) - MAX_COVERAGE_DIAGNOSTICS) + unique_file_count = len({record.path for record in records}) + return CoverageDiagnostics( + records=tuple(records[:MAX_COVERAGE_DIAGNOSTICS]), + omitted=omitted, + invalid=invalid, + failed_total=len(seen) + invalid, + unique_file_count=unique_file_count, + ) + + +def ocr_warning_text(warning: Any, *, _seen: set[int] | None = None) -> str: + """Return warning text relevant for provider failure classification.""" + + if _seen is None: + _seen = set() + if isinstance(warning, (dict, list)): + marker = id(warning) + if marker in _seen: + return "" + _seen.add(marker) + if isinstance(warning, dict): + parts: list[str] = [] + for key in ("type", "message", "code", "status", "status_code", "detail"): + text = clean_text(warning.get(key)) + if text: + parts.append(f"{key}: {text}" if key in {"code", "status", "status_code"} else text) + for key in ("error", "details"): + nested = warning.get(key) + if isinstance(nested, dict): + text = ocr_warning_text(nested, _seen=_seen) + if text: + parts.append(text) + else: + text = clean_text(nested) + if text: + parts.append(text) + return "\n".join(parts)[:4000] + if isinstance(warning, list): + return "\n".join( + text for value in warning[:40] if (text := ocr_warning_text(value, _seen=_seen)) + )[:4000] + return clean_text(warning) + + +def llm_billing_failure_reason( + warnings: Sequence[Any], +) -> ProviderFailureReason | None: + """Map a legacy OCR billing warning to the shared safe provider reason.""" + + for warning in warnings: + text = ocr_warning_text(warning) + if text and LLM_BILLING_FAILURE_RE.search(text): + return ProviderFailureReason.RATE_OR_SPENDING_LIMIT + return None diff --git a/src/ocr_toolkit/reporting/sections.py b/src/ocr_toolkit/reporting/sections.py new file mode 100644 index 0000000..c872294 --- /dev/null +++ b/src/ocr_toolkit/reporting/sections.py @@ -0,0 +1,82 @@ +"""Pure shared finding, coverage and warning summary sections.""" + +from __future__ import annotations + +from collections.abc import Sequence +from typing import Any + +from ocr_toolkit.common.markdown import inline_code as _inline_code +from ocr_toolkit.common.markdown import neutralize_quick_actions +from ocr_toolkit.common.redaction import redact_sensitive +from ocr_toolkit.reporting.metadata import ( + CATEGORY_EMOJI, + OCR_FINDING_CATEGORY_ORDER, + OCR_FINDING_SEVERITY_ORDER, + SEVERITY_EMOJI, + finding_metadata, +) +from ocr_toolkit.reporting.result import CoverageDiagnostics, ocr_warning_text +from ocr_toolkit.reporting.text import compact_escaped_text + + +def inline_code(value: str) -> str: + """Escape controls and delimiters in report labels.""" + return _inline_code(value, escape_controls=True) + + +def report_sections( + comments: Sequence[dict[str, Any]], + diagnostics: CoverageDiagnostics, + warnings: Sequence[Any], + *, + use_emoji: bool = False, +) -> list[str]: + """Render the same bounded diagnostic sections for every output adapter.""" + lines: list[str] = [] + severity_counts: dict[str, int] = {} + category_counts: dict[str, int] = {} + for comment in comments: + severity, category = finding_metadata(comment) + if severity: + severity_counts[severity] = severity_counts.get(severity, 0) + 1 + if category: + category_counts[category] = category_counts.get(category, 0) + 1 + if severity_counts or category_counts: + lines.extend(["", "### Findings", ""]) + for value in OCR_FINDING_SEVERITY_ORDER: + count = severity_counts.get(value, 0) + if count: + icon = f"{SEVERITY_EMOJI[value]} " if use_emoji else "" + lines.append(f"- {icon}{inline_code(value)}: {count}") + for value in OCR_FINDING_CATEGORY_ORDER: + count = category_counts.get(value, 0) + if count: + icon = f"{CATEGORY_EMOJI[value]} " if use_emoji else "" + lines.append(f"- {icon}{inline_code(value)}: {count}") + + if diagnostics.records or diagnostics.invalid or diagnostics.omitted: + lines.extend(["", "### Incomplete coverage", ""]) + for diagnostic in diagnostics.records: + detail = f" — {diagnostic.detail}" if diagnostic.detail else "" + lines.append(f"- {inline_code(diagnostic.path)} — {diagnostic.reason}{detail}") + if diagnostics.invalid: + lines.append( + f"- {diagnostics.invalid} failed item(s) had no safe repository-relative path" + ) + if diagnostics.omitted: + lines.append(f"- ... and {diagnostics.omitted} more failed file record(s)") + + safe_warnings = [] + for warning in warnings[:10]: + safe = compact_escaped_text( + neutralize_quick_actions(redact_sensitive(ocr_warning_text(warning))), 500 + ) + if safe: + safe_warnings.append(safe) + if safe_warnings: + lines.extend(["", "### Review warnings", ""]) + lines.extend(f"- {warning}" for warning in safe_warnings) + if len(warnings) > len(safe_warnings): + lines.append(f"- ... and {len(warnings) - len(safe_warnings)} more warning(s)") + + return lines diff --git a/src/ocr_toolkit/reporting/text.py b/src/ocr_toolkit/reporting/text.py new file mode 100644 index 0000000..11a7f04 --- /dev/null +++ b/src/ocr_toolkit/reporting/text.py @@ -0,0 +1,87 @@ +"""Small helpers for OCR comment value normalization.""" + +from __future__ import annotations + +import re +from typing import Any + +from ocr_toolkit.common.markdown import escape_control_chars + +MARKDOWN_INLINE_SPECIAL_RE = re.compile(r"([`*_{}\[\]()+.!|~-])") + + +def clean_text(value: Any) -> str: + """Convert a JSON value to a stripped string.""" + + return "" if value is None else str(value).strip() + + +def compact_text(value: str, max_chars: int) -> str: + """Collapse whitespace and bound text for one-line review summaries.""" + + if max_chars <= 0: + return "" + + collapsed = " ".join(value.split()) + if len(collapsed) <= max_chars: + return collapsed + + if max_chars <= 3: + return "." * max_chars + + return collapsed[: max_chars - 3].rstrip() + "..." + + +def compact_escaped_text(value: str, max_chars: int) -> str: + """Escape Markdown-sensitive text before compacting review summary snippets.""" + + escaped = escape_control_chars(value) + escaped = escaped.replace("&", "&").replace("<", "<").replace(">", ">") + escaped = escaped.replace("@", "@") + escaped = MARKDOWN_INLINE_SPECIAL_RE.sub(r"\\\1", escaped) + return compact_text(escaped, max_chars) + + +def compact_control_text(value: str, max_chars: int) -> str: + """Escape controls only before compacting text rendered inside inline code.""" + + return compact_text(escape_control_chars(value), max_chars) + + +def code_text(value: Any) -> str: + """Convert a JSON value to text while preserving code indentation.""" + + if value is None: + return "" + return str(value).rstrip("\n") + + +def line_number(value: Any) -> int: + """Parse a line number, returning zero when invalid.""" + + if isinstance(value, bool): + return 0 + if isinstance(value, int): + parsed = value + elif isinstance(value, str): + text = value.strip() + if not text.isdecimal(): + return 0 + parsed = int(text) + else: + return 0 + return parsed if parsed > 0 else 0 + + +def comment_line(comment: dict[str, Any]) -> int: + """Return the first relevant new-line number for a finding location. + + Anchor on the start of the range so multi-line findings highlight the + first problematic line rather than the end of the span. + """ + + for key in ("start_line", "line", "end_line"): + parsed = line_number(comment.get(key)) + if parsed > 0: + return parsed + return 0 diff --git a/src/ocr_toolkit/reporting/usage.py b/src/ocr_toolkit/reporting/usage.py new file mode 100644 index 0000000..7146210 --- /dev/null +++ b/src/ocr_toolkit/reporting/usage.py @@ -0,0 +1,224 @@ +"""Provider-neutral formatting of admitted OCR tool usage.""" + +from __future__ import annotations + +from typing import Any + +from ocr_toolkit.common.markdown import inline_code as _inline_code +from ocr_toolkit.evidence.actions import EVIDENCE_ACTIONS +from ocr_toolkit.ocr_result import ( + MAX_TOOLKIT_MCP_USAGE_COUNT, + MAX_TOOLKIT_MCP_USAGE_SERVERS, + PUBLIC_REVIEW_TOOL_CALL_NAMES, + TOOLKIT_MCP_SERVER_NAME_RE, +) +from ocr_toolkit.reporting.text import clean_text + + +def inline_code(value: str) -> str: + """Escape controls and delimiters in report labels.""" + return _inline_code(value, escape_controls=True) + + +def nonnegative_int(value: Any) -> int | None: + """Parse a non-negative integer from OCR JSON, ignoring malformed values.""" + + if isinstance(value, bool) or value is None: + return None + + if isinstance(value, int): + return value if value >= 0 else None + + if isinstance(value, float): + if value.is_integer() and value >= 0: + return int(value) + return None + + if isinstance(value, str): + try: + parsed = int(value.strip()) + except ValueError: + return None + return parsed if parsed >= 0 else None + + return None + + +def tool_call_name(value: Any) -> str: + """Extract one closed public tool name from common OCR call shapes.""" + + if isinstance(value, str): + 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 in PUBLIC_REVIEW_TOOL_CALL_NAMES: + return name + + function_value = value.get("function") + if isinstance(function_value, dict): + name = clean_text(function_value.get("name")) + return name if name in PUBLIC_REVIEW_TOOL_CALL_NAMES else "" + + return "" + + +def tool_call_counts_from_items( + items: list[Any], +) -> tuple[int | None, list[tuple[str, int]]]: + """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 + 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: + return None, [] + + return total, list(counts.items()) + + +def format_tool_calls_summary(tool_calls: Any) -> str: + """Return one bounded report line for admitted non-zero OCR tool counts.""" + + entries: list[tuple[str, int]] + total: int | None + 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 = [] + admitted_total = 0 + + if isinstance(by_tool_value, dict): + for raw_name, raw_count in by_tool_value.items(): + if not isinstance(raw_name, str) or raw_name not in PUBLIC_REVIEW_TOOL_CALL_NAMES: + continue + if ( + not isinstance(raw_count, int) + or isinstance(raw_count, bool) + or not 0 < raw_count <= MAX_TOOLKIT_MCP_USAGE_COUNT + ): + continue + 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 not by_tool_value and isinstance(calls_value, list): + list_total, entries = tool_call_counts_from_items(calls_value) + else: + list_total = 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 entries: + total = admitted_total + else: + return "" + else: + return "" + + if total is None: + return "" + 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" + entries.sort(key=lambda item: (-item[1], item[0])) + detail_parts = [f"{inline_code(name)}: {count}" for name, count in entries] + + return f"{line} ({', '.join(detail_parts)})" + + +def format_verified_mcp_usage(*, mcp_usage: Any, evidence: Any) -> str: + """Format execution-owner-verified facts, never model-supplied usage claims.""" + + if ( + not isinstance(mcp_usage, dict) + or len(mcp_usage) > MAX_TOOLKIT_MCP_USAGE_SERVERS + or any( + not isinstance(server, str) + or TOOLKIT_MCP_SERVER_NAME_RE.fullmatch(server) is None + or not isinstance(count, int) + or isinstance(count, bool) + or not 0 < count <= MAX_TOOLKIT_MCP_USAGE_COUNT + for server, count in mcp_usage.items() + ) + ): + return "" + used = sorted(mcp_usage.items()) + if not used: + return "" + details = ", ".join(f"{inline_code(server)}: {count}" for server, count in used) + lines = [f"- reconciled MCP attempts: {len(used)} server(s) ({details})"] + completed = validated_completed_actions(evidence) + if completed is not None: + positive = [action for action in EVIDENCE_ACTIONS if completed[action] > 0] + if positive: + lines.append( + "- completed built-in evidence actions: " + + ", ".join(f"{action}: {completed[action]}" for action in positive) + ) + return "\n".join(lines) + + +def validated_completed_actions(evidence: Any) -> dict[str, int] | None: + """Read exact execution action counts without consulting rendered prose.""" + + actions = evidence.get("actions") if isinstance(evidence, dict) else None + if isinstance(actions, dict) and set(actions) == {"state", "attempted", "completed"}: + attempted = actions.get("attempted") + completed = actions.get("completed") + evidence_calls = evidence.get("calls") if isinstance(evidence, dict) else None + mandatory = evidence.get("mandatory") if isinstance(evidence, dict) else None + evidence_used = evidence.get("used") if isinstance(evidence, dict) else None + if not ( + actions.get("state") == "verified" + and isinstance(attempted, dict) + and set(attempted) == {*EVIDENCE_ACTIONS, "unattributed"} + and isinstance(completed, dict) + and set(completed) == set(EVIDENCE_ACTIONS) + and all( + isinstance(count, int) + and not isinstance(count, bool) + and 0 <= count <= MAX_TOOLKIT_MCP_USAGE_COUNT + for count in (*attempted.values(), *completed.values()) + ) + and isinstance(evidence_calls, int) + and not isinstance(evidence_calls, bool) + and 0 <= evidence_calls <= MAX_TOOLKIT_MCP_USAGE_COUNT + and isinstance(mandatory, bool) + and isinstance(evidence_used, bool) + and all(completed[action] <= attempted[action] for action in EVIDENCE_ACTIONS) + and sum(attempted.values()) == evidence_calls + and evidence_used is (sum(completed.values()) > 0) + and (not mandatory or completed["summary"] >= 1) + ): + return None + return dict(completed) + return None diff --git a/src/ocr_toolkit/review_debug.py b/src/ocr_toolkit/review_debug.py new file mode 100644 index 0000000..bdd7b84 --- /dev/null +++ b/src/ocr_toolkit/review_debug.py @@ -0,0 +1,363 @@ +"""Own bounded private diagnostic artifacts without granting review authority.""" + +from __future__ import annotations + +import hashlib +import json +import os +import re +import secrets +import stat +from collections.abc import Iterable +from pathlib import Path +from typing import Literal, get_args + +from ocr_toolkit.common.filesystem import ( + fsync_directory, + open_private_parent_directory, + same_file_identity, +) +from ocr_toolkit.reporting.model import FailureStage + +DEBUG_SCHEMA = "ocr.toolkit-debug/v1" +MAX_DEBUG_JSON_BYTES = 20_000_000 +MAX_DEBUG_STDERR_BYTES = 2_000_000 +MAX_DEBUG_JOURNAL_BYTES = 1_000_000 +MAX_DEBUG_DECISIONS = 1_000 +MAX_DEBUG_TRANSITIONS = 128 +DebugStatus = Literal["passed", "failed", "degraded", "not-run"] +ArtifactName = Literal["raw-result.json", "raw-stderr.log", "safe-result.json", "summary.md"] +ARTIFACT_LIMITS: dict[ArtifactName, int] = { + "raw-result.json": MAX_DEBUG_JSON_BYTES, + "raw-stderr.log": MAX_DEBUG_STDERR_BYTES, + "safe-result.json": MAX_DEBUG_JSON_BYTES, + "summary.md": MAX_DEBUG_JSON_BYTES, +} +_SAFE_PATH_FIELDS = frozenset( + { + "comments", + "warnings", + "message", + "manifest", + "coverage", + "failed", + "path", + "reason", + "content", + "existing_code", + "suggestion_code", + "line", + "start_line", + "end_line", + "severity", + "category", + "tool_calls", + "by_tool", + "summary", + "status", + } +) +_FACT_KEYS = frozenset( + { + "base_sha", + "head_sha", + "policy_sha", + "provider", + "context_mode", + "output_format", + "audience", + "exit_code", + "record_count", + "diagnostic_count", + "tool_count", + "server_count", + "summary_completed", + "dlp_state", + "used", + "selected", + "completed", + "failure_count", + "report_artifact", + "console", + "reasoning_effort", + "reasoning_budget", + "llm_protocol", + "review_effort", + "language", + "llm_model_sha256", + "extra_body_present", + "extra_headers_present", + "telemetry_enabled", + "content_logging", + } +) + + +class DebugBundle: + """Hold a fresh owner-only directory and observations from real check owners. + + Observations are diagnostic data only. The caller decides execution outcomes; + no captured result, journal entry or detector observation authorizes admission. + """ + + def __init__(self, directory: Path, *, other_outputs: tuple[Path, ...]) -> None: + absolute = directory.absolute() + resolved = absolute.resolve() + if any(output.resolve().is_relative_to(resolved) for output in other_outputs): + raise ValueError("review outputs must be outside the debug directory") + parent_descriptor, leaf = open_private_parent_directory(absolute) + self.directory = absolute + self._parent_descriptor = parent_descriptor + self._leaf = leaf + self._descriptor = -1 + try: + os.mkdir(leaf, mode=0o700, dir_fd=parent_descriptor) + self._descriptor = os.open( + leaf, + os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, + dir_fd=parent_descriptor, + ) + os.fchmod(self._descriptor, 0o700) + self._identity = os.fstat(self._descriptor) + self._validate_directory() + except BaseException: + if self._descriptor >= 0: + os.close(self._descriptor) + self._descriptor = -1 + os.close(self._parent_descriptor) + self._parent_descriptor = -1 + raise + self.phases: dict[FailureStage, dict[str, object]] = { + stage: {"status": "not-run"} for stage in get_args(FailureStage) + } + self.artifacts: dict[ArtifactName, dict[str, object]] = { + name: {"status": "not-run", "limit_bytes": limit} + for name, limit in ARTIFACT_LIMITS.items() + } + self.transitions: list[dict[str, object]] = [] + self.decisions: list[dict[str, object]] = [] + self.omitted_transitions = 0 + self.omitted_decisions = 0 + + def _validate_directory(self) -> None: + if self._descriptor < 0 or self._parent_descriptor < 0: + raise OSError("debug directory is closed") + current = os.fstat(self._descriptor) + named = os.stat(self._leaf, dir_fd=self._parent_descriptor, follow_symlinks=False) + if ( + not same_file_identity(self._identity, current) + or not same_file_identity(self._identity, named) + or not stat.S_ISDIR(current.st_mode) + or current.st_uid != os.getuid() + or stat.S_IMODE(current.st_mode) != 0o700 + ): + raise OSError("debug directory identity or permissions changed") + + def _write(self, name: str, content: bytes) -> None: + self._validate_directory() + temporary = ".debug-" + secrets.token_hex(12) + descriptor = os.open( + temporary, + os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, + 0o600, + dir_fd=self._descriptor, + ) + try: + os.fchmod(descriptor, 0o600) + with os.fdopen(descriptor, "wb") as stream: + descriptor = -1 + stream.write(content) + stream.flush() + os.fsync(stream.fileno()) + self._validate_directory() + os.replace(temporary, name, src_dir_fd=self._descriptor, dst_dir_fd=self._descriptor) + fsync_directory(self._descriptor) + finally: + if descriptor >= 0: + os.close(descriptor) + try: + os.unlink(temporary, dir_fd=self._descriptor) + except FileNotFoundError: + pass + + def phase( + self, stage: FailureStage, status: DebugStatus, *, facts: dict[str, object] | None = None + ) -> None: + """Record caller-owned closed facts, never raw exceptions or configuration.""" + + if stage not in self.phases or status not in get_args(DebugStatus): + raise ValueError("unsupported diagnostic phase or status") + entry: dict[str, object] = {"status": status} + if facts: + if set(facts) - _FACT_KEYS or any( + not ( + isinstance(value, bool) + or (isinstance(value, int) and -1_000_000_000_000 <= value <= 1_000_000_000_000) + or (isinstance(value, str) and re.fullmatch(r"[A-Za-z0-9_ ()-]{1,64}", value)) + ) + for value in facts.values() + ): + raise ValueError("debug facts must use bounded closed fields") + entry["facts"] = facts.copy() + self.phases[stage] = entry + if len(self.transitions) < MAX_DEBUG_TRANSITIONS: + self.transitions.append({"stage": stage, "status": status}) + else: + self.omitted_transitions += 1 + + def decision( + self, + *, + path: tuple[object, ...], + action: str, + reason: str, + detector: str | None, + value: object, + ) -> None: + """Record a real DLP branch with a value-free correlation fingerprint.""" + + if len(self.decisions) >= MAX_DEBUG_DECISIONS: + self.omitted_decisions += 1 + return + if any( + not isinstance(label, str) or re.fullmatch(r"[A-Za-z0-9_:-]{1,64}", label) is None + for label in (action, reason, detector or "none") + ): + raise ValueError("debug decision labels must be closed identifiers") + safe_path: list[str | int] = [] + for part in path[:16]: + if (isinstance(part, int) and not isinstance(part, bool)) or ( + isinstance(part, str) and part in _SAFE_PATH_FIELDS + ): + safe_path.append(part) + else: + encoded_key = str(part).encode("utf-8", errors="backslashreplace") + safe_path.append("field-" + hashlib.sha256(encoded_key).hexdigest()[:16]) + entry: dict[str, object] = { + "path": safe_path, + "path_truncated": len(path) > 16, + "action": action, + "reason": reason, + "detector": detector, + } + if isinstance(value, str): + encoded = value.encode("utf-8", errors="backslashreplace") + entry.update( + { + "value_type": "string", + "characters": len(value), + "bytes": len(encoded), + "lines": value.count("\n") + 1, + "sha256": hashlib.sha256(encoded).hexdigest(), + } + ) + else: + entry["value_type"] = "non-string" + self.decisions.append(entry) + + def capture(self, name: ArtifactName, source: Path) -> None: + """Copy a bounded prefix of one regular artifact without following links.""" + + limit = ARTIFACT_LIMITS[name] + descriptor = -1 + source_opened = False + try: + descriptor = os.open(source, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) + source_opened = True + before = os.fstat(descriptor) + if ( + not stat.S_ISREG(before.st_mode) + or before.st_nlink != 1 + or before.st_uid != os.getuid() + ): + raise OSError("unsafe debug capture source") + with os.fdopen(descriptor, "rb") as stream: + descriptor = -1 + data = stream.read(limit + 1) + after = os.fstat(stream.fileno()) + prefix = data[:limit] + truncated = len(data) > limit + changed = (before.st_size, before.st_mtime_ns) != (after.st_size, after.st_mtime_ns) + self._write(name, prefix) + self.artifacts[name] = { + "status": "degraded" if truncated or changed else "passed", + "limit_bytes": limit, + "captured_bytes": len(prefix), + "source_bytes_observed": before.st_size, + "sha256_captured": hashlib.sha256(prefix).hexdigest(), + "truncated": truncated, + "source_changed": changed, + } + except FileNotFoundError: + self.artifacts[name] = { + "status": "unavailable" if source_opened else "missing", + "limit_bytes": limit, + } + except OSError: + self.artifacts[name] = {"status": "unavailable", "limit_bytes": limit} + finally: + if descriptor >= 0: + os.close(descriptor) + + def flush(self, *, complete: bool = False) -> None: + """Atomically persist one bounded journal; raw content is never embedded.""" + + document = { + "schema_version": DEBUG_SCHEMA, + "complete": complete, + "phases": self.phases, + "artifacts": self.artifacts, + "transitions": self.transitions, + "omitted_transitions": self.omitted_transitions, + "dlp_decisions": self.decisions, + "omitted_decisions": self.omitted_decisions, + } + content = json.dumps(document, sort_keys=True, ensure_ascii=True).encode("utf-8") + if len(content) > MAX_DEBUG_JOURNAL_BYTES: + raise ValueError("debug journal exceeds its byte limit") + self._write("journal.json", content) + + def capture_parts(self, name: ArtifactName, parts: Iterable[str]) -> None: + """Retain a bounded rendered summary even when normal delivery is unavailable.""" + + limit = ARTIFACT_LIMITS[name] + data = bytearray() + truncated = False + for part in parts: + encoded = part.encode("utf-8") + remaining = limit - len(data) + if remaining == 0: + if encoded: + truncated = True + break + continue + if len(encoded) > remaining: + data.extend(encoded[:remaining]) + truncated = True + break + data.extend(encoded) + prefix = bytes(data[:limit]) + try: + self._write(name, prefix) + except OSError: + self.artifacts[name] = {"status": "unavailable", "limit_bytes": limit} + return + self.artifacts[name] = { + "status": "degraded" if truncated else "passed", + "limit_bytes": limit, + "captured_bytes": len(prefix), + "source_bytes_observed": None if truncated else len(prefix), + "sha256_captured": hashlib.sha256(prefix).hexdigest(), + "truncated": truncated, + "source_changed": False, + } + + def close(self) -> None: + """Release the pinned directory descriptor exactly once.""" + + if self._descriptor >= 0: + os.close(self._descriptor) + self._descriptor = -1 + if self._parent_descriptor >= 0: + os.close(self._parent_descriptor) + self._parent_descriptor = -1 diff --git a/src/ocr_toolkit/review_progress.py b/src/ocr_toolkit/review_progress.py new file mode 100644 index 0000000..8559127 --- /dev/null +++ b/src/ocr_toolkit/review_progress.py @@ -0,0 +1,126 @@ +"""Emit bounded toolkit-only progress independently of OCR output and admission.""" + +from __future__ import annotations + +import io +import os +import threading +from typing import TextIO, get_args + +from ocr_toolkit.reporting.model import FailureStage + +HEARTBEAT_SECONDS = 30.0 +MAX_PROGRESS_MESSAGES = 120 + + +def progress_enabled(value: str) -> bool: + """Parse the explicit progress switch without printing operator input.""" + + normalized = value.strip().lower() + if normalized not in {"", "false", "true"}: + raise ValueError("OCR_REVIEW_PROGRESS must be true, false, or empty") + return normalized == "true" + + +class ReviewProgress: + """Own one timer and a bounded stream of closed toolkit phase labels.""" + + def __init__(self, stream: TextIO, *, interval: float = HEARTBEAT_SECONDS) -> None: + if interval <= 0: + raise ValueError("progress interval must be positive") + self._stream = stream + self._interval = interval + self._stop = threading.Event() + self._lock = threading.Lock() + self._stage: FailureStage = "configuration" + self._messages = 0 + self._thread: threading.Thread | None = None + self._terminal_descriptor: int | None = None + + def start(self) -> None: + """Start optional observation; resource failures cannot reject a review.""" + + if self._thread is not None or self._stop.is_set(): + return + self.phase("configuration") + if self._stop.is_set(): + return + thread = threading.Thread(target=self._heartbeat, name="ocr-review-progress", daemon=True) + try: + thread.start() + except (RuntimeError, OSError): + self._stop.set() + return + self._thread = thread + + def _write_descriptor(self, descriptor: int, encoded: bytes) -> bool: + """Write one optional record only through a nonblocking descriptor. + + ``dup()`` and common ``/dev/fd`` reopens may share the caller's + open-file status flags. For a terminal, opening its device path gives + this observer its own nonblocking writer. A conventional blocking + stderr pipe is unavailable rather than a risk of stalling the review. + """ + + if not os.get_blocking(descriptor): + return os.write(descriptor, encoded) == len(encoded) + if not os.isatty(descriptor): + return False + if self._terminal_descriptor is None: + self._terminal_descriptor = os.open( + os.ttyname(descriptor), os.O_WRONLY | os.O_NOCTTY | os.O_NONBLOCK + ) + return os.write(self._terminal_descriptor, encoded) == len(encoded) + + def _emit(self, *, waiting: bool) -> None: + """Write one optional update only through a nonblocking transport.""" + + with self._lock: + if self._stop.is_set() or self._messages >= MAX_PROGRESS_MESSAGES: + return + prefix = "waiting " if waiting else "" + message = f"OCR progress: {prefix}phase={self._stage}\n" + try: + try: + descriptor = self._stream.fileno() + except (AttributeError, io.UnsupportedOperation): + descriptor = None + if descriptor is None: + if not isinstance(self._stream, io.StringIO): + self._stop.set() + return + self._stream.write(message) + self._stream.flush() + elif not self._write_descriptor(descriptor, message.encode("ascii")): + self._stop.set() + return + except (BlockingIOError, OSError, ValueError): + self._stop.set() + return + self._messages += 1 + if self._messages >= MAX_PROGRESS_MESSAGES: + self._stop.set() + + def phase(self, stage: FailureStage) -> None: + """Accept only a toolkit-owned phase, never a subprocess message.""" + + if stage not in get_args(FailureStage): + return + with self._lock: + self._stage = stage + self._emit(waiting=False) + + def _heartbeat(self) -> None: + while not self._stop.wait(self._interval): + self._emit(waiting=True) + + def close(self) -> None: + """Stop the timer and leave no live heartbeat after returning.""" + + self._stop.set() + if self._thread is not None: + self._thread.join() + self._thread = None + if self._terminal_descriptor is not None: + os.close(self._terminal_descriptor) + self._terminal_descriptor = None diff --git a/src/ocr_toolkit/review_receipt.py b/src/ocr_toolkit/review_receipt.py index 066587e..f6db857 100644 --- a/src/ocr_toolkit/review_receipt.py +++ b/src/ocr_toolkit/review_receipt.py @@ -14,7 +14,10 @@ MAX_TOOLKIT_MCP_USAGE_SERVERS, TOOLKIT_MCP_SERVER_NAME_RE, ) -from ocr_toolkit.result_contract import OcrResultContractError, ReviewOutcome +from ocr_toolkit.reporting.dlp import publication_dlp_state as publication_dlp_state +from ocr_toolkit.reporting.dlp import ( + publication_outcome_for_summary as publication_outcome_for_summary, +) INVALID_APPROVAL_RECEIPT_REASON = "the review-time approval receipt is missing or invalid" UNPROTECTED_APPROVAL_REASON = ( @@ -113,99 +116,6 @@ def _sha256(value: Any) -> bool: ) -def publication_dlp_state(value: Any) -> str | None: - """Validate the exact current publication-policy receipt.""" - - if value == {"state": "passed"}: - return "passed" - if not isinstance(value, dict): - return None - if value.get("state") == "private-sanitized": - if set(value) != {"state", "reason_counts", "sanitized_fields"}: - return None - reason_counts = value.get("reason_counts") - sanitized_fields = value.get("sanitized_fields") - if ( - not _valid_dlp_reason_counts(reason_counts) - or not any(reason_counts.values()) - or not isinstance(sanitized_fields, int) - or isinstance(sanitized_fields, bool) - or not 0 < sanitized_fields <= MAX_TOOLKIT_MCP_USAGE_COUNT - ): - return None - return "private-sanitized" - if not isinstance(value, dict) or set(value) != { - "state", - "reason_counts", - "retained", - "omitted", - "original", - }: - return None - if value.get("state") != "publication-filtered": - return None - reason_counts = value.get("reason_counts") - retained = value.get("retained") - omitted = value.get("omitted") - original = value.get("original") - if ( - not _valid_dlp_reason_counts(reason_counts) - or not any(reason_counts.values()) - or not isinstance(retained, dict) - or set(retained) != {"comments", "warnings"} - or not isinstance(omitted, dict) - or set(omitted) != {"comments", "warnings", "fields"} - or any( - not isinstance(count, int) - or isinstance(count, bool) - or not 0 <= count <= MAX_TOOLKIT_MCP_USAGE_COUNT - for counts in (retained, omitted) - for count in counts.values() - ) - or not isinstance(original, dict) - or set(original) != {"outcome", "selected", "completed", "reused", "failed", "waived"} - or original.get("outcome") not in {"clean", "warning", "partial", "failed", "skipped"} - or any( - not isinstance(original.get(field), int) - or isinstance(original.get(field), bool) - or not 0 <= original[field] <= MAX_TOOLKIT_MCP_USAGE_COUNT - for field in ("selected", "completed", "reused", "failed", "waived") - ) - ): - return None - selected = original["selected"] - completed = original["completed"] - reused = original["reused"] - failed = original["failed"] - waived = original["waived"] - outcome = original["outcome"] - derived_outcomes = {"failed"} | ( - {"skipped"} - if selected == 0 - else {"clean", "warning"} - if failed == 0 - else {"failed"} - if failed == selected - else {"partial"} - ) - if selected != completed + reused + failed + waived or outcome not in derived_outcomes: - return None - return "publication-filtered" - - -def _valid_dlp_reason_counts(value: Any) -> bool: - return bool( - isinstance(value, dict) - and set(value) == {"forbidden", "invalid_text", "laundering", "limit", "pii", "secret"} - and all( - isinstance(count, int) - and not isinstance(count, bool) - and 0 <= count <= MAX_TOOLKIT_MCP_USAGE_COUNT - for count in value.values() - ) - ) - - def automatic_approval_metadata_reason(toolkit_metadata: Any) -> str: """Return the closed review-time receipt blocker for automatic approval.""" @@ -493,45 +403,6 @@ def validated_review_identity(toolkit_metadata: Any) -> ReceiptReviewIdentity | return receipt_review_identity(toolkit_metadata) -def publication_outcome_for_summary(outcome: ReviewOutcome, publication: Any) -> ReviewOutcome: - """Recover only validated original coverage facts from a filtered receipt.""" - - if publication_dlp_state(publication) != "publication-filtered": - return outcome - if outcome.kind != "partial" or outcome.manifest_present: - raise OcrResultContractError( - "publication-filtered receipt is not bound to a safe result projection" - ) - original = publication["original"] - kind = original["outcome"] - if outcome.budget_exceeded and kind != "partial": - raise OcrResultContractError( - "publication-filtered receipt contradicts the result budget state" - ) - counts = { - field: original[field] for field in ("selected", "completed", "reused", "failed", "waived") - } - manifest_present = any(counts.values()) - status = { - "clean": "complete" if manifest_present else "success", - "warning": "completed_with_warnings", - "partial": "budget_exceeded" if outcome.budget_exceeded else "completed_with_errors", - "failed": "failed", - "skipped": "skipped", - }[kind] - return ReviewOutcome( - status=status, - kind=kind, - budget_exceeded=outcome.budget_exceeded and kind == "partial", - manifest_present=manifest_present, - selected_count=counts["selected"], - completed_count=counts["completed"], - reused_count=counts["reused"], - failed_count=counts["failed"], - waived_count=counts["waived"], - ) - - def _completed_evidence_calls(value: Any) -> int: """Return the closed completed-action subtotal or a non-validating sentinel.""" diff --git a/src/ocr_toolkit/review_runner.py b/src/ocr_toolkit/review_runner.py index d6d2d55..bc07f85 100644 --- a/src/ocr_toolkit/review_runner.py +++ b/src/ocr_toolkit/review_runner.py @@ -16,7 +16,7 @@ import threading import time from collections import Counter -from collections.abc import Sequence +from collections.abc import Callable, Sequence from contextlib import ExitStack from dataclasses import dataclass from io import BufferedWriter @@ -112,6 +112,7 @@ PreExecutionStatusError, write_pre_execution_status, ) +from ocr_toolkit.provider_config import REASONING_EFFORTS from ocr_toolkit.provider_failure import ( ProviderFailureProjection, provider_failure_projection, @@ -125,8 +126,24 @@ ) from ocr_toolkit.providers.gitlab_ci import acquire_gitlab_ci_outcomes from ocr_toolkit.providers.gitlab_discussions import acquire_gitlab_context +from ocr_toolkit.providers.local import ( + local_report_parts, + prepare_local_report_path, + publish_local_report, + write_local_report, +) +from ocr_toolkit.providers.local_config import validate_local_context +from ocr_toolkit.reporting.model import ( + ExecutionFacts, + FailureStage, + ReviewReport, + failed_report, + report_from_result, +) from ocr_toolkit.result_contract import OcrResultContractError, parse_result_outcome from ocr_toolkit.result_usage import normalize_token_usage, token_usage_mapping +from ocr_toolkit.review_debug import DebugBundle, DebugStatus +from ocr_toolkit.review_progress import ReviewProgress, progress_enabled from ocr_toolkit.review_receipt import toolkit_receipt_is_valid, verified_evidence_actions STDERR_PROBE_BYTES = 64 * 1024 @@ -306,6 +323,38 @@ class ReviewRefs: head: str +@dataclass(slots=True) +class ReviewRunState: + """Track the actual execution owner reached and its admitted report.""" + + local: bool = False + stage: FailureStage = "configuration" + reviewed_sha: str = "" + report: ReviewReport | None = None + debug: DebugBundle | None = None + progress: ReviewProgress | None = None + + def enter(self, stage: FailureStage) -> None: + """Enter a real execution phase and notify the optional progress observer.""" + + self.stage = stage + if self.progress is not None: + self.progress.phase(stage) + + def observe( + self, stage: FailureStage, status: DebugStatus, *, facts: dict[str, object] | None = None + ) -> None: + """Record a result only when the production owner reaches that branch.""" + + if self.debug is not None: + self.debug.phase(stage, status, facts=facts) + + def admit_report(self, report: ReviewReport) -> None: + """Accept only the finalizer's result after its atomic safe replacement.""" + + self.report = report + + @dataclass(frozen=True, slots=True) class ReviewIdentity: """Bind immutable review/provider identities to context selection.""" @@ -338,10 +387,46 @@ class EnrichmentReceipt: bootstrap_hints: dict[str, int] -def _write_isolated_runtime_config() -> None: +def _write_isolated_runtime_config(*, debug: DebugBundle | None = None) -> None: """Rebuild only validated runtime settings inside the fresh OCR home.""" - update_ocr_config(configure.build_config_updates()) + updates = configure.build_config_updates() + update_ocr_config(updates) + if debug is not None: + body = updates.get("llm.extra_body") + reasoning = body.get("reasoning") if isinstance(body, dict) else None + effort = ( + reasoning.get("effort") + if updates["llm.protocol"] == "openai-responses" and isinstance(reasoning, dict) + else body.get("reasoning_effort") + if updates["llm.protocol"] == "openai" and isinstance(body, dict) + else None + ) + debug.phase( + "configuration", + "passed", + facts={ + "provider": "local", + "context_mode": "off", + "output_format": "json", + "audience": "agent", + "llm_protocol": updates["llm.protocol"], + "review_effort": updates["effort"], + "language": updates["language"], + "llm_model_sha256": hashlib.sha256( + updates["llm.model"].encode("utf-8") + ).hexdigest(), + "extra_body_present": "llm.extra_body" in updates, + "extra_headers_present": "llm.extra_headers" in updates, + "telemetry_enabled": updates["telemetry.enabled"], + "content_logging": updates["telemetry.content_logging"], + **( + {"reasoning_effort": effort} + if isinstance(effort, str) and effort in REASONING_EFFORTS + else {} + ), + }, + ) def _verify_evidence_mcp(store: EvidenceStore) -> None: @@ -562,20 +647,31 @@ def _dlp_reasons( budgets: TextBudgets, matcher: ForbiddenMatcher, allow_horizontal_tabs: bool = False, + debug: DebugBundle | None = None, + path: tuple[object, ...] = (), + action: str = "detect-sink", ) -> Counter[str]: """Count closed DLP failures without retaining hostile strings or locations.""" reasons: Counter[str] = Counter() - stack = [value] + stack = [(value, path)] while stack: - nested = stack.pop() + nested, nested_path = stack.pop() if isinstance(nested, dict): for key, value in nested.items(): if isinstance(key, str) and key in PROVIDER_PRIVATE_RESULT_KEYS: reasons["invalid_text"] += 1 - stack.extend((key, value)) + if debug is not None: + debug.decision( + path=(*nested_path, key), + action=action, + reason="invalid_text", + detector="reserved_field", + value=key, + ) + stack.extend(((key, (*nested_path, key)), (value, (*nested_path, key)))) elif isinstance(nested, list): - stack.extend(nested) + stack.extend((item, (*nested_path, index)) for index, item in enumerate(nested)) elif isinstance(nested, str): checked = check_text( nested, @@ -586,6 +682,14 @@ def _dlp_reasons( ) if not checked.admitted: reasons[checked.reason] += 1 + if debug is not None: + debug.decision( + path=nested_path, + action=action, + reason=checked.reason, + detector=checked.detector, + value=nested, + ) return reasons @@ -718,36 +822,41 @@ def _write_private_dlp_decisions( raise ReviewRunnerError("OCR private DLP diagnostics could not be written") from exc -def _publication_sinks(payload: dict[str, object]) -> list[tuple[object, bool]]: +def _publication_sinks(payload: dict[str, object]) -> list[tuple[object, bool, tuple[object, ...]]]: """Select only OCR-controlled values that the posting owner can render.""" - sinks: list[tuple[object, bool]] = [] + sinks: list[tuple[object, bool, tuple[object, ...]]] = [] message = payload.get("message") if message is not None: - sinks.append((message, False)) + sinks.append((message, False, ("message",))) comments = payload.get("comments") if isinstance(comments, list): - for item in comments: + for index, item in enumerate(comments): if not isinstance(item, dict): continue sinks.extend( ( value, isinstance(value, str) and key in {"existing_code", "suggestion_code"}, + ("comments", index, key), ) for key, value in item.items() if key in QUARANTINE_COMMENT_FIELDS ) warnings = payload.get("warnings") if warnings is not None: - sinks.append((warnings, False)) + sinks.append((warnings, False, ("warnings",))) manifest = payload.get("manifest") coverage = manifest.get("coverage") if isinstance(manifest, dict) else None failed = coverage.get("failed") if isinstance(coverage, dict) else None if isinstance(failed, list): - for item in failed: + for index, item in enumerate(failed): if isinstance(item, dict): - sinks.extend((item[key], False) for key in ("path", "reason") if key in item) + sinks.extend( + (item[key], False, ("manifest", "coverage", "failed", index, key)) + for key in ("path", "reason") + if key in item + ) return sinks @@ -782,7 +891,11 @@ def _is_static_public_tool_key_path(path: tuple[object, ...]) -> bool: def _sanitize_nonpublication_fields( - payload: dict[str, object], *, budgets: TextBudgets, matcher: ForbiddenMatcher + payload: dict[str, object], + *, + budgets: TextBudgets, + matcher: ForbiddenMatcher, + debug: DebugBundle | None = None, ) -> tuple[dict[str, object], Counter[str], int]: """Redact unsafe private result fields without changing publication sinks.""" @@ -806,6 +919,14 @@ def _sanitize_nonpublication_fields( child_path ): redacted_fields += 1 + if debug is not None: + debug.decision( + path=child_path, + action="remove-private-field", + reason="invalid_text", + detector="reserved_field", + value=key, + ) continue if not _is_publication_sink_path( child_path @@ -819,6 +940,14 @@ def _sanitize_nonpublication_fields( if not checked_key.admitted: reasons[checked_key.reason] += 1 redacted_fields += 1 + if debug is not None: + debug.decision( + path=child_path, + action="reject-key", + reason=checked_key.reason, + detector=checked_key.detector, + value=key, + ) continue if isinstance(value, dict): nested: dict[str, object] = {} @@ -847,6 +976,14 @@ def _sanitize_nonpublication_fields( if not checked.admitted: reasons[checked.reason] += 1 redacted_fields += 1 + if debug is not None: + debug.decision( + path=child_path, + action="redact-value", + reason=checked.reason, + detector=checked.detector, + value=value, + ) projected = replacements.setdefault( value, f"ocr-redacted-{len(replacements) + 1:06d}" ) @@ -858,7 +995,11 @@ def _sanitize_nonpublication_fields( def _safe_publication_comments( - value: object, *, budgets: TextBudgets, matcher: ForbiddenMatcher + value: object, + *, + budgets: TextBudgets, + matcher: ForbiddenMatcher, + debug: DebugBundle | None = None, ) -> tuple[list[dict[str, object]], int, int]: """Retain safe finding fields and omit only findings with unsafe content.""" @@ -866,7 +1007,7 @@ def _safe_publication_comments( return [], 0, 0 retained: list[dict[str, object]] = [] omitted_fields = 0 - for item in value: + for index, item in enumerate(value): if not isinstance(item, dict): continue projected: dict[str, object] = {} @@ -881,6 +1022,9 @@ def _safe_publication_comments( allow_horizontal_tabs=( isinstance(field_value, str) and key in {"existing_code", "suggestion_code"} ), + debug=debug, + path=("comments", index, key), + action="omit-field", ): omitted_fields += 1 content_unsafe = content_unsafe or key == "content" @@ -888,17 +1032,40 @@ def _safe_publication_comments( projected[key] = field_value if projected and not content_unsafe: retained.append(projected) + elif debug is not None: + debug.decision( + path=("comments", index), + action="omit-finding", + reason="unsafe_content" if content_unsafe else "empty_projection", + detector=None, + value=None, + ) return retained, len(value) - len(retained), omitted_fields def _safe_publication_warnings( - value: object, *, budgets: TextBudgets, matcher: ForbiddenMatcher + value: object, + *, + budgets: TextBudgets, + matcher: ForbiddenMatcher, + debug: DebugBundle | None = None, ) -> tuple[list[object], int]: """Retain DLP-safe warnings while dropping unsafe or malformed items atomically.""" if not isinstance(value, list): return [], 0 - retained = [item for item in value if not _dlp_reasons(item, budgets=budgets, matcher=matcher)] + retained = [ + item + for index, item in enumerate(value) + if not _dlp_reasons( + item, + budgets=budgets, + matcher=matcher, + debug=debug, + path=("warnings", index), + action="omit-warning", + ) + ] return retained, len(value) - len(retained) @@ -1146,19 +1313,22 @@ def _publication_projection( *, forbidden: tuple[str, ...], allowed_tools: frozenset[str], + debug: DebugBundle | None = None, ) -> tuple[dict[str, object], dict[str, object], bool]: """Return a DLP-safe result plus one exact v8 publication state.""" budgets = TextBudgets(max_chars=2_000_000, max_bytes=8_000_000, max_lines=100_000) matcher = ForbiddenMatcher.compile(forbidden) sink_reasons: Counter[str] = Counter() - for sink, allow_horizontal_tabs in _publication_sinks(payload): + for sink, allow_horizontal_tabs, sink_path in _publication_sinks(payload): sink_reasons.update( _dlp_reasons( sink, budgets=budgets, matcher=matcher, allow_horizontal_tabs=allow_horizontal_tabs, + debug=debug, + path=sink_path, ) ) failures = _tool_failure_telemetry(payload.get("tool_calls")) @@ -1175,7 +1345,7 @@ def _publication_projection( if key not in {"failure", "failure_by_tool", "failure_details"} } sanitized, private_reasons, redacted_fields = _sanitize_nonpublication_fields( - sanitization_payload, budgets=budgets, matcher=matcher + sanitization_payload, budgets=budgets, matcher=matcher, debug=debug ) if failures.present: sanitized["tool_calls"] = _closed_tool_calls( @@ -1199,10 +1369,10 @@ def _publication_projection( publication_changed = True if publication_changed: comments, omitted_comments, omitted_fields = _safe_publication_comments( - payload.get("comments"), budgets=budgets, matcher=matcher + payload.get("comments"), budgets=budgets, matcher=matcher, debug=debug ) warnings, omitted_warnings = _safe_publication_warnings( - payload.get("warnings"), budgets=budgets, matcher=matcher + payload.get("warnings"), budgets=budgets, matcher=matcher, debug=debug ) projected: dict[str, object] = { "status": "budget_exceeded" if outcome.budget_exceeded else "completed_with_errors", @@ -1263,6 +1433,8 @@ def _finalize_ocr_result( *, forbidden: tuple[str, ...], toolkit_advisory: OcrToolkitAdvisory | None = None, + report_consumer: Callable[[ReviewReport], None] | None = None, + state: ReviewRunState | None = None, ) -> tuple[dict[str, int], bool, dict[str, object]]: """Validate, DLP-project, and receipt-bind one result in one atomic read/replace.""" @@ -1271,9 +1443,10 @@ def _finalize_ocr_result( usage: dict[str, int] = {} failure_telemetry = ToolFailureTelemetry(False, True, None, {}, ()) allowed_tools = PUBLIC_REVIEW_TOOL_CALL_NAMES + report: ReviewReport | None = None def finalize(payload: dict[str, object]) -> dict[str, object]: - nonlocal failure_telemetry, filtered, publication, usage + nonlocal failure_telemetry, filtered, publication, usage, report for reserved in (TOOLKIT_RESULT_KEY, TOOLKIT_ADVISORY_KEY): if reserved in payload: raise OcrResultMalformed(f"OCR result contains reserved field {reserved!r}") @@ -1281,6 +1454,8 @@ def finalize(payload: dict[str, object]) -> dict[str, object]: if not isinstance(warnings, list): raise OcrResultMalformed("OCR result warnings must be a list") failure_telemetry = _tool_failure_telemetry(payload.get("tool_calls")) + if state is not None: + state.enter("mcp-use") metadata = _review_receipt( payload, composition, @@ -1288,9 +1463,31 @@ def finalize(payload: dict[str, object]) -> dict[str, object]: enrichment, evidence_action_counts, ) + if state is not None: + evidence = metadata.get("evidence") + actions = evidence.get("actions") if isinstance(evidence, dict) else None + completed = actions.get("completed") if isinstance(actions, dict) else None + facts = None + if isinstance(evidence, dict) and isinstance(completed, dict): + facts = { + "used": evidence.get("used"), + "summary_completed": completed.get("summary"), + } + state.observe("mcp-use", "passed", facts=facts) + state.enter("dlp") projected, publication, filtered = _publication_projection( - payload, forbidden=forbidden, allowed_tools=allowed_tools + payload, + forbidden=forbidden, + allowed_tools=allowed_tools, + **({"debug": state.debug} if state is not None and state.debug is not None else {}), ) + if state is not None: + state.observe( + "dlp", + "degraded" if publication.get("state") != "passed" else "passed", + facts={"dlp_state": publication["state"]}, + ) + state.enter("result-validation") metadata["publication"] = publication metadata["schema_version"] = TOOLKIT_RESULT_SCHEMA_VERSION provider_receipt = identity.target_protection in {"protected", "unprotected"} @@ -1305,6 +1502,18 @@ def finalize(payload: dict[str, object]) -> dict[str, object]: finalized = {**projected, TOOLKIT_RESULT_KEY: metadata} if provider_receipt else projected if toolkit_advisory is not None and provider_receipt: finalized[TOOLKIT_ADVISORY_KEY] = toolkit_advisory_payload(toolkit_advisory) + if report_consumer is not None: + evidence = metadata.get("evidence") + if not isinstance(evidence, dict): + raise ReviewRunnerError("verified evidence facts are unavailable") + try: + report = report_from_result( + projected, + execution=ExecutionFacts(usage, evidence, publication, toolkit_advisory), + reviewed_sha=identity.source_sha, + ) + except OcrResultContractError as exc: + raise ReviewRunnerError("admitted review report is inconsistent") from exc return finalized try: @@ -1318,6 +1527,10 @@ def finalize(payload: dict[str, object]) -> dict[str, object]: pass raise _print_tool_failure_diagnostics(failure_telemetry) + if state is not None: + state.observe("result-validation", "passed") + if report_consumer is not None and report is not None: + report_consumer(report) return usage, filtered, publication @@ -1604,6 +1817,7 @@ def _run_background_qualified_review( artifacts: EvidenceArtifacts, refs: ReviewRefs, identity: ReviewIdentity, + state: ReviewRunState | None = None, ) -> tuple[int, BackgroundQualification]: """Run the model review only after installed OCR accepts its background.""" @@ -1614,6 +1828,8 @@ def _run_background_qualified_review( session_home=session_home, ) except BackgroundQualificationRejected as exc: + if state is not None: + state.observe("preview", "failed") try: write_pre_execution_status( artifacts.pre_execution_status, @@ -1648,15 +1864,23 @@ def _run_background_qualified_review( ) for notice in qualification.operator_notices: print(f"OCR argument qualification notice: {notice}", file=sys.stderr) - return ( - run_review( + if state is not None: + state.observe("preview", "degraded" if qualification.advisory is not None else "passed") + state.enter("subprocess") + try: + code = run_review( result_path, stderr_path, production_args, ocr_binary=ocr_binary, - ), - qualification, - ) + ) + finally: + if state is not None and state.debug is not None: + state.debug.capture("raw-result.json", result_path) + state.debug.capture("raw-stderr.log", stderr_path) + if state is not None: + state.observe("subprocess", "passed" if code == 0 else "failed", facts={"exit_code": code}) + return code, qualification def _prepare_review_output_artifacts(result_path: Path, stderr_path: Path) -> None: @@ -1753,7 +1977,7 @@ def _record_rules_path_setup( def _prepare_policy_context( - refs: ReviewRefs, ocr_args: list[str], artifacts: EvidenceArtifacts + refs: ReviewRefs, ocr_args: list[str], artifacts: EvidenceArtifacts, *, local: bool = False ) -> tuple[ReviewIdentity, list[str]]: """Capture policy identity and selected context, then materialize rules.""" @@ -1761,11 +1985,16 @@ def _prepare_policy_context( context_mode = parse_review_context_mode(os.environ.get("OCR_REVIEW_CONTEXT_MODE")) except ReviewContextModeError as exc: raise ReviewRunnerError(str(exc)) from exc + if local: + try: + validate_local_context(os.environ) + except ValueError as exc: + raise ReviewRunnerError(str(exc)) from exc reader = GitRepositoryReader(Path.cwd()) context = None author_id = None target_protection = "local" - if is_merge_request_environment(os.environ): + if not local and is_merge_request_environment(os.environ): snapshot = acquire_review_snapshot( os.environ, expected_head=refs.head, @@ -2237,6 +2466,172 @@ def run_evidence_review( ocr_args: list[str], *, preserve_private_artifacts: bool = False, + local: bool = False, + report_path: Path | None = None, + debug_dir: Path | None = None, +) -> int: + """Run the review and deliver an admitted local report or a closed failure summary.""" + + state = ReviewRunState(local=local) + report_destination: Path | None = None + try: + try: + enabled = progress_enabled(os.environ.get("OCR_REVIEW_PROGRESS", "")) + except ValueError as exc: + raise ReviewRunnerError(str(exc)) from exc + if enabled: + state.progress = ReviewProgress(sys.stderr) + state.progress.start() + if debug_dir is not None: + if not local or preserve_private_artifacts: + raise ReviewRunnerError("--debug-dir requires --local without legacy retention") + try: + state.debug = DebugBundle( + debug_dir, + other_outputs=( + result_path, + stderr_path, + report_path or Path(str(result_path) + ".md"), + ), + ) + state.debug.flush() + except (OSError, ValueError, RuntimeError) as exc: + raise ReviewRunnerError("debug directory could not be prepared safely") from exc + if report_path is not None and not local: + raise ReviewRunnerError("--report requires --local") + if local and preserve_private_artifacts: + raise ReviewRunnerError( + "--local requires result finalization and cannot preserve legacy private artifacts" + ) + if local: + try: + validate_local_context(os.environ) + except ValueError as exc: + raise ReviewRunnerError(str(exc)) from exc + ocr_args = _local_review_options(ocr_args) + destination = report_path or Path(str(result_path) + ".md") + try: + prepare_local_report_path(destination, other_outputs=(result_path, stderr_path)) + except (OSError, ValueError, RuntimeError) as exc: + raise ReviewRunnerError( + "local report destination is unavailable or unsafe" + ) from exc + report_destination = destination + code = _run_evidence_review( + result_path, + stderr_path, + ocr_args, + preserve_private_artifacts=preserve_private_artifacts, + state=state, + ) + if local: + if code == 0 and state.report is None: + raise ReviewRunnerError("local review did not produce an admitted report") + report = ( + state.report + if code == 0 + else failed_report(state.stage, reviewed_sha=state.reviewed_sha) + ) + assert report is not None + state.enter("reporting") + try: + assert report_destination is not None + publish_local_report(report, report_destination) + if state.debug is not None: + state.debug.capture("summary.md", report_destination) + except (OSError, ValueError) as exc: + failure = failed_report("reporting") + if state.debug is not None: + state.debug.capture_parts("summary.md", local_report_parts(failure)) + try: + write_local_report(failure, sys.stdout) + except (OSError, ValueError): + pass + raise ReviewRunnerError("local report artifact output failed") from exc + try: + write_local_report(report, sys.stdout) + except (OSError, ValueError) as exc: + raise ReviewRunnerError("local report output failed") from exc + state.observe("reporting", "passed", facts={"report_artifact": True, "console": True}) + return code + except Exception: + state.observe(state.stage, "failed") + if local and state.stage != "reporting": + failure = failed_report(state.stage, reviewed_sha=state.reviewed_sha) + artifact_delivered = False + console_delivered = False + if state.debug is not None: + state.debug.capture_parts("summary.md", local_report_parts(failure)) + if report_destination is not None: + try: + publish_local_report(failure, report_destination) + artifact_delivered = True + if state.debug is not None: + state.debug.capture("summary.md", report_destination) + except (OSError, ValueError): + try: + print("Local failure report artifact output failed.", file=sys.stderr) + except (OSError, ValueError): + pass + try: + write_local_report(failure, sys.stdout) + console_delivered = True + except (OSError, ValueError): + try: + print("Local failure summary output failed.", file=sys.stderr) + except (OSError, ValueError): + pass + state.observe( + "reporting", + "passed" + if console_delivered and (report_destination is None or artifact_delivered) + else "failed", + facts={"report_artifact": artifact_delivered, "console": console_delivered}, + ) + raise + finally: + if state.progress is not None: + state.progress.close() + if state.debug is not None: + try: + if state.report is not None: + state.debug.capture("safe-result.json", result_path) + state.debug.flush(complete=True) + except (OSError, ValueError): + try: + print("Local debug journal output failed.", file=sys.stderr) + except (OSError, ValueError): + pass + finally: + state.debug.close() + + +def _local_review_options(args: list[str]) -> list[str]: + """Require JSON/agent output and refuse competing local execution inputs.""" + + for option in ("--repo", "--resume", "--background", "--background-file"): + if _option_values(args, option): + raise ReviewRunnerError(f"{option} is not supported by standalone local review") + if any(arg.startswith(("-b", "-B")) and not arg.startswith("--") for arg in args): + raise ReviewRunnerError("local review background is managed by the toolkit") + formats = _option_values(args, "--format", "-f") + formats.extend( + arg[2:].removeprefix("=") for arg in args if arg.startswith("-f") and len(arg) > 2 + ) + if any(value != "json" for value in formats): + raise ReviewRunnerError("local review requires --format json") + if any(value != "agent" for value in _option_values(args, "--audience")): + raise ReviewRunnerError("local review requires --audience agent") + return [*args, "--format", "json", "--audience", "agent"] + + +def _run_evidence_review( + result_path: Path, + stderr_path: Path, + ocr_args: list[str], + *, + preserve_private_artifacts: bool, + state: ReviewRunState, ) -> int: """Prepare private evidence and run OCR through the composed MCP context.""" @@ -2249,7 +2644,12 @@ def run_evidence_review( remove_private_artifact(artifacts.dlp_decisions) except OSError as exc: raise ReviewRunnerError("OCR private pre-execution state is unsafe") from exc - refs = _immutable_review_refs(_review_refs(ocr_args)) + state.enter("identity") + try: + refs = _immutable_review_refs(_review_refs(ocr_args)) + except RepositoryEvidenceError as exc: + raise ReviewRunnerError("immutable review refs could not be resolved") from exc + state.reviewed_sha = refs.head _reject_owned_review_options(ocr_args) _prepare_review_output_artifacts(result_path, stderr_path) print("OCR evidence preflight: collecting immutable review refs", file=sys.stderr) @@ -2269,11 +2669,27 @@ def run_evidence_review( previous_handlers = _install_termination_handlers() try: try: - _write_isolated_runtime_config() - identity, effective_ocr_args = _prepare_policy_context(refs, ocr_args, artifacts) + state.enter("configuration") + _write_isolated_runtime_config( + **({"debug": state.debug} if state.debug is not None else {}) + ) + state.enter("identity") + identity, effective_ocr_args = _prepare_policy_context( + refs, ocr_args, artifacts, **({"local": True} if state.local else {}) + ) preserve_authorized = _authorize_private_artifact_preservation( identity, requested=preserve_private_artifacts ) + state.observe( + "identity", + "passed", + facts={ + "base_sha": refs.base, + "head_sha": refs.head, + "policy_sha": identity.policy_sha, + }, + ) + state.enter("evidence") store = collect_repository_evidence( base_ref=refs.base, head_ref=refs.head, @@ -2281,7 +2697,7 @@ def run_evidence_review( include_policy_records=identity.target_protection != "unprotected", ) head_sha = store.head.commit_sha if store.head else "" - identifiers = invocation_identifiers(os.environ) + identifiers = () if state.local else invocation_identifiers(os.environ) for record in collect_invocation_evidence(identifiers, head_sha=head_sha): if not store.add(record): store.add_diagnostic("review invocation evidence was truncated by store limits") @@ -2294,6 +2710,18 @@ def run_evidence_review( context_config, enrichment = _prepare_enrichment( identity, artifacts, GitRepositoryReader(Path.cwd()) ) + if state.debug is not None: + diagnostic_count = len(store.diagnostics) + sum( + len(snapshot.diagnostics) + for snapshot in (store.base, store.head, store.policy) + if snapshot is not None + ) + state.observe( + "evidence", + "degraded" if diagnostic_count else "passed", + facts={"diagnostic_count": diagnostic_count}, + ) + state.enter("mcp-preflight") composition = mcp_config.build_mcp_composition( profile="gitlab_mr" if identity.mr_author_id is not None else "local", context=context_config, @@ -2329,6 +2757,14 @@ def run_evidence_review( records = summary.get("records") if not isinstance(records, int) or isinstance(records, bool) or records < 0: raise ReviewRunnerError("OCR evidence preflight returned an invalid MCP summary") + state.observe( + "mcp-preflight", + "passed", + facts={ + "record_count": records, + "server_count": len(composition.capabilities), + }, + ) print( "OCR evidence preflight: ready " f"base={summary.get('base')} head={summary.get('head')} records={records} " @@ -2349,6 +2785,7 @@ def run_evidence_review( "--background-file", str(artifacts.bootstrap), ] + state.enter("preview") ocr_binary = _resolve_ocr_binary() exit_code, background_qualification = _run_background_qualified_review( result_path, @@ -2359,10 +2796,13 @@ def run_evidence_review( artifacts=artifacts, refs=refs, identity=identity, + **({"state": state} if state.local or state.progress is not None else {}), ) finally: previous_mask = _block_termination_signals() try: + if state.progress is not None: + state.progress.phase("cleanup") if exit_code == 0 and not preserve_authorized: evidence_action_counts = read_action_receipt(artifacts.action_receipt) if not preserve_authorized: @@ -2374,11 +2814,13 @@ def run_evidence_review( shutil.rmtree(session_home) except OSError as exc: cleanup_error = cleanup_error or exc + state.observe("cleanup", "failed" if cleanup_error is not None else "passed") if previous_home is None: os.environ.pop("HOME", None) else: os.environ["HOME"] = previous_home if cleanup_error is None and exit_code == 0 and not preserve_authorized: + state.enter("result-validation") forbidden = (*composition.secret_values,) if enrichment is not None: forbidden += enrichment.forbidden_publication @@ -2391,6 +2833,8 @@ def run_evidence_review( evidence_action_counts, forbidden=forbidden, toolkit_advisory=background_qualification.advisory, + **({"report_consumer": state.admit_report} if state.local else {}), + **({"state": state} if state.local or state.progress is not None else {}), ) except ReviewRunnerError: try: @@ -2411,6 +2855,7 @@ def run_evidence_review( _restore_termination_handlers(previous_handlers) _restore_signal_mask(previous_mask) if cleanup_error is not None: + state.enter("cleanup") try: result_path.unlink(missing_ok=True) except OSError: diff --git a/tests/providers/installed_local_review.py b/tests/providers/installed_local_review.py new file mode 100644 index 0000000..71ea79d --- /dev/null +++ b/tests/providers/installed_local_review.py @@ -0,0 +1,357 @@ +"""Cross installed CLI, immutable Git and real stdio MCP with a synthetic OCR peer.""" + +from __future__ import annotations + +import json +import os +import shlex +import shutil +import signal +import subprocess +import sys +from itertools import product +from pathlib import Path +from typing import Any + + +def rpc(process: subprocess.Popen[str], request: dict[str, Any]) -> dict[str, Any]: + """Exchange a single request with the real installed evidence server.""" + + assert process.stdin is not None and process.stdout is not None + process.stdin.write(json.dumps(request) + "\n") + process.stdin.flush() + response = json.loads(process.stdout.readline()) + assert response.get("id") == request["id"] and "error" not in response, response + return response["result"] + + +def peer(mode: str) -> int: + """Stand in for OCR only; evidence and action attribution remain real.""" + + if "--preview" in sys.argv: + return 0 + config = json.loads((Path.home() / ".opencodereview" / "config.json").read_text()) + assert config["llm"]["extra_body"]["reasoning_effort"] == "none" + server = config["mcp_servers"]["ocr_toolkit_evidence"] + if mode != "forged": + process = subprocess.Popen( + [server["command"], *server["args"]], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + rpc( + process, + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "synthetic-review-peer", "version": "1"}, + }, + }, + ) + registry = rpc(process, {"jsonrpc": "2.0", "id": 2, "method": "tools/list"}) + assert "ocr_toolkit_evidence" in {tool["name"] for tool in registry["tools"]} + summary = rpc( + process, + { + "jsonrpc": "2.0", + "id": 3, + "method": "tools/call", + "params": {"name": "ocr_toolkit_evidence", "arguments": {"action": "summary"}}, + }, + ) + assert summary.get("isError") is not True + finally: + process.communicate(timeout=10) + assert process.returncode == 0 + external = config["mcp_servers"]["synthetic_context"] + process = subprocess.Popen( + [external["command"], *external["args"]], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + response = rpc( + process, + { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": {"name": "synthetic_lookup", "arguments": {}}, + }, + ) + assert response == {"content": [{"type": "text", "text": "Synthetic context."}]} + finally: + process.communicate(timeout=10) + assert process.returncode == 0 + print( + json.dumps( + { + "status": { + "warning": "completed_with_warnings", + "partial": "completed_with_errors", + "budget": "budget_exceeded", + }.get(mode, "success"), + "comments": [] + if mode in {"clean", "warning"} + else [ + {"path": "app.py", "line": 1, "content": "Check the boundary."}, + *( + [ + { + "path": "app.py", + "line": 1, + "content": "Contact synthetic@example.invalid", + } + ] + if mode == "filtered" + else [] + ), + ], + "warnings": ["Some context was unavailable."] if mode == "warning" else [], + "summary": {"budget_exceeded": mode == "budget"}, + "tool_calls": { + "total": 2, + "by_tool": {"ocr_toolkit_evidence": 1, "synthetic_lookup": 1}, + }, + } + ) + ) + if mode == "process-signaled": + sys.stdout.flush() + os.kill(os.getpid(), signal.SIGTERM) + if mode == "parent-signaled": + sys.stdout.flush() + os.kill(os.getppid(), signal.SIGTERM) + return 2 if mode == "process-failed" else 0 + + +def external_server() -> int: + """Serve one optional operator-owned synthetic context tool over stdio.""" + + for line in sys.stdin: + request = json.loads(line) + assert request["method"] == "tools/call" + assert request["params"] == {"name": "synthetic_lookup", "arguments": {}} + print( + json.dumps( + { + "jsonrpc": "2.0", + "id": request["id"], + "result": {"content": [{"type": "text", "text": "Synthetic context."}]}, + } + ), + flush=True, + ) + return 0 + + +def main(root: Path, cli: Path) -> int: + """Exercise admission and forged-usage rejection without a live model call.""" + + root.mkdir(mode=0o700) + repository = root / "repository" + repository.mkdir() + binary_directory = root / "bin" + binary_directory.mkdir() + git = shutil.which("git") + assert git is not None + + def git_run(*args: str) -> str: + return subprocess.run( + [git, *args], cwd=repository, check=True, capture_output=True, text=True, timeout=20 + ).stdout.strip() + + git_run("init", "-q") + git_run("config", "user.name", "Synthetic") + git_run("config", "user.email", "synthetic@example.invalid") + shadow = repository / "ocr_toolkit" + shadow.mkdir() + (shadow / "__init__.py").write_text("raise RuntimeError('repository shadow imported')\n") + (repository / "app.py").write_text("VALUE = 1\n") + git_run("add", "app.py", "ocr_toolkit/__init__.py") + git_run("-c", "commit.gpgsign=false", "commit", "-qm", "base") + (repository / "app.py").write_text("VALUE = 2\n") + git_run("-c", "commit.gpgsign=false", "commit", "-qam", "head") + head = git_run("rev-parse", "HEAD") + environment = { + **os.environ, + "HOME": str(root / "operator-home"), + "PATH": os.pathsep.join((str(binary_directory), os.environ["PATH"])), + "CI": "true", + "GITHUB_ACTIONS": "true", + "CI_PROJECT_ID": "41", + "CI_MERGE_REQUEST_IID": "12", + "CI_API_V4_URL": "https://forge.example.invalid/api/v4", + "OCR_LLM_MODEL": "openai/synthetic-model", + "OCR_LLM_PROTOCOL": "openai", + "OCR_LLM_REASONING_EFFORT": "none", + "OCR_REVIEW_LANGUAGE": "English (UK)", + "OCR_LLM_TOKEN": "synthetic-provider-token", + "OCR_LLM_URL": "https://provider.example.invalid/v1", + "OCR_MCP_SERVERS_JSON": json.dumps( + { + "synthetic_context": { + "type": "stdio", + "command": sys.executable, + "args": ["-I", __file__, "--external"], + "tools": ["synthetic_lookup"], + } + } + ), + } + baselines: dict[str, tuple[int, str, bytes | None]] = {} + for mode, (debug, progress) in product( + ( + "verified", + "clean", + "warning", + "partial", + "budget", + "forged", + "filtered", + "process-failed", + "process-signaled", + "parent-signaled", + ), + ((False, False), (False, True), (True, False), (True, True)), + ): + launcher = binary_directory / "ocr" + launcher.write_text( + "#!/bin/sh\nexec " + + " ".join( + shlex.quote(value) for value in (sys.executable, "-I", __file__, "--peer", mode) + ) + + ' "$@"\n' + ) + launcher.chmod(0o700) + scenario = mode + ("-debug" if debug else "") + ("-progress" if progress else "") + result = root / f"{scenario}.json" + stderr = root / f"{scenario}.stderr" + debug_directory = root / f"{scenario}-bundle" + completed = subprocess.run( + [ + str(cli), + "review", + "--local", + "--result", + str(result), + "--stderr", + str(stderr), + *(["--debug-dir", str(debug_directory)] if debug else []), + "--", + "--commit", + head, + ], + cwd=repository, + env={**environment, "OCR_REVIEW_PROGRESS": str(progress).lower()}, + capture_output=True, + text=True, + timeout=45, + ) + if mode not in {"forged", "process-failed", "process-signaled", "parent-signaled"}: + assert completed.returncode == 0, completed.stderr + payload = json.loads(result.read_text()) + assert "_ocr_toolkit" not in payload + assert ("Check the boundary." in completed.stdout) == (mode not in {"clean", "warning"}) + assert "ocr_toolkit_evidence" in completed.stdout + assert "synthetic_context" in completed.stdout + assert head in completed.stdout + assert result.stat().st_mode & 0o777 == 0o600 + else: + assert completed.returncode == ( + 256 - signal.SIGTERM if mode == "process-signaled" else 2 + ), completed.stderr + assert "Review failed" in completed.stdout + stage = "mcp-use" if mode == "forged" else "subprocess" + assert f"Stopped at: `{stage}`" in completed.stdout + assert "Check the boundary." not in completed.stdout + if mode == "forged": + assert not result.exists() + assert "Traceback" not in completed.stderr + progress_lines = [ + line for line in completed.stderr.splitlines() if line.startswith("OCR progress:") + ] + # capture_output=True supplies a conventional blocking stderr pipe. The + # observer must not risk blocking the review or mutate that descriptor. + assert not progress_lines + assert len(progress_lines) <= 120 + assert "synthetic@example.invalid" not in "\n".join(progress_lines) + assert "OCR progress:" not in stderr.read_text() + markdown = Path(str(result) + ".md") + assert markdown.read_text() == completed.stdout + assert markdown.stat().st_mode & 0o777 == 0o600 + observed = ( + completed.returncode, + completed.stdout, + result.read_bytes() if result.exists() else None, + ) + if not debug and not progress: + baselines[mode] = observed + else: + assert observed == baselines[mode] + if debug: + journal_text = (debug_directory / "journal.json").read_text() + journal = json.loads(journal_text) + assert journal["complete"] is True + assert journal["phases"]["configuration"]["facts"]["llm_protocol"] == "openai" + assert journal["phases"]["configuration"]["facts"]["language"] == "English (UK)" + assert journal["phases"]["configuration"]["facts"]["reasoning_effort"] == "none" + assert journal["phases"]["cleanup"]["status"] == "passed" + assert journal["phases"]["reporting"]["status"] == "passed" + assert (debug_directory / "summary.md").read_text() == completed.stdout + if mode in {"forged", "process-failed", "process-signaled", "parent-signaled"}: + stage = "mcp-use" if mode == "forged" else "subprocess" + assert journal["phases"][stage]["status"] == "failed" + assert journal["phases"]["dlp"]["status"] == "not-run" + assert not (debug_directory / "safe-result.json").exists() + else: + assert (debug_directory / "safe-result.json").read_bytes() == result.read_bytes() + assert journal["phases"]["mcp-use"]["status"] == "passed" + assert journal["phases"]["result-validation"]["status"] == "passed" + if mode == "filtered": + assert ( + "synthetic@example.invalid" in (debug_directory / "raw-result.json").read_text() + ) + assert "synthetic@example.invalid" not in completed.stdout + journal_text + assert journal["phases"]["dlp"]["status"] == "degraded" + assert "omit-finding" in {item["action"] for item in journal["dlp_decisions"]} + assert {item.name for item in debug_directory.iterdir()} <= { + "raw-result.json", + "raw-stderr.log", + "safe-result.json", + "summary.md", + "journal.json", + } + assert debug_directory.stat().st_mode & 0o777 == 0o700 + assert all(item.stat().st_mode & 0o777 == 0o600 for item in debug_directory.iterdir()) + assert not (repository / ".review-context" / "evidence.json").exists() + assert not (repository / ".review-context" / "evidence-actions.json").exists() + print( + json.dumps( + { + "verified": True, + "forged_usage_rejected": True, + "ci_identity_ignored": True, + "debug_parity": True, + "progress_parity": True, + } + ) + ) + return 0 + + +if __name__ == "__main__": + if sys.argv[1] == "--external": + raise SystemExit(external_server()) + if sys.argv[1] == "--peer": + raise SystemExit(peer(sys.argv[2])) + raise SystemExit(main(Path(sys.argv[1]), Path(sys.argv[2]))) diff --git a/tests/providers/test_local_artifact.py b/tests/providers/test_local_artifact.py new file mode 100644 index 0000000..5965342 --- /dev/null +++ b/tests/providers/test_local_artifact.py @@ -0,0 +1,219 @@ +"""Real local report persistence and failure-path delivery boundaries.""" + +import io +import os +from pathlib import Path + +import pytest + +from ocr_toolkit import cli, review_runner +from ocr_toolkit.providers import local +from ocr_toolkit.reporting.model import failed_report + + +def test_atomic_private_artifact_matches_console(tmp_path: Path) -> None: + report = failed_report("identity") + console = io.StringIO() + local.write_local_report(report, console) + destination = tmp_path / "report.md" + local.publish_local_report(report, destination) + assert destination.read_text() == console.getvalue() + assert destination.stat().st_mode & 0o777 == 0o600 + assert list(tmp_path.iterdir()) == [destination] + + +@pytest.mark.parametrize("kind", ["file", "symlink", "hardlink", "directory", "fifo"]) +def test_existing_report_target_is_preserved(tmp_path: Path, kind: str) -> None: + original = tmp_path / "original" + original.write_text("caller data") + destination = tmp_path / "report.md" + if kind == "file": + destination.write_text("caller report") + elif kind == "symlink": + destination.symlink_to(original) + elif kind == "hardlink": + destination.hardlink_to(original) + elif kind == "directory": + destination.mkdir() + else: + os.mkfifo(destination) + with pytest.raises(ValueError, match="fresh"): + local.publish_local_report(failed_report("identity"), destination) + assert original.read_text() == "caller data" + assert destination.exists() + assert not list(tmp_path.glob(".ocr-report-*")) + + +def test_parent_symlink_is_rejected(tmp_path: Path) -> None: + parent = tmp_path / "link" + parent.symlink_to(tmp_path, target_is_directory=True) + with pytest.raises(ValueError, match="parent"): + local.publish_local_report(failed_report("identity"), parent / "report.md") + + +def test_partial_render_is_never_published(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + def fail(*_args: object) -> None: + raise OSError("write failed") + + monkeypatch.setattr(local, "write_local_report", fail) + with pytest.raises(OSError, match="write failed"): + local.publish_local_report(failed_report("identity"), tmp_path / "report.md") + assert list(tmp_path.iterdir()) == [] + + +def test_link_race_preserves_new_destination( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + destination = tmp_path / "report.md" + real_link = os.link + + def race(source: str, target: str, **kwargs: object) -> None: + descriptor = os.open( + target, + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + 0o600, + dir_fd=kwargs["dst_dir_fd"], # type: ignore[arg-type] + ) + with os.fdopen(descriptor, "w", encoding="utf-8") as stream: + stream.write("concurrent caller") + real_link(source, target, **kwargs) + + monkeypatch.setattr(local.os, "link", race) + with pytest.raises(FileExistsError): + local.publish_local_report(failed_report("identity"), destination) + assert destination.read_text() == "concurrent caller" + assert list(tmp_path.iterdir()) == [destination] + + +def test_parent_replacement_cannot_redirect_private_report( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + parent = tmp_path / "private" + parent.mkdir() + destination = parent / "report.md" + replacement = tmp_path / "replacement" + replacement.mkdir() + original = local.open_private_parent_directory + + def replace_after_open(path: Path) -> tuple[int, str]: + descriptor, name = original(path) + parent.rename(tmp_path / "moved") + parent.symlink_to(replacement, target_is_directory=True) + return descriptor, name + + monkeypatch.setattr(local, "open_private_parent_directory", replace_after_open) + local.publish_local_report(failed_report("identity"), destination) + assert (tmp_path / "moved" / "report.md").is_file() + assert not (replacement / "report.md").exists() + + +@pytest.mark.parametrize("target", ["result.json", "stderr.log"]) +def test_output_collision_rejected_before_execution( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, target: str +) -> None: + monkeypatch.setattr( + review_runner, "_run_evidence_review", lambda *_a, **_kw: pytest.fail("run") + ) + with pytest.raises(review_runner.ReviewRunnerError, match="destination"): + review_runner.run_evidence_review( + tmp_path / "result.json", + tmp_path / "stderr.log", + [], + local=True, + report_path=tmp_path / target, + ) + assert list(tmp_path.iterdir()) == [] + + +def test_explicit_report_persists_closed_execution_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + def fail(*_args: object, **_kwargs: object) -> int: + raise review_runner.ReviewRunnerError("execution rejected") + + monkeypatch.setattr(review_runner, "_run_evidence_review", fail) + destination = tmp_path / "report.md" + assert ( + cli.main( + [ + "review", + "--local", + "--result", + str(tmp_path / "result.json"), + "--stderr", + str(tmp_path / "stderr.log"), + "--report", + str(destination), + ] + ) + == 2 + ) + assert destination.read_text() == capsys.readouterr().out + assert "Review failed" in destination.read_text() + + +@pytest.mark.parametrize("interruption", [KeyboardInterrupt, SystemExit]) +def test_interruption_propagates_without_local_failure_delivery( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + interruption: type[BaseException], +) -> None: + def interrupt(*_args: object, **_kwargs: object) -> int: + raise interruption() + + monkeypatch.setattr(review_runner, "_run_evidence_review", interrupt) + destination = tmp_path / "report.md" + with pytest.raises(interruption): + review_runner.run_evidence_review( + tmp_path / "result.json", + tmp_path / "stderr.log", + [], + local=True, + report_path=destination, + ) + assert not destination.exists() + + +def test_report_requires_local(tmp_path: Path) -> None: + with pytest.raises(review_runner.ReviewRunnerError, match="requires --local"): + review_runner.run_evidence_review( + tmp_path / "result", tmp_path / "stderr", [], report_path=tmp_path / "report" + ) + assert list(tmp_path.iterdir()) == [] + + +def test_artifact_delivery_failure_has_closed_console_summary( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + def complete(*_args: object, state: review_runner.ReviewRunState, **_kwargs: object) -> int: + state.report = failed_report("identity") + return 0 + + def reject(*_args: object) -> None: + raise OSError("private path detail") + + monkeypatch.setattr(review_runner, "_run_evidence_review", complete) + monkeypatch.setattr(review_runner, "publish_local_report", reject) + with pytest.raises(review_runner.ReviewRunnerError, match="artifact output failed"): + review_runner.run_evidence_review(tmp_path / "result", tmp_path / "stderr", [], local=True) + output = capsys.readouterr().out + assert "Stopped at: `reporting`" in output + assert "private path detail" not in output + assert list(tmp_path.iterdir()) == [] + + +def test_console_delivery_failure_preserves_complete_artifact( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + def complete(*_args: object, state: review_runner.ReviewRunState, **_kwargs: object) -> int: + state.report = failed_report("identity") + return 0 + + def reject(*_args: object) -> None: + raise BrokenPipeError("closed console") + + monkeypatch.setattr(review_runner, "_run_evidence_review", complete) + monkeypatch.setattr(review_runner, "write_local_report", reject) + with pytest.raises(review_runner.ReviewRunnerError, match="report output failed"): + review_runner.run_evidence_review(tmp_path / "result", tmp_path / "stderr", [], local=True) + assert "Stopped at: `identity`" in (tmp_path / "result.md").read_text() diff --git a/tests/providers/test_local_review.py b/tests/providers/test_local_review.py new file mode 100644 index 0000000..1676769 --- /dev/null +++ b/tests/providers/test_local_review.py @@ -0,0 +1,181 @@ +"""Standalone provider selection and closed CLI failure contracts.""" + +import sys +from pathlib import Path + +import pytest + +from ocr_toolkit import cli, preflight, review_runner +from ocr_toolkit.evidence.artifacts import prepare_artifact_directory, repository_artifacts + + +def test_local_preflight_does_not_validate_gitlab(monkeypatch: pytest.MonkeyPatch) -> None: + seen: list[str] = [] + monkeypatch.setattr(preflight, "validate_ocr_binary", lambda: seen.append("ocr")) + monkeypatch.setattr(preflight, "validate_llm_model", lambda: seen.append("llm")) + monkeypatch.setattr(preflight, "validate_gitlab_access", lambda: pytest.fail("GitLab called")) + assert cli.main(["preflight", "--local"]) == 0 + assert seen == ["ocr", "llm"] + + +@pytest.mark.parametrize( + "args", + [ + ["--format", "text"], + ["-fsarif"], + ["-f=text"], + ["--audience=human"], + ["--repo", "other"], + ["--resume=old"], + ["-Bprivate.md"], + ["--background", "override"], + ], +) +def test_local_rejects_competing_inputs_before_artifacts( + args: list[str], tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + result, stderr = tmp_path / "result.json", tmp_path / "stderr.log" + with pytest.raises(review_runner.ReviewRunnerError): + review_runner.run_evidence_review(result, stderr, args, local=True) + output = capsys.readouterr().out + assert "Review failed" in output and "Stopped at: `configuration`" in output + assert not result.exists() and not stderr.exists() + + +def test_explicit_local_ignores_inherited_forge_identity( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.setenv("CI_PROJECT_ID", "41") + monkeypatch.setenv("CI_MERGE_REQUEST_IID", "12") + monkeypatch.setenv("CI_API_V4_URL", "https://forge.example.invalid/api/v4") + monkeypatch.delenv("OCR_REVIEW_CONTEXT_MODE", raising=False) + monkeypatch.delenv("OCR_REVIEW_CONTEXT_ADAPTERS_JSON", raising=False) + monkeypatch.setattr(review_runner, "GitRepositoryReader", lambda _root: object()) + monkeypatch.setattr( + review_runner, "acquire_review_snapshot", lambda *_a, **_kw: pytest.fail("GitLab called") + ) + refs = review_runner.ReviewRefs("a" * 40, "b" * 40) + artifacts = repository_artifacts(tmp_path) + prepare_artifact_directory(artifacts) + identity, args = review_runner._prepare_policy_context(refs, [], artifacts, local=True) + assert identity.source_sha == refs.head + assert identity.policy_sha == refs.base + assert identity.target_protection == "local" + assert identity.mr_author_id is None and identity.context is None + assert args == [] + + +@pytest.mark.parametrize( + ("name", "value"), + [ + ("OCR_REVIEW_CONTEXT_MODE", "metadata"), + ("OCR_REVIEW_CONTEXT_MODE", "enriched"), + ("OCR_REVIEW_CONTEXT_ADAPTERS_JSON", "[]"), + ], +) +def test_explicit_unsupported_context_is_not_successful_empty_acquisition( + name: str, value: str, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.delenv("OCR_REVIEW_CONTEXT_MODE", raising=False) + monkeypatch.delenv("OCR_REVIEW_CONTEXT_ADAPTERS_JSON", raising=False) + monkeypatch.setenv(name, value) + monkeypatch.setattr( + review_runner, "GitRepositoryReader", lambda _root: pytest.fail("repository read") + ) + with pytest.raises(review_runner.ReviewRunnerError, match="does not support"): + review_runner._prepare_policy_context( + review_runner.ReviewRefs("a" * 40, "b" * 40), + [], + repository_artifacts(tmp_path), + local=True, + ) + + +@pytest.mark.parametrize( + ("name", "value"), + [ + ("OCR_REVIEW_CONTEXT_MODE", "metadata"), + ("OCR_REVIEW_CONTEXT_MODE", "invalid"), + ("OCR_REVIEW_CONTEXT_ADAPTERS_JSON", "[]"), + ], +) +def test_local_context_rejected_before_preflight_io_or_review_artifacts( + name: str, + value: str, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setenv(name, value) + monkeypatch.setattr(preflight, "validate_ocr_binary", lambda: pytest.fail("binary invoked")) + monkeypatch.setattr(preflight, "validate_llm_model", lambda: pytest.fail("provider called")) + monkeypatch.setattr( + review_runner, "repository_artifacts", lambda: pytest.fail("artifacts prepared") + ) + assert cli.main(["preflight", "--local"]) == 1 + assert ( + cli.main( + [ + "review", + "--local", + "--result", + str(tmp_path / "result.json"), + "--stderr", + str(tmp_path / "stderr.log"), + ] + ) + == 2 + ) + output = capsys.readouterr() + assert "Stopped at: `configuration`" in output.out + assert "Traceback" not in output.err + + +def test_local_invalid_ref_has_closed_identity_failure( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.chdir(tmp_path) + + def reject_refs(_refs: review_runner.ReviewRefs) -> review_runner.ReviewRefs: + raise review_runner.RepositoryEvidenceError("private repository diagnostic") + + monkeypatch.setattr(review_runner, "_immutable_review_refs", reject_refs) + assert ( + cli.main( + [ + "review", + "--local", + "--result", + str(tmp_path / "result.json"), + "--stderr", + str(tmp_path / "stderr.log"), + "--", + "--commit", + "missing", + ] + ) + == 2 + ) + output = capsys.readouterr() + assert "Stopped at: `identity`" in output.out + assert "immutable review refs could not be resolved" in output.err + assert "private repository diagnostic" not in output.err + output.out + assert "Traceback" not in output.err + + +def test_failed_summary_sinks_do_not_replace_original_failure( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + class BrokenSink: + def write(self, _value: str) -> int: + raise BrokenPipeError("closed output") + + def flush(self) -> None: + pass + + monkeypatch.setattr(sys, "stdout", BrokenSink()) + monkeypatch.setattr(sys, "stderr", BrokenSink()) + with pytest.raises(review_runner.ReviewRunnerError, match="requires --format json"): + review_runner.run_evidence_review( + tmp_path / "result.json", tmp_path / "stderr.log", ["--format=text"], local=True + ) diff --git a/tests/reporting/test_adapter_parity.py b/tests/reporting/test_adapter_parity.py new file mode 100644 index 0000000..1e4329f --- /dev/null +++ b/tests/reporting/test_adapter_parity.py @@ -0,0 +1,60 @@ +"""Characterize shared report facts independently of adapter delivery envelopes.""" + +from __future__ import annotations + +import pytest + +from ocr_toolkit.posting.formatting import summarize_result +from ocr_toolkit.providers.local import local_summary +from ocr_toolkit.reporting.model import ReviewReport +from ocr_toolkit.reporting.result import normalize_coverage_diagnostics +from ocr_toolkit.reporting.sections import report_sections +from ocr_toolkit.result_contract import ReviewOutcome + + +@pytest.mark.parametrize("partial", [False, True]) +def test_adapters_share_finding_warning_coverage_and_usage_facts(partial: bool) -> None: + comments = ({"path": "src/example.py", "severity": "high", "category": "bug"},) + warnings = ({"file": "src/other.py", "message": "provider request failed"},) + outcome = ReviewOutcome( + status="completed_with_errors" if partial else "completed_with_warnings", + kind="partial" if partial else "warning", + budget_exceeded=False, + ) + diagnostics = normalize_coverage_diagnostics(outcome, warnings) + report = ReviewReport( + outcome=outcome, + comments=comments, + warnings=warnings, + diagnostics=diagnostics, + tool_calls_summary="- all OCR tool calls: 1 total (`read_file`: 1)", + token_usage_summary="- token usage: 100 total", + mcp_usage_summary="- reconciled MCP attempts: 1 server(s) (`evidence`: 1)", + publication={"state": "passed"}, + ) + local = local_summary(report) + gitlab = summarize_result( + total=1, + inline_count=1, + fallback_count=0, + warning_count=1, + comments=comments, + warnings=warnings, + outcome_status=outcome.status, + coverage_diagnostics=diagnostics, + tool_calls_summary=report.tool_calls_summary, + token_usage_summary=report.token_usage_summary, + mcp_usage_summary=report.mcp_usage_summary, + emoji=False, + ) + shared = "\n".join(report_sections(comments, diagnostics, warnings)) + assert shared and shared in local and shared in gitlab + for detail in ( + report.tool_calls_summary, + report.token_usage_summary, + report.mcp_usage_summary, + ): + assert detail in local and detail in gitlab + assert "1 finding published" in gitlab + assert "1 finding published" not in local + assert "
" in gitlab and "
" not in local diff --git a/tests/reporting/test_local.py b/tests/reporting/test_local.py new file mode 100644 index 0000000..e4367d3 --- /dev/null +++ b/tests/reporting/test_local.py @@ -0,0 +1,232 @@ +"""Production local adapter contracts, with synthetic execution-owner facts.""" + +from __future__ import annotations + +import ast +import io +from pathlib import Path + +import pytest + +from ocr_toolkit.evidence.actions import EVIDENCE_ACTIONS +from ocr_toolkit.providers.local import local_summary, write_local_report +from ocr_toolkit.reporting.model import ExecutionFacts, failed_report, report_from_result +from ocr_toolkit.result_contract import OcrResultContractError + + +def execution_facts() -> ExecutionFacts: + completed = dict.fromkeys(EVIDENCE_ACTIONS, 0) + completed["summary"] = 1 + return ExecutionFacts( + mcp_usage={"evidence": 1}, + evidence={ + "mandatory": True, + "used": True, + "calls": 1, + "actions": { + "state": "verified", + "attempted": {**completed, "unattributed": 0}, + "completed": completed, + }, + }, + publication={"state": "passed"}, + ) + + +@pytest.mark.parametrize( + ("status", "expected"), + [ + ("success", "Review complete"), + ("completed_with_warnings", "Review complete with warnings"), + ("completed_with_errors", "Review incomplete"), + ("budget_exceeded", "Review stopped at token budget"), + ("skipped", "Review skipped"), + ], +) +def test_local_outcome_matrix(status: str, expected: str) -> None: + result = { + "status": status, + "comments": [], + "warnings": [], + "summary": {"budget_exceeded": status == "budget_exceeded"}, + } + report = report_from_result(result, execution=execution_facts(), reviewed_sha="a" * 40) + summary = local_summary(report) + assert expected in summary + assert "- Reviewed commit: `" + "a" * 40 + "`" in summary + assert "completed built-in evidence actions: summary: 1" in summary + assert "DLP admission: passed" in summary + assert "
" not in summary + assert "Posting:" not in summary + assert "GitLab" not in summary + assert "approval" not in summary.lower() + + +def test_all_findings_ignore_posting_environment(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OCR_MAX_POST_COMMENTS", "1") + monkeypatch.setenv("OCR_POST_BADGES", "true") + comments = [ + { + "path": f"src/module_{index}.py", + "start_line": 10, + "severity": "HIGH", + "category": "bug", + "content": f"Finding body {index}", + "existing_code": "old()", + "suggestion_code": "new()", + } + for index in range(32) + ] + result = {"status": "success", "comments": comments, "warnings": []} + report = report_from_result(result, execution=execution_facts()) + stream = io.StringIO() + write_local_report(report, stream) + output = stream.getvalue() + assert output.count("### Finding ") == 32 + assert output.count("#### Existing code") == 32 + assert "32 findings" in output + assert "- `high`: 32" in output + assert "shields.io" not in output + assert "published" not in output + assert "
" not in output + comments[0]["content"] = "changed after report snapshot" + second = io.StringIO() + write_local_report(report, second) + assert second.getvalue() == output + + +def test_model_content_cannot_escape_literal_fences_or_control_terminal() -> None: + result = { + "status": "success", + "comments": [ + { + "path": "src/example.py\n/approve", + "content": "```\n\n\x1b[31m", + "suggestion_code": "````\n![tracking](https://example.invalid/pixel)", + } + ], + "warnings": [], + } + stream = io.StringIO() + write_local_report(report_from_result(result, execution=execution_facts()), stream) + output = stream.getvalue() + assert "\x1b" not in output + assert "````\n```\n