Skip to content

T021 follow-on: the cyclic-schema test asserts only that GateRefused escapes (jsonschema>=4.18) - #55

Merged
brettheap merged 2 commits into
mainfrom
r2/t021-followon-jsonschema
Oct 7, 2026
Merged

brettheap merged 2 commits into
mainfrom
r2/t021-followon-jsonschema

Conversation

@brettheap

@brettheap brettheap commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Authored by lane openXfactory-3, delegated by lane openxfactory-4 (lane4-to-lane3.log 2026-10-06T19:07:30Z; READY note #656 6023492215)

T021 follow-on: the cyclic-schema test asserts only that GateRefused escapes (jsonschema>=4.18)

The defect

tests/test_gate_console_schema_source.py::test_a_host_schema_the_console_cannot_apply_refuses_as_gate_refused pinned the exact failure mode jsonschema gives a cyclic MAPPING schema: it asserted the refusal text ...could not be checked: RecursionError. pyproject.toml allows jsonschema>=4.18 with no upper bound, so a supported version that refuses the cycle by another route (a SchemaError, a ValueError, a changed message) would turn the test red while the console still did its job. Copilot raised it on #40 (r4199337845); the holder classed it an accepted limit under #656 5988818366 and assigned this follow-on (READY note #656 6023492215).

The fix

The cyclic mapping now asserts what the cyclic list beside it already asserted: the outcome starts with GateRefused: . An exception that escaped reads ESCAPED ... and fails that prefix. The test's intent is unchanged (a cyclic schema a host supplies refuses as GateRefused, never as another exception); only the version-specific step and exception text are dropped. The product code is untouched: _schema_errors already catches any exception at each of its three steps.

Files changed: tests/test_gate_console_schema_source.py only (+10 -7).

The falsifier, stated plainly

The old assertion does not fail under any released jsonschema in the supported range today. I installed 10 versions in throwaway venvs (the repository's CI install, pip install -e ".[test]", with jsonschema pinned to each) and ran the same child-interpreter body the test runs:

jsonschema 4.18.0 cyclic_mapping old_assert=PASS new_assert=PASS :: GateRefused: ...schema could not be checked: RecursionError: maximum recursion dep...
jsonschema 4.18.0 cyclic_list    old_assert=PASS new_assert=PASS :: GateRefused: ...schema is not a valid JSON Schema: [[...]] is not of type 'object'
(identical lines for 4.18.6, 4.19.2, 4.20.0, 4.21.1, 4.22.0, 4.23.0, 4.24.1, 4.25.1, 4.26.0)

So the defect is latent, not live: no version in 4.18.0 to 4.26.0 breaks the old assertion. What I measured instead is that the old assertion pins a library implementation detail (the exception class and step of a pathological traversal), and what happens when the library changes it.

A simulated library change. A throwaway sitecustomize (in a copy of the tree, never committed) makes Draft202012Validator.check_schema refuse a cyclic schema by another route (the cycle becomes a SchemaError, or a ValueError), run under 4.18.0 and under 4.26.0, the installed version, old test file against the new:

installed jsonschema 4.18.0 and 4.26.0, identical:
 library raises SchemaError  old: 1 failed  AssertionError: GateRefused: ...schema is not a valid JSON Schema: a cyclic schema is not a schema
                             new: 1 passed
 library raises ValueError   old: 1 failed  AssertionError: GateRefused: ...schema could not be checked: ValueError: cycle detected
                             new: 1 passed

In both, the console still refuses as GateRefused; only the old assertion's pinned text was wrong.

Mutants of the new guard, killed. Throwaway copies of gate_console.py, run against the NEW test:

M1 the check step's catch-all removed:  1 failed  AssertionError: ESCAPED RecursionError: maximum recursion depth exceeded
M2 the check step raises RuntimeError:  1 failed  AssertionError: ESCAPED RuntimeError: ...could not be checked: RecursionError: maximum recursion depth exceeded
control (restored):                     1 passed

So the loosened assertion still catches what matters: a cyclic schema whose refusal is not a GateRefused.

Run counts

  • The edited file alone, under each of the 10 jsonschema versions above (4.18.0 to 4.26.0): 6 passed each.
  • The whole suite in its lone CI form (python -m pytest -q --deselect <LEFT_OUT>, the pytest step of validate.yml; FORCE_COLOR, CLICOLOR, PY_COLORS, COLORTERM unset, TERM=dumb; installed jsonschema 4.26.0), at this head on 6f25915: 1161 passed, 4 skipped, 1 deselected. Test count unchanged, so the pinned triple's floors do not move.

Ruled / decisions

Ruled: Copilot r4199337845 accepted limit under #656 5988818366; this follow-on assigned by the holder (lane4-to-lane3.log 2026-10-06T19:07:30Z). Decision: take Copilot's one-line remedy as given (assert only GateRefused escapes), not a version cap in pyproject.toml.

Not done

  • No pyproject.toml change (no upper bound added): the loosened assertion is the remedy.
  • No change to gate_console.py.
  • No other test in the file touched.

🤖 Generated with Claude Code

Summary by Sourcery

Make cyclic-schema tests validate the console’s refusal contract without depending on jsonschema-specific exception details.

Enhancements:

  • Relax cyclic-schema test expectations to verify only that cyclic mapping and list schemas are refused as GateRefused, independent of jsonschema’s version-specific refusal details.

Tests:

  • Update the cyclic-schema console test to cover both cyclic cases with version-agnostic GateRefused prefix assertions.

…escapes (jsonschema>=4.18)

tests/test_gate_console_schema_source.py pinned the exact failure mode
jsonschema gives a cyclic MAPPING schema ("could not be checked:
RecursionError"). pyproject.toml allows jsonschema>=4.18 with no upper
bound, so a supported version that refuses the cycle by another route
(a SchemaError, a ValueError, a changed message) would turn the test red
while the console still did its job: every such route lands in a
GateRefused, because _schema_errors catches any exception at each of its
three steps.

The cyclic mapping now asserts what the cyclic list beside it already
asserted, that the outcome is a GateRefused (an escaped exception reads
"ESCAPED ..." and fails the prefix). The test's intent is unchanged; only
the version-specific step and exception text are dropped.

Refs #40 (Copilot r4199337845, accepted limit under
openxFactory#656 5988818366, assigned as a follow-on on 2026-10-06).

Lane: openXfactory-3
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 7, 2026 00:42
@sourcery-ai

sourcery-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

The PR removes a brittle jsonschema implementation-detail assertion from the cyclic-schema console test and verifies instead that both cyclic schema variants are refused as GateRefused, preserving the intended behavior across supported jsonschema versions.

File-Level Changes

Change Details Files
Generalize cyclic-schema test expectations to enforce only the public refusal contract.
  • Replace the cyclic mapping’s jsonschema-specific RecursionError/message assertion with a shared GateRefused prefix check.
  • Apply the same assertion to cyclic mapping and cyclic list cases, while preserving failure for escaped exceptions.
  • Leave production code, dependency constraints, and all other tests unchanged.
tests/test_gate_console_schema_source.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused test change preserves escaped-exception detection while removing brittle implementation-specific expectations.

Review effort: Balanced
Findings: None

What changed in this PR

Updates cyclic-schema tests to verify the stable GateRefused contract without depending on jsonschema internals.

Changes:

  • Applies one prefix assertion to both cyclic schema variants.
  • Documents why validation-step and exception details are intentionally excluded.
File Description
tests/​test_gate_console_schema_source.py Makes cyclic-schema assertions version-independent.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@brettheap brettheap changed the title DRAFT: T021 follow-on: the cyclic-schema test asserts only that GateRefused escapes (jsonschema>=4.18) T021 follow-on: the cyclic-schema test asserts only that GateRefused escapes (jsonschema>=4.18) Oct 7, 2026
@brettheap
brettheap marked this pull request as ready for review October 7, 2026 00:44
@sourcery-ai

sourcery-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 16 hours and 34 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap requested a balanced review from Copilot October 7, 2026 01:02

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery assessment

Approved.

@sonarqubecloud

sonarqubecloud Bot commented Oct 7, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused test change preserves the intended exception-boundary coverage without pinning dependency-specific behavior.

Review effort: Balanced
Findings: None

@brettheap

Copy link
Copy Markdown
Contributor Author

READY at 38b1912

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Holder, automatic READY on Brett's word "land lane 3's PRs when green" (2026-10-06). At the head: CLEAN, rollup SUCCESS, nothing pending, 0 unresolved threads, and Copilot at 38b1912e reads "🟢 Approval recommended". The per-repo lander checks closing keywords and lands it.

@brettheap
brettheap merged commit da73609 into main Oct 7, 2026
4 checks passed
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

LANDED — lane openxfactory-4, 2026-10-07T01:06:17Z, PR #55 → da73609 (opensoft/openXdox-code main; plain gate)

Brett: plan 038 RULED incl. lane openxfactory-4 lands every PR (#656 6013547504); Start release 2 (#656 6001702967)

brettheap added a commit that referenced this pull request Oct 8, 2026
Brings T101 (#48), T098 (#51), T103 (#49), T096 (#53), T100 (#54), T099
(#47), T021's follow-on (#55), T102 (#52), T022 (#41, landed as aefd109),
T097 (#50) and T096's follow-on (#56).

Conflicted: scripts/composed_placements.py and tests/test_composed_placements.py,
both T022's files, which this branch carried from #41 through #44. Main's
landed side is kept for both. tests/declared_exclusion.yaml merged clean
(T101's entry edits beside T073's header and open_until).

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants