Skip to content

Fix two checks that appeared to run and didn't: inert complexity budget, CI-skipped real-SDK tracing test - #112

Merged
mrojas54 merged 2 commits into
mainfrom
ai-ccd-cc/complexity-config-tracing-ci
Aug 11, 2026
Merged

Fix two checks that appeared to run and didn't: inert complexity budget, CI-skipped real-SDK tracing test#112
mrojas54 merged 2 commits into
mainfrom
ai-ccd-cc/complexity-config-tracing-ci

Conversation

@mrojas54

Copy link
Copy Markdown
Owner

Fixes the two High findings from the 20260811 tech-debt assessment. Both are the same shape: a check that appeared to run and didn't. Neither changes runtime behaviour.

1 — The complexity budget had two declarations, and the authoritative-looking one was inert

pyproject.toml declared [tool.ruff.lint.mccabe] max-complexity = 10. Ruff only evaluates C901 when the rule is selected, and this project keeps Ruff's default rule set (E4, E7, E9, F), which excludes it — so ruff check . never read the value.

Verified by control: re-running against report.py, which contains a complexity-20 function, with --config "lint.mccabe.max-complexity = 1" still reports clean.

The budget that actually governs is complexity_gate.DEFAULT_THRESHOLD. The gate cannot read pyproject.toml by design — collect_complexities() invokes Ruff with --isolated precisely so the base snapshot, written to a temp dir with no pyproject.toml, is measured under the same config as the working tree. Two declarations of "10", one decorative; tightening the obvious one would have been a silent no-op.

A test was pinning the decoration. test_project_config_and_ci_enforce_the_regression_gate asserted the inert key — under that name. Deleting the dead config turned the suite red and told you to put it back, which is how it survived in a repo this disciplined.

The test now asserts the invariant instead: a mccabe budget may be declared only when C901 is actually selected. That permits either design — enforce through Ruff, or leave the gate as sole owner — and forbids only the misleading middle state. Confirmed non-vacuous by evaluating the new assertion against the old config state (it fails, as it should).

Why not just enable the rule?

extend-select = ["C90"] was the alternative. It fails CI immediately on 8 pre-existing functions in src/toolbench — topped by report.py:623 _render_summary (20) and complex_runner.py:209 _assert_deps_base_safe (17), and including shell_safety.py:154 read_escapes (14), the safety-critical bash tokenizer. That is a refactor decision, not a side effect of a config cleanup, so this PR leaves it open. The gate's threshold is unmoved at 10.

The assessment filed those 8 separately as a Medium finding, with a suggestion to record them as a dated baseline so the ratchet has a visible starting line.

2 — The only real-SDK tracing test never ran in CI

tests/test_observability.py::test_real_sdk_export_is_sanitized is guarded on lmnr being importable. The gate job installs no optional extras, so it skipped on every push and pull request while CI reported green. It is the only test exercising the real Laminar SDK's export sanitization rather than a fake.

The code it guards took 35 of the last 46 commits, many corrective — sanitizing SDK-derived context, isolating SDK system exits, preserving interrupts, disabling inherited debugger context. The most-churned and most security-relevant path had its only real-SDK check dark.

Why nobody noticed: a working .venv generally has the extra installed, so the test runs on a developer machine and skips only in CI — the one arrangement where the skip is invisible. Confirmed during review: after a plain uv sync pruned the local venv to CI's 13 packages, the skip appeared locally for the first time.

Adds a second tracing job rather than adding the extra to gate. The default install is 13 packages with zero required runtime dependencies — the repo's strongest supply-chain property, worth keeping intact. The new lane is the single place the opt-in is actually taken.

The lane asserts lmnr is importable before running pytest, so a rename or lock change fails loudly instead of quietly restoring the same green skips. A new test also asserts the lane exists in ci.yml, matching the repo's existing convention of testing its own CI config.

Verification

Check Result
gate default install (no extra, lmnr absent) 748 passed / 3 skipped, exit 0
tracing lane (extra installed) 748 passed / 3 skipped, exit 0 — test_observability.py:99 absent from the skip list, i.e. it ran
ruff check . clean, exit 0
mypy --strict src/toolbench tests clean, exit 0 — also clean with the extra installed
complexity_gate --base main 0 errors, 0 warnings
ci.yml parses; both jobs present; the folded python -c joins to one valid line

Both lanes were run against real synced environments, not the developer venv.

Reviewer notes

  • No behaviour change. Removing the inert config cannot alter linting, because the setting was never evaluated.
  • The tracing job uses a shallow checkout deliberately — unlike gate, it runs no Git-base comparison, so fetch-depth: 0 is unnecessary.
  • CI cost: one extra job installing 49 packages instead of 13, running a ~21s suite.
  • No ${{ }} interpolation is introduced into any run: command; the new job's commands are entirely static.

🤖 Generated with Claude Code

mrojas54 and others added 2 commits August 11, 2026 06:17
`[tool.ruff.lint.mccabe] max-complexity = 10` in pyproject.toml was inert.
Ruff only evaluates C901 when the rule is selected, and this project keeps
Ruff's default rule set (E4, E7, E9, F), which excludes it -- so `ruff check .`
never read the value. Verified by control: re-running with
`--config "lint.mccabe.max-complexity = 1"` against report.py, which contains a
complexity-20 function, still reports clean.

The budget that actually governs is `complexity_gate.DEFAULT_THRESHOLD`, and
`collect_complexities()` invokes Ruff with `--isolated` precisely so the base
snapshot -- written to a temp dir with no pyproject.toml -- is measured under
the same config as the working tree. The gate therefore *cannot* read
pyproject.toml by design. Two declarations of "10", one of them decorative:
tightening the obvious one would have been a silent no-op.

Remove the inert block and document the real owner from both sides.

test_project_config_and_ci_enforce_the_regression_gate asserted the decorative
key, which is what made the lie load-bearing -- deleting it turned the test red
and told you to put it back. It now asserts the invariant instead: a mccabe
budget may be declared only when C901 is actually selected. That permits either
design -- enforce through Ruff, or leave the gate as sole owner -- and forbids
only the misleading middle state. Confirmed non-vacuous against the old state.

No behaviour change: the gate's threshold is unmoved at 10.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lled

tests/test_observability.py::test_real_sdk_export_is_sanitized is guarded on
`lmnr` being importable, and the `gate` job installs no optional extras -- so
it skipped on every push and pull request while CI reported green. It is the
only test exercising the real Laminar SDK's export sanitization rather than a
fake, and the code it guards took 35 of the last 46 commits, many corrective
(sanitizing SDK-derived context, isolating SDK system exits, preserving
interrupts). The most-churned path had its only real-SDK check dark in CI.

This was invisible locally because a working `.venv` generally has the extra
installed, so the test runs on a developer machine and skips in CI -- the one
arrangement where nobody sees the skip.

Add a second `tracing` job rather than adding the extra to `gate`: the default
install is 13 packages with zero required runtime dependencies, which is the
repo's strongest supply-chain property and worth keeping intact. The new lane
is the single place the opt-in is actually taken.

The lane asserts `lmnr` is importable before running pytest, so a rename or a
lock change fails loudly instead of quietly restoring the same green skips.

Verified both lanes: default install (no lmnr) 747 passed / 4 skipped; with the
extra 748 passed / 3 skipped and test_observability.py:99 absent from the skip
list -- the real-SDK test runs. mypy --strict is clean under the extra too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mrojas54
mrojas54 merged commit e49f01b into main Aug 11, 2026
2 checks passed
@mrojas54
mrojas54 deleted the ai-ccd-cc/complexity-config-tracing-ci branch August 11, 2026 10:58
cursor Bot pushed a commit that referenced this pull request Aug 14, 2026
Take shipped stamp-invalidation docs (#102) and suite 748; keep the
verified no-logfire S20 wording from this branch; retarget the complexity
budget to complexity_gate.DEFAULT_THRESHOLD after #112.

Co-authored-by: Michelle Rojas <mrojas54@users.noreply.github.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.

1 participant