Skip to content

Ruff#2

Merged
lbliii merged 3 commits intomainfrom
ruff
Mar 1, 2026
Merged

Ruff#2
lbliii merged 3 commits intomainfrom
ruff

Conversation

@lbliii
Copy link
Copy Markdown
Owner

@lbliii lbliii commented Mar 1, 2026

No description provided.

Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
@lbliii lbliii self-assigned this Mar 1, 2026
Copilot AI review requested due to automatic review settings March 1, 2026 13:11
@lbliii lbliii merged commit 29bc6f8 into main Mar 1, 2026
8 checks passed
@lbliii lbliii deleted the ruff branch March 1, 2026 13:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates Ruff tooling versions and adds documentation + a new benchmark script to demonstrate parallel highlighting scaling on free-threaded Python (3.14t).

Changes:

  • Bump Ruff versions in pyproject.toml and .pre-commit-config.yaml.
  • Add benchmarks/benchmark_parallel.py to compare sequential highlight() vs parallel highlight_many().
  • Refresh README messaging around free-threading and add instructions + sample output for the new benchmark.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
pyproject.toml Updates Ruff dev dependency and reformats Ruff per-file ignores.
benchmarks/benchmark_parallel.py New benchmark script for parallel highlighting/free-threading visibility.
README.md Updates free-threading bullet + adds benchmark instructions and example table.
.pre-commit-config.yaml Bumps ruff-pre-commit hook revision to match the newer Ruff.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- **O(n) guaranteed** — Hand-written state machines, no regex backtracking
- **Zero ReDoS** — No exploitable patterns, safe for untrusted input
- **Thread-safe** — Immutable state, optimized for Python 3.14t free-threading
- **Free-threading native** — All lexer state is local variables, keyword tables are `frozenset`, tokens are immutable. Highlight from any number of threads with zero contention.
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The README claim of “zero contention” is stronger than what the implementation guarantees. Even if lexer state is local/immutable, the library still uses shared machinery (e.g., registry caching via functools.cache, ThreadPoolExecutor internals), so there can be contention outside the lexer state. Consider rephrasing to something precise like “no shared mutable lexer state” / “safe to call concurrently” rather than “zero contention.”

Suggested change
- **Free-threading native** — All lexer state is local variables, keyword tables are `frozenset`, tokens are immutable. Highlight from any number of threads with zero contention.
- **Free-threading native** — All lexer state is local variables, keyword tables are `frozenset`, tokens are immutable. Safe to call from many threads concurrently with no shared mutable lexer state.

Copilot uses AI. Check for mistakes.

Demonstrates thread scaling when highlighting many code blocks in parallel
under Python 3.14t free-threading. Uses highlight_many() for parallel path
and sequential highlight() loop for baseline. Stdlib only.
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The module docstring says “Stdlib only”, but this script imports project modules (rosettes, benchmarks.sample_code). If the intent is “no third-party deps”, consider updating the wording to avoid being misleading.

Suggested change
and sequential highlight() loop for baseline. Stdlib only.
and sequential highlight() loop for baseline. No third-party dependencies.

Copilot uses AI. Check for mistakes.


def _load_sample_code() -> None:
"""Add repo root to path and load sample_code (for script execution)."""
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

_load_sample_code()’s docstring says it “load[s] sample_code”, but the function only prepends the repo root to sys.path. Consider either importing benchmarks.sample_code inside the function (so it actually “loads” it) or updating the docstring to describe what it really does.

Suggested change
"""Add repo root to path and load sample_code (for script execution)."""
"""Ensure repo root is on sys.path so benchmarks.sample_code can be imported."""

Copilot uses AI. Check for mistakes.
Comment on lines +72 to +77
# GIL detection
gil_enabled = getattr(sys, "_is_gil_enabled", lambda: True)()
if gil_enabled:
print("Free-threaded build: No (GIL enabled)")
print("\nRun with Python 3.14t (free-threading) to see parallel scaling.")
print("Example: python3.14t benchmarks/benchmark_parallel.py")
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

This benchmark duplicates free-threading/GIL detection via sys._is_gil_enabled. Since the project already has rosettes._parallel.is_free_threaded() (with fallback behavior), consider reusing that helper to keep detection logic consistent and avoid depending on a CPython private attribute in two places.

Copilot uses AI. Check for mistakes.
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