Skip to content

Commit 889b709

Browse files
flyersworderclaude
andcommitted
chore: replace file:line references with symbol names in comments
Line-number references drift silently. Several were already wrong: factory.py:627-628 / :631-636 / :632-702 (cited as run_query, now lookup_domain), validator.py:307 (cited as the EXPLAIN round-trip, now the relationship advisory block), test_validator.py:412 (a class boundary), and factory.py:634-636 in test_sdk. Each now names the symbol it means -- Validator.validate -> explain_adapter.explain, run_query's _with_remaining, Validator's frozen _QueryRuleEntry, RequiredFilterValuesChecker's class docstring, and TestValidatorWithSemanticSource::test_validator_with_semantic_source_emits_warnings. The four references that were still accurate are converted too, so the convention is uniform and the drift class is gone. Comments only -- no behaviour change. Full suite 812 passed; prek green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d788fdc commit 889b709

8 files changed

Lines changed: 36 additions & 29 deletions

File tree

src/agentic_data_contracts/tools/factory.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,8 @@ def create_tools(
235235
)
236236

237237
# Build relationship index for BFS path-finding in lookup_relationships.
238-
# This is a snapshot, same pattern as Validator (validator.py:70) which also
239-
# captures relationships at construction time. Direct table lookups go
238+
# This is a snapshot, same pattern as Validator's frozen _QueryRuleEntry,
239+
# which also captures its inputs at construction time. Direct lookups go
240240
# through semantic_source.get_relationships_for_table() instead.
241241
_rel_index = (
242242
build_relationship_index(semantic_source.get_relationships())
@@ -392,8 +392,8 @@ async def preview_table(args: dict[str, Any]) -> dict[str, Any]:
392392
# QUERY SHAPE the caller writes — `required_filter`, `no_select_star`,
393393
# `require_limit`, `max_joins` — because preview synthesises its own
394394
# SELECT * LIMIT N and those rules guard run_query's user-supplied SQL.
395-
# Mirrors Validator._is_table_in_scope + _rule_applies_to_principal
396-
# (validator.py:233-247); keep the table/principal predicates in sync.
395+
# Mirrors Validator._is_table_in_scope + _rule_applies_to_principal;
396+
# keep the table/principal predicates in sync.
397397
block_msgs: list[str] = []
398398
warn_msgs: list[str] = []
399399
log_msgs: list[str] = []
@@ -765,8 +765,9 @@ async def trace_metric_impacts(args: dict[str, Any]) -> dict[str, Any]:
765765
async def inspect_query(args: dict[str, Any]) -> dict[str, Any]:
766766
sql = args.get("sql", "")
767767
# validate() runs a synchronous EXPLAIN/dry-run round-trip to the DB
768-
# (validator.py:307) when an explain adapter is configured, so offload
769-
# it to a worker thread to keep the event loop responsive.
768+
# (Validator.validate -> explain_adapter.explain) when an explain
769+
# adapter is configured, so offload it to a worker thread to keep the
770+
# event loop responsive.
770771
result = await asyncio.to_thread(validator.validate, sql)
771772
data: dict[str, Any] = {
772773
"valid": not result.blocked,
@@ -799,8 +800,9 @@ def _with_remaining(msg: str) -> str:
799800
)
800801

801802
# Phase 1 + 2: query checks + EXPLAIN. validate() makes a synchronous
802-
# EXPLAIN/dry-run DB round-trip (validator.py:307), so offload it to a
803-
# worker thread to avoid blocking the event loop.
803+
# EXPLAIN/dry-run DB round-trip (Validator.validate ->
804+
# explain_adapter.explain), so offload it to a worker thread to avoid
805+
# blocking the event loop.
804806
vresult = await asyncio.to_thread(validator.validate, sql)
805807
if vresult.blocked:
806808
session.record_retry()

src/agentic_data_contracts/tools/langchain.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
violations as JSON without blocking. The in-tool path here therefore only
2626
runs ``session.check_limits()`` and the ``BLOCKED —`` prefix sniff; SQL
2727
validation is left to the underlying tools (``run_query`` self-validates
28-
at ``factory.py:632-702``).
28+
inside ``factory.create_tools``).
2929
3030
Requires the ``[langchain]`` extra: ``pip install agentic-data-contracts[langchain]``.
3131
"""
@@ -54,8 +54,8 @@
5454

5555
def _with_remaining(message: str, session: ContractSession) -> str:
5656
"""Append the canonical ``Remaining: {budget}`` suffix used by
57-
``run_query`` (factory.py:627-628) so wrapper-emitted blocks carry
58-
the same diagnostic footprint as run_query's own blocks."""
57+
``run_query``'s own ``_with_remaining`` helper, so wrapper-emitted
58+
blocks carry the same diagnostic footprint as run_query's own blocks."""
5959
return f"{message}\nRemaining: {json.dumps(session.remaining(), default=str)}"
6060

6161

@@ -220,9 +220,9 @@ def _check(self, request: ToolCallRequest) -> ToolMessage | None:
220220
# Session-limit breach: do NOT call ``record_retry()`` here. The
221221
# session is already past its cap; recording another retry would
222222
# increment past it for no benefit and risks double-counting if a
223-
# future ceiling is added. This mirrors ``factory.py:631-636`` where
224-
# ``run_query`` similarly skips ``record_retry`` on limit-exceeded
225-
# but does record on validation-block (next branch).
223+
# future ceiling is added. This mirrors ``run_query`` in ``factory``,
224+
# which similarly skips ``record_retry`` on limit-exceeded but does
225+
# record on validation-block (next branch).
226226
try:
227227
self._session.check_limits()
228228
except LimitExceededError as e:
@@ -274,9 +274,10 @@ async def awrap_tool_call(
274274
handler: Callable[[ToolCallRequest], Awaitable[ToolMessage | Command[Any]]],
275275
) -> ToolMessage | Command[Any]:
276276
# _check runs the synchronous Validator.validate(), which makes a
277-
# blocking EXPLAIN/dry-run DB round-trip (validator.py:307). Offload it
278-
# to a worker thread so a slow check cannot stall the event loop in an
279-
# async runtime. The sync wrap_tool_call path calls _check directly.
277+
# blocking EXPLAIN/dry-run DB round-trip via explain_adapter.explain.
278+
# Offload it to a worker thread so a slow check cannot stall the event
279+
# loop in an async runtime. The sync wrap_tool_call path calls _check
280+
# directly.
280281
blocked = await asyncio.to_thread(self._check, request)
281282
if blocked is not None:
282283
return blocked

src/agentic_data_contracts/tools/middleware.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ async def wrapper(args: dict[str, Any]) -> dict[str, Any]:
4949
sql = args.get("sql", "")
5050
if sql:
5151
# validate() makes a synchronous EXPLAIN/dry-run DB round-trip
52-
# (validator.py:307), so offload it to a worker thread to keep
53-
# the event loop responsive — same rationale as factory.py.
52+
# (Validator.validate -> explain_adapter.explain), so offload it
53+
# to a worker thread to keep the event loop responsive — same
54+
# rationale as factory.py.
5455
result = await asyncio.to_thread(validator.validate, sql)
5556
if result.blocked:
5657
session.record_retry()

src/agentic_data_contracts/tools/sdk.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
1111
SQL validation is intentionally **not** auto-applied. Doing so would
1212
block ``inspect_query`` from reporting violations as JSON; the canonical
13-
``run_query`` self-validation at ``factory.py:632-702`` already covers
14-
the cost path.
13+
``run_query`` self-validation inside ``factory.create_tools`` already
14+
covers the cost path.
1515
1616
Pass ``apply_middleware=False`` to opt out (preserves the pre-0.20.0
1717
behavior where only ``run_query`` self-checked limits).
@@ -35,8 +35,8 @@
3535

3636
def _with_remaining(message: str, session: ContractSession) -> str:
3737
"""Append the canonical ``Remaining: {budget}`` suffix used by
38-
``run_query`` (factory.py:627-628) so wrapper-emitted blocks carry
39-
the same diagnostic footprint as run_query's own blocks."""
38+
``run_query``'s own ``_with_remaining`` helper, so wrapper-emitted
39+
blocks carry the same diagnostic footprint as run_query's own blocks."""
4040
return f"{message}\nRemaining: {json.dumps(session.remaining(), default=str)}"
4141

4242

tests/test_portability/test_contract_round_trip.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@
3333

3434
# A query whose verdict depends on the semantic source: joining orders -> customers
3535
# without the relationship's required_filter ("status != 'cancelled'") makes the
36-
# producer emit a warning (mirrors test_validator.py:412). The table allowlist and
37-
# rules (tenant_id filter present, explicit columns) are satisfied, so it is never
36+
# producer emit a warning (mirrors
37+
# TestValidatorWithSemanticSource::test_validator_with_semantic_source_emits_warnings).
38+
# The table allowlist and rules (tenant_id filter, explicit columns) are
39+
# satisfied, so it is never
3840
# *blocked* — the only enforcement signal is the semantic-source-derived warning.
3941
SEMANTIC_DEPENDENT_SQL = (
4042
"SELECT o.id, c.name FROM analytics.orders o"

tests/test_tools/test_factory_principals.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,8 +461,8 @@ async def test_required_filter_values_unmapped_caller_passes(
461461
self, preview_adapter: DuckDBAdapter
462462
) -> None:
463463
# C2 companion: callers absent from values_by_principal fall through
464-
# the rule (mirrors RequiredFilterValuesChecker's documented contract,
465-
# checkers.py:339-340). Preview proceeds.
464+
# the rule (mirrors RequiredFilterValuesChecker's documented
465+
# contract in its class docstring). Preview proceeds.
466466
contract = _preview_contract(
467467
"""
468468
- name: tenant_scoped_to_alice

tests/test_tools/test_row_format.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,8 @@ async def test_preview_table_compact_carries_columns(
242242
text = result["content"][0]["text"]
243243
# `columns` must precede `rows` in the serialized body — json.dumps
244244
# preserves insertion order, so the model reads the header before the
245-
# positional values it must align to (factory.py:449-450).
245+
# positional values it must align to (see the key-order comment in
246+
# preview_table).
246247
assert text.index('"columns"') < text.index('"rows"')
247248
body = json.loads(text)
248249
assert body["columns"] == ["id", "amount", "tenant_id"]

tests/test_tools/test_sdk.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ async def test_wrap_with_session_check_blocks_on_limit_exceeded(
146146
callable returns the canonical BLOCKED envelope without invoking
147147
the inner function. The envelope includes a ``Remaining:`` suffix so
148148
the agent can see remaining budget — matching ``run_query``'s own
149-
blocked-path format at ``factory.py:634-636``."""
149+
blocked-path format, produced by its ``_with_remaining`` helper."""
150150
session = ContractSession(contract_with_limits)
151151
for _ in range(4): # exceed max_retries=3
152152
session.record_retry()

0 commit comments

Comments
 (0)