Skip to content

Commit f3d163d

Browse files
mattgodboltclaude
andcommitted
Add comprehensive ruff configuration and fix code quality issues
- Add modern Python linting rules to pyproject.toml covering style, performance, and best practices - Fix pre-commit config to use ruff-check instead of deprecated ruff alias - Increase line length to 120 for better readability - Fix nested if statements for better code clarity - Break long system prompt into readable multi-line format - Add missing docstrings to metrics methods - Fix import sorting and organization - Maintain API field camelCase compatibility with targeted ignores 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 98b55fd commit f3d163d

File tree

9 files changed

+126
-70
lines changed

9 files changed

+126
-70
lines changed

.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ repos:
1414
- repo: https://github.com/astral-sh/ruff-pre-commit
1515
rev: v0.11.10
1616
hooks:
17-
- id: ruff
17+
- id: ruff-check
1818
args: [ --fix ]
1919
- id: ruff-format
2020
- repo: https://github.com/jumanjihouse/pre-commit-hooks

CLAUDE.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,7 @@ Uses AWS Embedded Metrics for CloudWatch integration when deployed, with a no-op
8989
## Code Style Guidelines
9090

9191
- Prefer using modern Python 3.9+ type syntax. Good: `a: list[str] | None`. Bad: `a: Optional[List[str]]`
92+
93+
## Development Workflow Notes
94+
95+
- The pre commit hooks may modify the code and so: always run them before `git add`, and if a commit hook fails then it's probably you'll need to `git add` again if it indicated if fixed issues (e.g. `ruff`)

app/config.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33

44
class Settings(BaseSettings):
5+
"""Application-level settings."""
6+
57
anthropic_api_key: str
68
root_path: str = ""
79
model_config = SettingsConfigDict(env_file=".env")

app/explain.py

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
import logging
33

44
from anthropic import Anthropic
5-
from app.explain_api import ExplainRequest, ExplainResponse, TokenUsage, CostBreakdown
6-
from app.metrics import MetricsProvider
75

6+
from app.explain_api import CostBreakdown, ExplainRequest, ExplainResponse, TokenUsage
7+
from app.metrics import MetricsProvider
88

99
# Configure logging
1010
LOGGER = logging.getLogger("explain")
@@ -53,12 +53,13 @@ def select_important_assembly(
5353
continue
5454

5555
# Source mapping makes this important
56-
if asm_item.get("source") and asm_item["source"] is not None:
57-
if (
58-
isinstance(asm_item["source"], dict)
59-
and asm_item["source"].get("line") is not None
60-
):
61-
important_indices.add(idx)
56+
if (
57+
asm_item.get("source")
58+
and asm_item["source"] is not None
59+
and isinstance(asm_item["source"], dict)
60+
and asm_item["source"].get("line") is not None
61+
):
62+
important_indices.add(idx)
6263

6364
# Function returns and epilogues are important
6465
text = asm_item.get("text", "").strip()
@@ -80,7 +81,7 @@ def select_important_assembly(
8081
# If we still have too many lines, prioritize
8182
if len(all_indices) > max_lines:
8283
# Prioritize function boundaries and source mappings over context
83-
important_indices_list = sorted(list(important_indices))
84+
important_indices_list = sorted(important_indices)
8485
all_indices = set(important_indices_list[:max_lines])
8586

8687
# Collect selected assembly items
@@ -137,9 +138,7 @@ def prepare_structured_data(body: ExplainRequest) -> dict:
137138

138139
if len(asm_dicts) > MAX_ASSEMBLY_LINES:
139140
# If assembly is too large, we need smart truncation
140-
structured_data["assembly"] = select_important_assembly(
141-
asm_dicts, body.labelDefinitions or {}
142-
)
141+
structured_data["assembly"] = select_important_assembly(asm_dicts, body.labelDefinitions or {})
143142
structured_data["truncated"] = True
144143
structured_data["originalLength"] = len(asm_dicts)
145144
else:
@@ -180,15 +179,20 @@ def process_request(
180179

181180
structured_data = prepare_structured_data(body)
182181

183-
# TODO: consider not baking the language and arch here for system prompt caching later on. We'll need to hit minimum token lengths.
184-
system_prompt = f"""You are an expert in {arch} assembly code and {language}, helping users of the Compiler Explorer website understand how their code compiles to assembly.
185-
The request will be in the form of a JSON document, which explains a source program and how it was compiled, and the resulting assembly code that was generated.
182+
# TODO: consider not baking the language and arch here for system prompt caching later on.
183+
# We'll need to hit minimum token lengths.
184+
system_prompt = f"""You are an expert in {arch} assembly code and {language}, helping users of the
185+
Compiler Explorer website understand how their code compiles to assembly.
186+
The request will be in the form of a JSON document, which explains a source program and how it was compiled,
187+
and the resulting assembly code that was generated.
186188
Provide clear, concise explanations. Focus on key transformations, optimizations, and important assembly patterns.
187189
Explanations should be educational and highlight why certain code constructs generate specific assembly instructions.
188-
Give no commentary on the original source: it is expected the user already understands their input, and is only looking for guidance on the assembly output.
190+
Give no commentary on the original source: it is expected the user already understands their input, and is only
191+
looking for guidance on the assembly output.
189192
If it makes it easiest to explain, note the corresponding parts of the source code, but do not focus on this.
190193
Do not give an overall conclusion.
191-
Be precise and accurate about CPU features and optimizations - avoid making incorrect claims about branch prediction or other hardware details."""
194+
Be precise and accurate about CPU features and optimizations - avoid making incorrect claims about branch
195+
prediction or other hardware details."""
192196

193197
# Call Claude API with JSON structure
194198
LOGGER.info(f"Using Anthropic client with model: {MODEL}")

app/explain_api.py

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
"""
2-
Pydantic models for the Claude Explain API.
1+
"""Pydantic models for the Claude Explain API.
32

43
Defines request and response types based on the API specification
54
in claude_explain.md.
@@ -42,20 +41,12 @@ class ExplainRequest(BaseModel):
4241
"""Request body for the /explain endpoint."""
4342

4443
language: str = Field(..., description="Programming language (e.g., 'c++', 'rust')")
45-
compiler: str = Field(
46-
..., description="Compiler identifier (e.g., 'g112', 'clang1500')"
47-
)
44+
compiler: str = Field(..., description="Compiler identifier (e.g., 'g112', 'clang1500')")
4845
code: str = Field(..., description="Original source code")
49-
compilationOptions: list[str] = Field(
50-
default_factory=list, description="Array of compiler flags/options"
51-
)
52-
instructionSet: str | None = Field(
53-
None, description="Target architecture (e.g., 'amd64', 'arm64')"
54-
)
46+
compilationOptions: list[str] = Field(default_factory=list, description="Array of compiler flags/options")
47+
instructionSet: str | None = Field(None, description="Target architecture (e.g., 'amd64', 'arm64')")
5548
asm: list[AssemblyItem] = Field(..., description="Array of assembly objects")
56-
labelDefinitions: dict[str, int] | None = Field(
57-
None, description="Optional map of label names to line numbers"
58-
)
49+
labelDefinitions: dict[str, int] | None = Field(None, description="Optional map of label names to line numbers")
5950

6051

6152
class TokenUsage(BaseModel):
@@ -79,9 +70,7 @@ class ExplainResponse(BaseModel):
7970

8071
explanation: str | None = Field(None, description="The generated explanation")
8172
status: str = Field(..., description="'success' or 'error'")
82-
message: str | None = Field(
83-
None, description="Error message (only present on error)"
84-
)
73+
message: str | None = Field(None, description="Error message (only present on error)")
8574
model: str | None = Field(None, description="The Claude model used")
8675
usage: TokenUsage | None = Field(None, description="Token usage information")
8776
cost: CostBreakdown | None = Field(None, description="Cost breakdown")
@@ -97,9 +86,7 @@ class ExplainErrorResponse(BaseModel):
9786
class ExplainSuccessResponse(BaseModel):
9887
"""Success response from the /explain endpoint."""
9988

100-
status: str = Field(
101-
"success", description="Always 'success' for successful responses"
102-
)
89+
status: str = Field("success", description="Always 'success' for successful responses")
10390
explanation: str = Field(..., description="The generated explanation")
10491
model: str = Field(..., description="The Claude model used")
10592
usage: TokenUsage = Field(..., description="Token usage information")

app/explain_test.py

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
import json
22
from unittest.mock import MagicMock
3+
34
import pytest
45

56
from app.explain import (
6-
process_request,
7-
select_important_assembly,
8-
prepare_structured_data,
97
MAX_ASSEMBLY_LINES,
10-
MODEL,
118
MAX_TOKENS,
9+
MODEL,
10+
prepare_structured_data,
11+
process_request,
12+
select_important_assembly,
1213
)
1314
from app.explain_api import (
14-
ExplainRequest,
1515
AssemblyItem,
16+
ExplainRequest,
1617
SourceMapping,
1718
)
1819
from app.metrics import NoopMetricsProvider
@@ -82,18 +83,13 @@ def noop_metrics():
8283
class TestProcessRequest:
8384
"""Test the main process_request function."""
8485

85-
def test_process_request_success(
86-
self, sample_request, mock_anthropic_client, noop_metrics
87-
):
86+
def test_process_request_success(self, sample_request, mock_anthropic_client, noop_metrics):
8887
"""Test successful processing of a request."""
8988
response = process_request(sample_request, mock_anthropic_client, noop_metrics)
9089

9190
# Verify response structure
9291
assert response.status == "success"
93-
assert (
94-
response.explanation
95-
== "This assembly code implements a simple square function..."
96-
)
92+
assert response.explanation == "This assembly code implements a simple square function..."
9793
assert response.model == MODEL
9894

9995
# Check usage information
@@ -145,9 +141,7 @@ def test_process_request_success(
145141
structured_data = json.loads(messages[0]["content"][1]["text"])
146142
assert structured_data["language"] == "c++"
147143
assert structured_data["compiler"] == "g++"
148-
assert (
149-
structured_data["sourceCode"] == "int square(int x) {\n return x * x;\n}"
150-
)
144+
assert structured_data["sourceCode"] == "int square(int x) {\n return x * x;\n}"
151145

152146

153147
class TestSelectImportantAssembly:
@@ -206,13 +200,14 @@ def test_select_important_assembly_over_limit(self):
206200
# Check that important lines (with sources) are included
207201
has_source_lines = False
208202
for line in result:
209-
if "isOmissionMarker" not in line and line.get("source") is not None:
210-
if (
211-
isinstance(line["source"], dict)
212-
and line["source"].get("line") is not None
213-
):
214-
has_source_lines = True
215-
break
203+
if (
204+
"isOmissionMarker" not in line
205+
and line.get("source") is not None
206+
and isinstance(line["source"], dict)
207+
and line["source"].get("line") is not None
208+
):
209+
has_source_lines = True
210+
break
216211
assert has_source_lines
217212

218213
def test_select_important_assembly_preserves_function_boundaries(self):

app/main.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import logging
22

3-
from anthropic import Anthropic, __version__ as AnthropicVersion
3+
import aws_embedded_metrics
4+
from anthropic import Anthropic
5+
from anthropic import __version__ as anthropic_version
6+
from fastapi import FastAPI
7+
from fastapi.middleware.cors import CORSMiddleware
8+
from mangum import Mangum
9+
410
from app.config import settings
511
from app.explain import process_request
612
from app.explain_api import ExplainRequest, ExplainResponse
713
from app.metrics import NoopMetricsProvider
8-
from fastapi import FastAPI
9-
from fastapi.middleware.cors import CORSMiddleware
10-
from mangum import Mangum
11-
import aws_embedded_metrics
1214

1315
logging.basicConfig(level=logging.INFO)
1416
logger = logging.getLogger()
@@ -28,7 +30,7 @@
2830
handler = Mangum(app)
2931

3032
anthropic_client = Anthropic(api_key=settings.anthropic_api_key)
31-
logger.info(f"Anthropic SDK version: {AnthropicVersion}")
33+
logger.info(f"Anthropic SDK version: {anthropic_version}")
3234

3335
metrics_provider = NoopMetricsProvider()
3436
# if metrics:
@@ -39,10 +41,12 @@
3941

4042
@app.get("/")
4143
async def root() -> str:
44+
"""Temporary placeholder for a better API."""
4245
return "Hello, world!"
4346

4447

4548
@aws_embedded_metrics.metric_scope
4649
@app.post("/")
4750
async def explain(request: ExplainRequest) -> ExplainResponse:
51+
"""Explain a Compiler Explorer compilation from its source and output assembly."""
4852
return process_request(request, anthropic_client, metrics_provider)

app/metrics.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,10 @@ class MetricsProvider(ABC):
99
@abstractmethod
1010
def put_metric(self, name: str, value: int | float) -> None:
1111
"""Record a metric with the given name and value."""
12-
pass
1312

1413
@abstractmethod
1514
def set_property(self, name: str, value: str) -> None:
1615
"""Set a property/dimension for metrics."""
17-
pass
1816

1917

2018
class CloudWatchMetricsProvider(MetricsProvider):
@@ -24,17 +22,19 @@ def __init__(self, metrics_logger: MetricsLogger):
2422
self.metrics = metrics_logger
2523

2624
def put_metric(self, name: str, value: int | float) -> None:
25+
"""Record a metric with the given name and value in CloudWatch."""
2726
self.metrics.put_metric(name, value)
2827

2928
def set_property(self, name: str, value: str) -> None:
29+
"""Set a property/dimension for metrics."""
3030
self.metrics.set_property(name, value)
3131

3232

3333
class NoopMetricsProvider(MetricsProvider):
3434
"""Metrics provider that does nothing - for testing."""
3535

3636
def put_metric(self, name: str, value: int | float) -> None:
37-
pass
37+
"""Does nothing."""
3838

3939
def set_property(self, name: str, value: str) -> None:
40-
pass
40+
"""Does nothing."""

pyproject.toml

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,63 @@ dev = [
2020
"pre-commit>=4.2.0",
2121
"pytest>=8.3.5",
2222
]
23+
24+
[tool.ruff]
25+
target-version = "py313"
26+
line-length = 120
27+
28+
[tool.ruff.lint]
29+
select = [
30+
# Pyflakes
31+
"F",
32+
# pycodestyle
33+
"E",
34+
"W",
35+
# isort
36+
"I",
37+
# pep8-naming
38+
"N",
39+
# pydocstyle
40+
"D",
41+
# pyupgrade
42+
"UP",
43+
# flake8-bugbear
44+
"B",
45+
# flake8-simplify
46+
"SIM",
47+
# flake8-comprehensions
48+
"C4",
49+
# flake8-pie
50+
"PIE",
51+
# flake8-return
52+
"RET",
53+
# flake8-unused-arguments
54+
"ARG",
55+
# flake8-use-pathlib
56+
"PTH",
57+
# Ruff-specific rules
58+
"RUF",
59+
]
60+
61+
ignore = [
62+
# Allow non-lowercase variable names (e.g., for API field names)
63+
"N806",
64+
# Allow missing docstrings for some cases
65+
"D100", # Missing docstring in public module
66+
"D104", # Missing docstring in public package
67+
"D107", # Missing docstring in __init__
68+
# Allow imperative mood in docstrings (e.g., "Create a new user")
69+
"D401",
70+
]
71+
72+
[tool.ruff.lint.pydocstyle]
73+
convention = "google"
74+
75+
[tool.ruff.lint.per-file-ignores]
76+
"test*.py" = [
77+
"D", # Don't require docstrings in tests
78+
"ARG", # Allow unused arguments in tests (fixtures)
79+
]
80+
"app/explain_api.py" = [
81+
"N815", # Allow camelCase field names for API compatibility
82+
]

0 commit comments

Comments
 (0)