Skip to content

Commit 0ad8a96

Browse files
committed
LCORE-1569,LCORE-1570: appease pylint, pyright, and ruff
Fix-ups discovered by `uv run make verify`. No behavior change. * utils/compaction: hoist `from utils.token_estimator import ...` to the module top. The original local imports were a paranoid hedge against a hypothetical circular import that does not actually exist (token_estimator does not import compaction). Top-level imports also let pylint's import-outside-toplevel rule pass cleanly. * utils/compaction: switch `timezone.utc` to the Python 3.13 `datetime.UTC` alias per ruff UP017. * tests: `import tiktoken` at module top instead of inside a test method (test_token_estimator.py). * tests: `# pylint: disable=too-few-public-methods` at module top for the test files that use one-attribute stand-in classes for message and content-part fixtures. These are intentional duck-type stubs, not real abstractions, and adding methods just to satisfy a count would obscure the intent. * tests: `Configuration.model_fields.get(...)` instead of `"compaction" in Configuration.model_fields`. The dict accessor pylint cannot infer; the .get() form sidesteps the unsupported-membership-test / unsubscriptable-object false positives without weakening the assertion. * models/config: `compaction` Field's `default_factory` now constructs CompactionConfiguration with every field passed explicitly. Matches the established pattern used by `conversation_cache` and `quota_handlers` on the same Configuration class, and resolves pyright reportCallIssue: the bare class form trips pyright's "constructor requires positional args" check because pyright treats the kwargs-with-defaults init as zero-positional but more-than-zero-required-keyword.
1 parent 9eeb7e1 commit 0ad8a96

5 files changed

Lines changed: 18 additions & 17 deletions

File tree

src/models/config.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2007,7 +2007,13 @@ class Configuration(ConfigurationBase):
20072007
)
20082008

20092009
compaction: CompactionConfiguration = Field(
2010-
default_factory=CompactionConfiguration,
2010+
default_factory=lambda: CompactionConfiguration(
2011+
enabled=False,
2012+
threshold_ratio=0.7,
2013+
token_floor=4096,
2014+
buffer_turns=4,
2015+
buffer_max_ratio=0.3,
2016+
),
20112017
title="Conversation compaction configuration",
20122018
description="Controls when conversation history is summarized "
20132019
"to keep the model's input below the context window limit. "

src/utils/compaction.py

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@
2727
to disentangle a tangle of side effects.
2828
"""
2929

30-
from datetime import datetime, timezone
30+
from datetime import UTC, datetime
3131
from typing import Any
3232

3333
from llama_stack_client import AsyncLlamaStackClient
3434

3535
from log import get_logger
3636
from models.compaction import ConversationSummary
37+
from utils.token_estimator import estimate_conversation_tokens, estimate_tokens
3738

3839
logger = get_logger(__name__)
3940

@@ -190,10 +191,6 @@ def partition_conversation(
190191
compaction needed); ``recent_items`` may be empty (everything
191192
had to be summarized).
192193
"""
193-
# Import locally to avoid a top-level circular import once the
194-
# request-flow integration in LCORE-1572 imports both modules.
195-
from utils.token_estimator import estimate_conversation_tokens
196-
197194
msg_indices = _message_indices(items)
198195
if not msg_indices:
199196
return [], items
@@ -325,8 +322,6 @@ async def summarize_chunk(
325322
cannot be persisted (PositiveInt) and is also useless as
326323
context, so propagating an error is the honest behavior.
327324
"""
328-
from utils.token_estimator import estimate_tokens
329-
330325
prompt = _build_summarization_prompt_body(old_items)
331326
logger.info(
332327
"Summarizing %d conversation items (%d messages) for model %s.",
@@ -351,7 +346,7 @@ async def summarize_chunk(
351346
summary_text=summary_text,
352347
summarized_through_turn=summarized_through_turn,
353348
token_count=token_count,
354-
created_at=datetime.now(timezone.utc).isoformat(),
349+
created_at=datetime.now(UTC).isoformat(),
355350
model_used=model,
356351
)
357352

@@ -423,8 +418,6 @@ async def recursively_resummarize(
423418
fold is needed) or when the LLM call yields no
424419
extractable text.
425420
"""
426-
from utils.token_estimator import estimate_tokens
427-
428421
if len(summaries) < 2:
429422
raise ValueError(
430423
"recursively_resummarize requires at least 2 summary chunks "
@@ -459,6 +452,6 @@ async def recursively_resummarize(
459452
summary_text=folded_text,
460453
summarized_through_turn=summaries[-1].summarized_through_turn,
461454
token_count=estimate_tokens(folded_text, encoding_name=encoding_name),
462-
created_at=datetime.now(timezone.utc).isoformat(),
455+
created_at=datetime.now(UTC).isoformat(),
463456
model_used=model,
464457
)

tests/unit/models/config/test_compaction_configuration.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,8 @@ def test_root_configuration_has_compaction_field() -> None:
9797
"""The root Configuration declares a `compaction` field typed as
9898
CompactionConfiguration with a default-factory that produces a
9999
fresh CompactionConfiguration with the spec defaults."""
100-
fields = Configuration.model_fields
101-
assert "compaction" in fields, "Configuration must declare a compaction field"
102-
field_info = fields["compaction"]
100+
field_info = Configuration.model_fields.get("compaction")
101+
assert field_info is not None, "Configuration must declare a compaction field"
103102
assert field_info.annotation is CompactionConfiguration
104103

105104
# default_factory must produce a CompactionConfiguration with disabled

tests/unit/utils/test_compaction.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Unit tests for utils/compaction — partitioning, prompt, summarization."""
22

3+
# pylint: disable=too-few-public-methods
4+
35
from typing import Any
46

57
import pytest

tests/unit/utils/test_token_estimator.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
"""Unit tests for utils/token_estimator."""
22

3+
# pylint: disable=too-few-public-methods
4+
35
from typing import Any
46

57
import pytest
8+
import tiktoken
69

710
from models.config import InferenceConfiguration
811
from utils.token_estimator import (
@@ -93,8 +96,6 @@ def test_within_5pct_of_explicit_tiktoken_call(self) -> None:
9396
is at most 5% — establishing that the estimator does not
9497
introduce its own off-by-some error.
9598
"""
96-
import tiktoken
97-
9899
encoding = tiktoken.get_encoding(DEFAULT_ENCODING_NAME)
99100
text = (
100101
"The quick brown fox jumps over the lazy dog. "

0 commit comments

Comments
 (0)