Skip to content

style(skills): apply ruff format and wire skills/ into workspace checks - #1077

Open
Arnxvvv wants to merge 3 commits into
apache:mainfrom
Arnxvvv:style/skills-ruff-format
Open

style(skills): apply ruff format and wire skills/ into workspace checks#1077
Arnxvvv wants to merge 3 commits into
apache:mainfrom
Arnxvvv:style/skills-ruff-format

Conversation

@Arnxvvv

@Arnxvvv Arnxvvv commented Aug 10, 2026

Copy link
Copy Markdown

Summary

  • Adopts skills/ as a workspace member (skills/pyproject.toml added and declared in root [tool.uv.workspace] members), bringing helper scripts and guards under standard CI linting, type-checking, and testing.
  • Applies ruff format across all Python code under skills/, reflowing long set literals, regex constants, and multiline argument lists to comply with the 110-character line-length standard without altering runtime logic.
  • Removes temporary skip: deleted the [tool.magpie.checks] skip = ["ruff-format"] block so that run-workspace-check.sh and pre-commit hooks run the formatter check automatically across skills/.

Type of change

  • Skill change (.claude/skills/<name>/) — eval fixtures updated below
  • Tool / bridge contract (tools/<system>/*.md)
  • Python package (tools/*/ with pyproject.toml)
  • Groovy reference impl
  • Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
  • Documentation (docs/, README.md, CONTRIBUTING.md)
  • Project template (projects/_template/)
  • CI / dev loop (prek, workflows, validators)
  • Other:

Test plan

  • For Python packages touched: uv run pytest / ruff check / mypy passes:
    • uv run --directory skills --project . ruff format --check . (9 files verified formatted)
    • uv run --directory skills --project . ruff check . (all lint rules passed clean)
    • uv run --directory skills --project . python -m mypy . (clean type-check across 9 source files)
    • uv run --directory skills --project . python -m pytest (9 passed in 0.14s)
  • uv lock --check passes with magpie-skills v0.1.0 member resolved.

RFC-AI-0004 compliance

  • HITL — any new mutation is gated on explicit user confirmation
  • Sandbox — no new unrestricted host access; network reach declared in the adapter
  • Vendor neutrality — placeholders (<PROJECT>, <tracker>, <upstream>, <security-list>) used in all skill / tool prose (the check-placeholders prek hook is the mechanical gate)
  • Conversational + correctable — agentic-override path documented if behaviour is adopter-tunable
  • Write-access discipline — no autonomous outbound messages; drafts only, sent on confirmation
  • Privacy LLM — private content does not reach a non-approved LLM; redactor invoked where needed

Linked issues

Notes for reviewers (optional)

  • Includes the foundational infrastructure from PR ci(skills): lint, type-check and test the Python under skills/ #1054 (skills/pyproject.toml, workspace member registration, uv.lock update, and type annotations in scan_ci_runners.py / collect_status.py) along with the mechanical ruff format changes so the branch is completely self-contained and ready for immediate CI testing.

Arnxvvv and others added 3 commits August 10, 2026 09:53
Applies the infrastructure from apache#1054 (create skills/pyproject.toml as a workspace member with ruff, mypy, and pytest config; fix type-checking issues in scan_ci_runners.py and collect_status.py) and then runs ruff format on all Python under skills/.

The formatter reflowed 9 files — long set literals, regex constants, and argument lists wrapped to respect the 110-char line limit. No logic moves.

The temporary [tool.magpie.checks] skip = [ruff-format] block is removed now that formatting has landed. Closes apache#1076.
`ruff format` does not touch `.md`, so the four hunks in
`contributor-nomination/fetch.md`, `contributor-sentiment/SKILL.md`, and
`issue-reproducer/verification.md` were hand edits rather than formatter
output, and the stated verification (`ruff format --check .` over the
nine Python files) did not cover them.

One was a regression: the Python "Bad:" example in `verification.md`
column-aligns its comment to match the Groovy example immediately below,
so reformatting only the Python left the pair visibly inconsistent.

Reverted all three to `main`. The Python reflow — the actual point of the
change — is untouched, and `ruff format (workspace)` now passes with the
`[tool.magpie.checks]` skip removed.

Co-authored-by: Jarek Potiuk <potiuk@apache.org>
Generated-by: Claude Code (Opus 5)

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving — with an explanation of what I changed and how this PR now
relates to #1054, because the two overlapped substantially.

What the overlap was

This branch did not sit on top of #1054; it contained a copy of it.
Specifically, skills/pyproject.toml here was #1054's 113-line file with
exactly its lines 51–60 removed — the [tool.magpie.checks]
skip = ["ruff-format"] block and the comment tracking it as #1076.
The root pyproject.toml member registration, the uv.lock delta, and
the type-annotation fixes in scan_ci_runners.py were all #1054's too.
Two open PRs adding the same file means whichever merged second would
have conflicted.

Rather than ask you to rebase away work you had correctly identified as
prerequisite, we have combined the two. This PR is now the single
change, and #1054 is closed as superseded. Both authors are credited as
co-authors on the merge.

Before combining I verified the two were genuinely compatible rather than
assuming it: I took #1054's tree, deleted its ruff-format skip, ran
uv run --directory skills --project . ruff format ., and diffed every
skills/**/*.py against this branch. All of them are byte-identical.
So this branch's Python is exactly #1054's work plus a pure mechanical
reflow — nothing of #1054 was dropped, and nothing extra was introduced.

The one thing I fixed

Four hunks reformatted Python code blocks inside three markdown files:
contributor-nomination/fetch.md, contributor-sentiment/SKILL.md, and
issue-reproducer/verification.md.

ruff format does not touch .md. The test plan's evidence was
ruff format --check . over nine files, all Python, so those four hunks
were hand edits presented as formatter output with nothing verifying
them. And one was a small regression — the Python "Bad:" example in
verification.md deliberately column-aligns its comment to line up with
the Groovy example immediately below it:

if "xs" in output:                       # matches xsi too
if (output.contains("foo")) {            // matches foo-bar, foobar

Reformatting only the Python side left the pair visibly mismatched.
I have reverted all three files to main; the Python reflow, which is
the actual substance of your contribution, is untouched.

Verification on the combined branch

  • prek run --from-ref main fully green — 23 hooks, including
    lychee, markdownlint, check-workspace-members, mypy, pytest,
    and skill-and-tool-validate.
  • ruff format (workspace) passes, which is the whole point: it
    confirms the temporary skip #1054 introduced is genuinely no longer
    needed and could be deleted rather than merely suppressed.
  • Net change against main is now exactly nine files — one member
    registration line, the 101-line config carrier, six reflowed Python
    files, and the lockfile. No markdown.

Note for next time

Where a PR depends on another open PR, branch from that PR's head and
mention the dependency, rather than copying its content in. The result is
reviewable as a stack, and neither PR blocks the other. Nothing was lost
here, but it took a file-by-file comparison to establish that — which is
work a stacked branch makes unnecessary.

Thanks for finishing the formatting pass. Turning a documented temporary
exemption into a deleted one, with the checker actually enforcing it, is
exactly the right follow-up and it closes #1076 properly.


This review was drafted by an AI-assisted tool and
confirmed by a Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Magpie handles maintainer review:
CONTRIBUTING.md.

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.

ci(skills): remove the ruff-format skip once the formatting churn lands

2 participants