Instrument release-admission metrics (#627) - #641
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Implement bounded release-admission observability
WalkthroughAdd a fail-closed release-admission gate with bounded JSONL metrics and traces. Run it in a non-blocking workflow job, publish its result, retain artefacts, and validate success, failure, timeout, trace-delivery, and identifier-boundedness behaviour. ChangesRelease admission observability
Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant Gate as require-release-admission-canaries.sh
participant Git
participant GitHubAPI
participant Metrics as JSONL metrics and traces
ReleaseWorkflow->>Gate: execute admission check
Gate->>GitHubAPI: resolve commit and workflow run
Gate->>Git: fetch candidate revision
Gate->>GitHubAPI: verify freshness and evidence
Gate->>Metrics: write bounded records
Gate-->>ReleaseWorkflow: report outcome and error category
Suggested labels: Poem
Merge Risk: 🟡 Moderate · up to The admission scaffold can lose operation-level observability on a clock failure, while a hung job may occupy the release workflow for hours. These should be fixed before merge. Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 3 warnings)
✅ Passed checks (10 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 62.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 75 functions across 8 files. (7 skipped: 7 unsupported.) Full details: Testing (Overall)Explanation Fail the testing check because the PR adds six documented adapter boundaries, but the runtime tests inject only Resolution Add isolated subprocess tests that inject each documented adapter and assert the adapter receives the expected arguments and bounded records. Add exact trace assertions for every executed operation, gate completion, workflow-output delivery, and trace-delivery result, including failure and observation paths. Add a valid evidence-producer fixture if gate-success behaviour remains part of the contract, and assert its success metrics and outputs. Full details: Developer DocumentationExplanation Fail the developer-documentation check because the pull request adds the Resolution Add a developer-guide subsection or quality-gates entry for Full details: Unit ArchitectureExplanation FAIL: Refactor the new admission script before merge. The pull request introduces Resolution Split the implementation into explicit units. Make admission queries and evidence decisions return structured results with explicit errors and no metric, trace, file, or workflow-output writes. Keep GitHub and Git access behind narrow injected query or command adapters. Make timing a checked injected clock dependency; remove the hard-coded Full details: Domain ArchitectureExplanation Separate the release-admission policy from the GitHub and Git protocol details before merge. The new script places external representations inside policy-named functions: Resolution Create explicit adapter functions such as Comment |
Reviewer's GuideThis PR instruments the RFC 0005 release-admission scaffold with bounded, identifier-safe JSONL counters and duration observations, exports and summarizes the results in GitHub Actions, and fail-closes release publication behind a tested read-only admission job. Sequence diagram for release admission and publication gatingsequenceDiagram
participant Workflow as GitHub Actions
participant Gate as Admission script
participant GitHub as GitHub API
participant Git as Git fetch
participant Evidence as Evidence checks
participant Artifact as JSONL artifact
participant Release as Publication job
Workflow->>Gate: run require-release-admission-canaries.sh
Gate->>GitHub: resolve_tag_commit()
Gate->>Git: fetch_candidate_revision()
Gate->>GitHub: fetch_workflow_run()
Gate->>Evidence: check_scan_freshness()
Gate->>Evidence: verify_evidence()
Gate-->>Workflow: emit gate outcome and operation metrics
Workflow->>Artifact: upload release-admission-metrics
alt admission succeeds
Workflow->>Release: allow publication
else admission fails or is unknown
Workflow-->>Release: block publication
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please investigate the cause of the following issue using codegraph exploration and research, identify a fix and provide an AI coding agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7204972b5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai please addapt the following issue text for netsuke's Python infrastructure and raise a GitHub issue against leynos/netsuke: |
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- current PR revision ---'
gh pr view 641 --repo leynos/netsuke --json headRefOid,baseRefOid,commits \
--jq '{headRefOid, baseRefOid, commits: [.commits[] | {oid, messageHeadline}]}'
git rev-parse HEAD
printf '%s\n' '--- admission gate source map ---'
ast-grep outline .github/scripts/require-release-admission-canaries.sh \
--items all --view expanded
printf '%s\n' '--- admission gate boundaries, policy, adapters, and ambient inputs ---'
rg -n -C 5 \
'^(github_|git_|clock_|metric_|trace_|output_|resolve_tag_commit|fetch_candidate_revision|fetch_workflow_run|check_scan_freshness|verify_evidence|complete_gate|main|emit_|run_)|NETSUKE_RELEASE_ADMISSION_(GH|GIT|CLOCK|METRICS|TRACE|OUTPUT|ENFORCE)|GITHUB_(REPOSITORY|SHA|OUTPUT)|gh api|git fetch|workflow_runs|repos/' \
.github/scripts/require-release-admission-canaries.sh
printf '%s\n' '--- runtime test source map and adapter injection/assertions ---'
ast-grep outline scripts/tests/test_release_admission_metrics.py \
--items all --view expanded
rg -n -C 6 \
'NETSUKE_RELEASE_ADMISSION_(GH|GIT|CLOCK|METRICS|TRACE|OUTPUT)|_write_recording_adapters|test_explicit_adapters|trace|adapter|arguments|calls_by_adapter' \
scripts/tests/test_release_admission_metrics.py \
scripts/tests/release_admission_test_support.py \
scripts/tests/test_release_admission_metric_failures.py \
scripts/tests/test_release_admission_metric_boundedness.py
printf '%s\n' '--- runtime target and CI integration ---'
rg -n -C 5 \
'^(PYTHON_BASELINE|PYTEST_VERSION|HYPOTHESIS_VERSION|test-release-admission)|test-release-admission|pull_request' \
Makefile .github/workflows/ci.yml
printf '%s\n' '--- developer documentation for runtime test target and architecture ---'
rg -n -C 5 \
'test-release-admission|pytest==|hypothesis==|Python 3\.14|release-admission|ADR-018|adapter|boundary' \
docs/developers-guide.md docs/adr-018-release-admission-observability.md
printf '%s\n' '--- related issues ---'
gh issue list --repo leynos/netsuke --state all --limit 100 \
--search '"release admission" OR "adapter" OR "architecture" OR "test-release-admission" in:title,body' \
--json number,title,state,url,body \
--jq '.[] | {number,title,state,url,body}'Length of output: 202565 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- revision ---'
gh pr view 641 --repo leynos/netsuke --json headRefOid --jq .headRefOid
git rev-parse HEAD
printf '%s\n' '--- gate function declarations ---'
rg -n '^[a-zA-Z_][a-zA-Z0-9_]*\(\)[[:space:]]*\{' \
.github/scripts/require-release-admission-canaries.sh
printf '%s\n' '--- gate policy and adapter implementation ---'
sed -n '1,360p' .github/scripts/require-release-admission-canaries.sh
printf '%s\n' '--- runtime test function declarations ---'
rg -n '^def (test_|_assert|_write|_run|_trace)' \
scripts/tests/test_release_admission_metrics.py \
scripts/tests/release_admission_test_support.py
printf '%s\n' '--- explicit-adapter runtime test ---'
rg -n -A150 -B10 \
'^def test_explicit_adapters_receive_native_boundary_contracts' \
scripts/tests/test_release_admission_metrics.py
printf '%s\n' '--- recording-adapter helper ---'
rg -n -A180 -B10 \
'^def _write_recording_adapters' \
scripts/tests/test_release_admission_metrics.py
printf '%s\n' '--- developer guide release-admission section ---'
rg -n -A180 -B15 \
'^## Release-admission|^### Release-admission|test-release-admission' \
docs/developers-guide.md
printf '%s\n' '--- Make target and CI step ---'
rg -n -A20 -B8 '^test-release-admission:|test-release-admission' \
Makefile .github/workflows/ci.ymlLength of output: 62389
Mark all four checks as resolved at current PR head
Keep the current implementation. Do not create a follow-up issue for these findings. Repository commands were not executed in this inspection environment. Static inspection verifies the requested architecture, test coverage, CI integration, and documentation changes. You are interacting with an AI system. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message in a code block that I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Complex Methodscripts/tests/test_release_admission_metric_boundedness.py: test_identifiers_never_become_metric_labels What lead to degradation?test_identifiers_never_become_metric_labels has a cyclomatic complexity of 9, threshold = 9 Why does this problem occur?A Complex Method has a high cyclomatic complexity. The recommended threshold for the Python language is a cyclomatic complexity lower than 9. How to fix it?There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_method.js
function postItem(item) {
if (!item.id) {
- if (item.x != null && item.y != null) {
- post(item);
- } else {
- throw Error("Item must have x and y");
- }
+ // extract a separate function for creating new item
+ postNew(item);
} else {
- if (item.x < 10 && item.y > 25) {
- put(item);
- } else {
- throw Error("Item must have an x and y value between 10 and 25");
- }
+ // and one for updating existing items
+ updateItem(item);
}
}
+
+function postNew(item) {
+ validateNew(item);
+ post(item);
+}
+
+function updateItem(item) {
+ validateUpdate(item);
+ put(item);
+}
+ |
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat o11y, code safety, documentation and validation coverage as in scope). ❌ Failed checks (2 errors, 3 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
Record the bounded metric vocabulary, fail-closed classification, JSONL workflow artefact export, and operator workflow for the release-admission gate. Index the ADR and link the observability work from roadmap section 9.4.
Point the implementation references at the release-admission test modules present in the workflow-contract and script test suites.
Emit bounded JSONL counters and operation latency observations from the release-admission scaffold. Retain the records as a workflow artefact and surface the gate outcome in the job summary before publication can proceed. Validate fixed labels and error categories with subprocess, property, and workflow-contract tests.
Load the workflow-contract validator without changing the global Python search path, and narrow the workflow dependency before inspecting it. Terminate the fetched revision options explicitly so generated test values cannot be interpreted as Git options.
Fail the gate when its final metric cannot be written, and bound every GitHub or Git command with a validated timeout category. Move runner-only paths to step environments, preserve dry-run artefact behaviour, and retain the scaffold as non-blocking until a real evidence producer is available. Expand the metric, workflow, and boundedness contracts, and document the operator-facing behaviour.
Cover mismatched revisions, absent workflow evidence, and elapsed timeout metrics so the bounded emission contract remains enforced.
Extract narrow validation and workflow-contract assertions to satisfy the configured CodeScene complexity and method-size limits without changing the bounded metric contract.
Record unavailable evidence as a bounded admission failure while allowing the RFC 0005 scaffold workflow to complete in observation mode. Tighten timeout cleanup and cover the configuration and workflow contracts.
Reject synthetic freshness as missing evidence while retaining its non-blocking observation result. Provision the required timing runtime and cover targeted workflow-run and configuration failure boundaries.
Align the release-admission job with main's GitHub-hosted runner policy so actionlint recognises the workflow after the rebase.
Gate the runtime contract in pull-request CI and isolate external Bash effects behind narrow adapters. Export bounded trace records alongside existing metrics without changing observation-mode publication.
Preserve bounded operation results when the monotonic clock fails, and strengthen release-admission runtime and workflow-contract coverage.
Preserve the empty-run distinction and narrow fake-command values so the review repairs satisfy the repository's Python quality gates.
Isolate external protocol effects and bounded policy decisions from gate orchestration and reporting. Exercise every adapter and complete trace hand-off while documenting the dedicated runtime gate.
Extract independent metric, output, trace, and sink assertions so the contract tests remain below the CodeScene complexity threshold.
Extract the boundedness assertions and consolidate invalid configuration coverage without weakening the gate contracts. Preserve both ADR decisions after `main` assigned ADR-018 to Ninja option handling.
b2d5bd1 to
02e3ba0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
4 participants
Summary
Instrument the limited RFC 0005 release-admission scaffold with bounded JSONL
counters and duration observations. Operators can identify fixed GitHub API,
Git fetch, and evidence-check failures without exposing revisions, run IDs,
paths, URLs, or workflow content as metric labels.
Until a real evidence producer is connected, the read-only scaffold remains
non-blocking. It retains its metrics artefact and job-summary result for every
non-dry-run execution, while recording any admission failure for operators.
Closes #627
Review walkthrough
Validation
make check-fmt: passedmake typecheck: passedmake lint: passedmake doc-coverage: passed (99.11%)make test: passed (2,753 tests and doctests)make test-workflow-contracts: passed (110 tests)make markdownlint: passedmake nixie: passedcomenq(not yet complete)References
Summary by Sourcery
Instrument the RFC 0005 release-admission scaffold with bounded, artifact-backed observability while keeping publication non-blocking until a real evidence producer is available.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: