Skip to content

fix(ci): pin ruff and declare the lint rule set explicitly - #61

Merged
kavya-chennoju merged 2 commits into
mainfrom
fix/ci-pin-ruff
Aug 17, 2026
Merged

fix(ci): pin ruff and declare the lint rule set explicitly#61
kavya-chennoju merged 2 commits into
mainfrom
fix/ci-pin-ruff

Conversation

@atsyplikhin

Copy link
Copy Markdown
Collaborator

Symptom

lint has failed on every CI run on main since 2026-07-24, including the nightly scheduled runs. It reports 704 findings on a tree nobody changed. Every PR inherits the red check.

Cause

The lint job ran pip install ruff, unpinned. ruff 0.16.0 was published 2026-07-23 at 19:10 UTC and widened the built-in default rule set. The 2026-07-23 06:45 scheduled run got ruff 0.15.22 and was green; the 2026-07-24 06:45 run got 0.16.0 and was red. That is the entire delta, no source change involved.

The new findings are all style families this tree was never linted against:

rule count
UP045 non-pep604-annotation-optional 207
UP006 non-pep585-annotation 196
I001 unsorted-imports 94
UP035 deprecated-import 51
BLE001 blind-except 48
PIE790 unnecessary-placeholder 39

The underlying fragility is that ruff.toml set only line-length, so the rule set was inherited from ruff's built-in default. That default is not a stable contract, which makes lint results a function of release timing rather than of the tree.

Fix

  1. ruff.toml declares the rule set explicitly: select = ["E4", "E7", "E9", "F"], exactly what ruff selected by default through 0.15.x. This preserves the rules the tree was written against and makes future ruff upgrades a no-op for lint.
  2. ci.yml pins the version through a RUFF_VERSION env var (0.16.1, current latest), so a linter bump becomes a reviewable commit rather than a surprise on a Friday.

Belt and braces on purpose: the pin makes today reproducible, the explicit select makes the next bump safe.

Verification

Ran the lint job's four commands locally against three ruff versions straddling the break:

ruff result
0.15.22 (last good) All checks passed
0.16.0 (the break) All checks passed
0.16.1 (pinned) All checks passed

Nothing previously caught is now suppressed. The 12 F821 mentions in the failing log were pre-existing # noqa: F821 comments quoted inside UP045 fix diffs, not new findings; --select F821 is clean tree-wide.

Follow-up, deliberately not here

Adopting the wider 0.16 rule set is worth doing. It is a ~700-finding autofix sweep touching every package, would conflict with anything in flight, and the UP006/UP045 annotation rewrites want their own review pass on the pydantic models. That belongs in its own PR, once main is green again.

The lint job installed ruff unpinned, so CI tracked whatever ruff published.
ruff 0.16.0 (released 2026-07-23 19:10 UTC) widened the built-in default rule
set, and the next scheduled run on main went red on an unchanged tree: 704
findings, all style families the repo had never selected (UP045, UP006, I001,
BLE001, PIE790). main has been red on every run since 2026-07-24; the last
green run, 2026-07-23, was the last one to get ruff 0.15.x.

Two changes, both needed:

* ruff.toml declares select = ["E4", "E7", "E9", "F"] -- exactly what ruff
  selected by default through 0.15.x. The built-in default is not a stable
  contract, so inheriting it makes lint results a function of release timing
  rather than of the tree.
* ci.yml pins the version via a RUFF_VERSION env var, so a linter bump is a
  reviewable commit instead of a surprise.

Verified green on all four linted paths under 0.15.22 (last good), 0.16.0
(the break), and 0.16.1 (pinned). Nothing is suppressed that was previously
caught: the F821 hits in the failing log were existing "# noqa: F821"
comments quoted inside UP045 fix diffs, and --select F821 is clean tree-wide.

Adopting the wider 0.16 rule set is worth doing, but it is a ~700-finding
autofix sweep touching every package and belongs in its own PR.
## What

One line: `model_config = {"extra": "allow"}` on `DeviceStatus`,
matching the policy `DeviceIdentity` already has.

## Why

`DeviceStatus` declared no `extra` policy, so pydantic's default
silently dropped unknown keys. That makes the two halves of a device
record behave differently: identity accepts device-specific metadata,
status does not.

It also puts the typed and untyped paths out of step.
`DeviceRegistry.update_status()` merges heartbeat payloads into the
record as raw dicts with no schema validation, so deployment-specific
runtime state written over the heartbeat subject already lands in
`status` and is readable by agents. The only place it vanished was the
typed path: a driver returning it from `DeviceDriver.status()`, or an
explicit `DeviceStatus` passed at registration, where
`status.model_dump(exclude_none=True)` (device.py) never saw the field.
No error, no log line, just a missing key.

The motivating case is an authoritative network-side location provider
writing an egocentric awareness object ("halo": the tracked entities
around a node) onto device records. This PR carries none of that
vocabulary into core, and takes no position on the naming or on
selector-side exposure. It only makes the extension supported instead of
accidental.

## Scope

- No new fields, no behaviour change for any existing device.
- Declared-field validation is unchanged: `busy_score=1.5` still raises,
covered by a new test.
- Nested dicts and lists roundtrip through `model_dump(mode="json")` and
back.

## Tests

Three tests added to `TestDeviceStatus` in
`packages/device-connect-edge/tests/test_types.py`: extras accepted and
readable, extras surviving a dump/reload roundtrip, and a guard that
declared-field validation still applies.

- edge suite: 568 passed
- agent-tools suite: 269 passed. The 12 errors in `test_integration.py`
are Zenoh broker connection failures at fixture setup
(`tcp/localhost:7447`, needs the Docker compose stack); they reproduce
identically on unmodified `main` in the same environment.
@atsyplikhin

Copy link
Copy Markdown
Collaborator Author

Hi @kavya-chennoju, would you have a moment for this one? It is small, 54 lines added and 2 removed across 4 files.

It fixes the lint job, which has been red on every CI run on main since 2026-07-24, including the nightly scheduled ones, so every open PR currently inherits a red check. The cause was an unpinned pip install ruff: ruff 0.16.0 published on 2026-07-23 and widened the default rule set, so the tree started reporting 704 findings with no source change. The fix pins ruff and declares the rule set explicitly rather than inheriting whatever the latest release defaults to.

CI is green and it is mergeable. Thank you.

@atsyplikhin

Copy link
Copy Markdown
Collaborator Author

Hi @kavya-chennoju, gently resurfacing this one because the cost is still accruing: the scheduled CI run on main failed again this morning, as it has every day this week, and every open PR in the repo inherits that red check until this lands.

Nothing has changed on the branch since I first asked: 4 files, 54 lines added, pins ruff and declares the lint rule set explicitly so a new ruff release cannot widen the defaults under us again. Green and mergeable.

If it is easier to hand this to someone else, that is completely fine too, just say the word. Thank you.

@kavya-chennoju kavya-chennoju left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving. Verified the diagnosis and the fix independently:

  • The break is real and ongoing: lint is the only failing job on main, red on 12 consecutive scheduled runs (2026-07-31 through this morning's 06:06 UTC run).
  • The cause checks out. Ruff's 0.16.0 changelog confirms the default rule set went from 59 to 413 rules, so an unpinned pip install ruff flipped the tree red with no commit behind it.
  • select = ["E4", "E7", "E9", "F"] restores the pre-0.16 default exactly, so no coverage is lost. Worth noting it is also stricter than plain 0.16 defaults on the 18 rules 0.16.0 dropped from the default set (E401, E402, E7xx, F403/F405/F406/F722) - all still covered by the E4/E7/F selectors here. Nothing previously caught is suppressed.
  • ci.yml is the only place in the repo that invokes ruff, and there is no pre-commit config, so the pin has no second source to drift against.

All 9 checks green. Agreed that the wider 0.16 rule set belongs in its own sweep.

Note for anyone reading the description later: this branch also carries #60 (DeviceStatus extra: allow), which was stacked on this branch and merged into it, so this merge lands both. That one was reviewed on its own PR.

@kavya-chennoju
kavya-chennoju merged commit c1c6e7a into main Aug 17, 2026
9 checks passed
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