From 5b67f5b66cef0f279c710b129a6416f7891fdcc6 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 11 Aug 2026 17:37:33 +0200 Subject: [PATCH] fix: clear the ruff findings that needed a decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 21 the automatic pass could not make on its own. `ruff check` and `ruff format --check` are both clean now; typecheck is still red and is next. `in_reasoning` in chat/service.py was a complete state machine that nothing read: initialised False, set True when a reasoning delta arrived, set False when the summary ended — three assignments, zero reads. Ruff reported one at a time, and removing each revealed the next, so what looked like a single stray variable took three passes to bottom out. The branches themselves do real work and are untouched; only the flag is gone. Four `raise HTTPException` inside `except` blocks now chain with `from e`. Until now a failure while handling an error was indistinguishable from the error, which matters most in exactly the situation where the traceback is all you have. In biographer/tools.py the binding was unused but the call is not: MemoryType() is called for the ValueError it raises on an invalid name. The binding is gone and the call and its comment stay, because dropping the line would have removed the validation. The rest are unused bindings in tests where the assertions are on something else (call_args, mostly), plus three unused loop variables and an isinstance tuple. One correction to my own work: removing a dead comprehension in test_error_handling.py left an `if` block with nothing but comments in it, which is a SyntaxError. Ruff caught it immediately. The block now says what the test actually pins — that the stream parses without crashing, which reaching that line demonstrates — rather than computing a list nobody asserts on. `make test` is intermittent here, and it is not this change. test_tatlock_tool_call_logging_calculator failed in two of five full runs across both HEAD and this branch, and passes in the other three; it also fails in isolation at HEAD while passing in isolation here. Order- or timing-dependent. Recorded rather than chased, since tests are not gated in this repo yet. Co-Authored-By: Claude --- src/agents/biographer/tools.py | 2 +- src/chat/service.py | 4 +--- src/core/tracing_router.py | 2 +- src/responses/router.py | 6 +++--- tests/agents/test_lorem_tester.py | 6 +++--- tests/core/test_logging_config.py | 4 ++-- tests/core/test_memory_service.py | 8 ++++---- tests/e2e/test_orchestration_e2e.py | 6 ++---- tests/integration/test_tatlock_streaming.py | 2 +- tests/responses/test_advanced_features.py | 2 +- tests/responses/test_error_handling.py | 6 ++++-- tests/responses/test_history.py | 2 +- 12 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/agents/biographer/tools.py b/src/agents/biographer/tools.py index 642555d..960f390 100644 --- a/src/agents/biographer/tools.py +++ b/src/agents/biographer/tools.py @@ -325,7 +325,7 @@ async def list_memories( # Convert string to MemoryType try: - mem_type = MemoryType(memory_type) + MemoryType(memory_type) # validated for its ValueError; the value is unused except ValueError: return f"Invalid memory type '{memory_type}'. Use: user_profile, preference, or learned_fact" diff --git a/src/chat/service.py b/src/chat/service.py index bfec135..a7ef2b6 100644 --- a/src/chat/service.py +++ b/src/chat/service.py @@ -161,7 +161,6 @@ async def create_chat_completion_stream( # Stream from Responses API coordinator = StreamingCoordinator() - in_reasoning = False if use_steward: stream_generator = coordinator.stream_response_with_steward(response_request) @@ -172,7 +171,6 @@ async def create_chat_completion_stream( if event.event == StreamEventType.REASONING_SUMMARY_DELTA: # Stream reasoning via reasoning_content field (DeepSeek R1 format) # Open WebUI renders this as collapsible thinking block - in_reasoning = True yield ChatCompletionChunk( id=completion_id, object=constants.CHAT_COMPLETION_CHUNK_OBJECT, @@ -189,7 +187,7 @@ async def create_chat_completion_stream( elif event.event == StreamEventType.REASONING_SUMMARY_DONE: # Signal end of reasoning block (no content needed) - in_reasoning = False + pass # nothing downstream reads this; the event just ends the block elif event.event == StreamEventType.OUTPUT_TEXT_DELTA: # Stream message content diff --git a/src/core/tracing_router.py b/src/core/tracing_router.py index 5fbc1c0..bffb00a 100644 --- a/src/core/tracing_router.py +++ b/src/core/tracing_router.py @@ -155,4 +155,4 @@ async def get_trace(trace_id: str): return JSONResponse(content=data) except Exception as e: logger.error("trace_read_error", trace_id=trace_id, error=str(e)) - raise HTTPException(status_code=500, detail="Failed to read trace") + raise HTTPException(status_code=500, detail="Failed to read trace") from e diff --git a/src/responses/router.py b/src/responses/router.py index e6570fa..bb2c8a0 100644 --- a/src/responses/router.py +++ b/src/responses/router.py @@ -73,12 +73,12 @@ async def create_response( except ModelNotFoundError as e: logger.error(f"Model not found: {e}") - raise HTTPException(status_code=404, detail=str(e)) + raise HTTPException(status_code=404, detail=str(e)) from e except AppException as e: logger.error(f"Application error: {e}") - raise HTTPException(status_code=e.status_code, detail=e.message) + raise HTTPException(status_code=e.status_code, detail=e.message) from e except Exception as e: logger.error(f"Unexpected error: {e}", exc_info=True) - raise HTTPException(status_code=500, detail="Internal server error") + raise HTTPException(status_code=500, detail="Internal server error") from e diff --git a/tests/agents/test_lorem_tester.py b/tests/agents/test_lorem_tester.py index 261936a..a8c2317 100644 --- a/tests/agents/test_lorem_tester.py +++ b/tests/agents/test_lorem_tester.py @@ -136,7 +136,7 @@ async def test_lorem_tester_rate_limit_trigger(): messages = [{"role": "user", "content": "trigger_rate_limit"}] with pytest.raises(RateLimitError) as exc_info: - async for item in agent.generate_response(messages): + async for _item in agent.generate_response(messages): pass assert "rate limit" in str(exc_info.value).lower() @@ -151,7 +151,7 @@ async def test_lorem_tester_context_overflow_trigger(): messages = [{"role": "user", "content": "trigger_context_overflow"}] with pytest.raises(ContextLengthError) as exc_info: - async for item in agent.generate_response(messages): + async for _item in agent.generate_response(messages): pass assert "context" in str(exc_info.value).lower() @@ -166,7 +166,7 @@ async def test_lorem_tester_invalid_tool_trigger(): messages = [{"role": "user", "content": "trigger_invalid_tool"}] with pytest.raises(APIError) as exc_info: - async for item in agent.generate_response(messages): + async for _item in agent.generate_response(messages): pass assert "tool" in str(exc_info.value).lower() diff --git a/tests/core/test_logging_config.py b/tests/core/test_logging_config.py index 2483360..3d67e7d 100644 --- a/tests/core/test_logging_config.py +++ b/tests/core/test_logging_config.py @@ -75,7 +75,7 @@ class TestLogOperation: @pytest.mark.asyncio async def test_log_operation_success(self): """Test log_operation for successful operation.""" - logger = get_logger("test") + get_logger("test") async with log_operation("test_operation", {"user_id": "123"}) as ctx: # Can update context during operation @@ -89,7 +89,7 @@ class TestLogOperation: @pytest.mark.asyncio async def test_log_operation_failure(self): """Test log_operation for failed operation.""" - logger = get_logger("test") + get_logger("test") with pytest.raises(ValueError): async with log_operation("test_operation") as ctx: diff --git a/tests/core/test_memory_service.py b/tests/core/test_memory_service.py index da07204..271beb1 100644 --- a/tests/core/test_memory_service.py +++ b/tests/core/test_memory_service.py @@ -160,7 +160,7 @@ class TestMemoryServicePreferenceMethods: mock_set.return_value = True with patch("src.core.memory_service.get_user", return_value="testuser"): - result = await service.set_preference("theme", "dark") + await service.set_preference("theme", "dark") call_kwargs = mock_set.call_args[1] assert call_kwargs["importance"] == 0.7 @@ -179,7 +179,7 @@ class TestMemoryServiceFactMethods: mock_set.return_value = True with patch("src.core.memory_service.get_user", return_value="testuser"): - result = await service.store_fact("car", "Tesla Model 3") + await service.store_fact("car", "Tesla Model 3") call_kwargs = mock_set.call_args[1] assert call_kwargs["importance"] == 0.5 @@ -193,7 +193,7 @@ class TestMemoryServiceFactMethods: mock_set.return_value = True with patch("src.core.memory_service.get_user", return_value="testuser"): - result = await service.store_fact( + await service.store_fact( "employer", "Acme Corp", importance=0.8, @@ -254,7 +254,7 @@ class TestMemoryServicePrefetch: mock_prefs.return_value = {} with patch("src.core.memory_service.get_user", return_value="testuser"): - result = await service.prefetch_context( + await service.prefetch_context( profile_keys=["location"], include_preferences=False, ) diff --git a/tests/e2e/test_orchestration_e2e.py b/tests/e2e/test_orchestration_e2e.py index 50aab1c..03b7349 100644 --- a/tests/e2e/test_orchestration_e2e.py +++ b/tests/e2e/test_orchestration_e2e.py @@ -151,7 +151,7 @@ def assert_llm_behavior( Returns: LLMAssertionResult with pass/fail and evidence """ - response_lower = response_text.lower() + response_text.lower() matches = [] unexpected_matches = [] @@ -716,9 +716,7 @@ class TestScenario1WeatherWithMemory: print(f"Weather query - Steward: {reasoning_text[:200]}...") # Should mention location/memory and search capabilities - has_memory_mention = ( - "biographer" in reasoning_text.lower() or "memory" in reasoning_text.lower() - ) + ("biographer" in reasoning_text.lower() or "memory" in reasoning_text.lower()) has_search_mention = ( "tatlock_core" in reasoning_text.lower() or "search" in reasoning_text.lower() ) diff --git a/tests/integration/test_tatlock_streaming.py b/tests/integration/test_tatlock_streaming.py index 52eba10..9cebfa2 100644 --- a/tests/integration/test_tatlock_streaming.py +++ b/tests/integration/test_tatlock_streaming.py @@ -47,7 +47,7 @@ async def test_tatlock_streaming_no_duplication(async_client: AsyncClient): continue if line.startswith("event: "): - event_type = line[7:].strip() + line[7:].strip() elif line.startswith("data: "): data_str = line[6:].strip() if data_str != "[DONE]": diff --git a/tests/responses/test_advanced_features.py b/tests/responses/test_advanced_features.py index 5de876d..fadc028 100644 --- a/tests/responses/test_advanced_features.py +++ b/tests/responses/test_advanced_features.py @@ -300,7 +300,7 @@ async def test_max_tokens_in_streaming(async_client: AsyncClient): continue if line.startswith("event: "): - event_type = line[7:].strip() + line[7:].strip() elif line.startswith("data: "): data_str = line[6:].strip() if data_str != "[DONE]": diff --git a/tests/responses/test_error_handling.py b/tests/responses/test_error_handling.py index 5c29616..f72e067 100644 --- a/tests/responses/test_error_handling.py +++ b/tests/responses/test_error_handling.py @@ -151,8 +151,10 @@ async def test_streaming_rate_limit_error(async_client: AsyncClient) -> None: # Should either have error status or error event if response.status_code == 200: - # Check for error event - error_events = [e for e in events if e["event"] == "error"] + # An error event may or may not appear, depending on where the failure + # occurs. What this test pins is that the stream parses and does not + # crash; arriving here is that assertion. + pass # May or may not have error event depending on where error occurs # At minimum, should not crash diff --git a/tests/responses/test_history.py b/tests/responses/test_history.py index 3a616a0..8ef9605 100644 --- a/tests/responses/test_history.py +++ b/tests/responses/test_history.py @@ -295,7 +295,7 @@ async def test_context_window_usage_stats(): assert stats["max_tokens"] == 1000 assert stats["reserved_tokens"] == 100 assert stats["available_tokens"] == 900 - assert isinstance(stats["usage_percent"], (int, float)) + assert isinstance(stats["usage_percent"], int | float) @pytest.mark.unit