Skip to content

Codebase quality cleanup: file-size violations, tuple returns, ty pass #280

Description

@padak

Background

After introducing the Code Quality Patterns section in CONTRIBUTING.md and wiring ty (Astral type-checker) into post-edit hooks + pre-commit, we have a clear picture of what the existing codebase needs to satisfy the new rules. The new rules apply hard to new code; legacy is grandfathered, but this issue tracks bringing legacy up to par.

Source of the audit: make typecheck, wc -l, grep -rE '-> *tuple\[', make check-error-codes. Reproducible locally.

Audit snapshot (kbagent 0.31.0)

Category Status Count
ruff check clean 0
ruff format clean 0
Raw error_code strings (vs ErrorCode enum) clean 0
ty check diagnostics dirty 386
File-size HARD ceiling violations dirty 10 files
File-size SOFT ceiling violations dirty 5 files
tuple[...] returns in services/commands dirty 51 sites
Lambdas assigned to variables mostly OK 11 (most are the formatter.output(result, lambda c, d: ...) pattern -- not a violation)

File-size violations (HARD, must split)

3016 LOC  src/keboola_agent_cli/client.py                 [budget 2000]
2765 LOC  src/keboola_agent_cli/services/sync_service.py  [budget 1500]
2494 LOC  src/keboola_agent_cli/commands/storage.py       [budget 1200]
2352 LOC  src/keboola_agent_cli/commands/config.py        [budget 1200]
2180 LOC  src/keboola_agent_cli/services/storage_service.py
1908 LOC  src/keboola_agent_cli/services/data_app_service.py
1730 LOC  src/keboola_agent_cli/services/config_service.py
1483 LOC  src/keboola_agent_cli/commands/lineage.py
1280 LOC  src/keboola_agent_cli/commands/project.py
1238 LOC  src/keboola_agent_cli/commands/data_app.py

SOFT (next-edit-must-split): mcp_service.py (1488), deep_lineage_service.py (1321), commands/sync.py (1031), commands/context.py (948), commands/flow.py (851).

client.py is the most urgent -- per peer review, it currently mixes Storage, Queue, Sandboxes, encryption, and a Google API call that doesn't belong in the Keboola HTTP client.

ty 386 diagnostics -- top categories

139x unresolved-attribute     # mostly dict/Any return values (httpx response.json())
104x invalid-argument-type
 25x unresolved-import        # may include scripts/ path setup; verify
 24x invalid-assignment
 23x unknown-argument
 19x not-subscriptable
 16x not-iterable
 15x invalid-return-type
 13x missing-argument

Estimated split: ~30-40% are ty false positives on dynamic surfaces (httpx response payloads, MCP tool input shapes). Real bugs likely in the 30-80 range. Fix systematically by category, not file-by-file.

tuple[...] returns -- 51 sites

Top offenders (services/):

  • schedule_service.py (6)
  • config_service.py (6)
  • storage_service.py (4)
  • 2 each: flow_service, job_service, mcp_service, member_service, project_service, search_service, variables_service

Patterns recurring across the codebase:

  • tuple[str, ProjectConfig] (alias + config) -- candidate for ResolvedProject
  • tuple[str, list[dict], bool] | tuple[str, dict[str, str]] (success + error union) -- candidate for ServiceResult[T] Result-style dataclass
  • tuple[int, int] (counts) -- candidate for per-domain named pairs

A small set of 5-10 shared dataclasses in models.py likely covers >80% of the 51 sites.

Proposed PR breakdown

Each PR is independent and should land separately so reviews stay scoped.

PR-1: Split client.py by endpoint family

  • Extract into client/storage.py, client/queue.py, client/sandboxes.py, client/files.py, client/encryption.py.
  • Keep BaseHttpClient in http_base.py shared.
  • Move the misplaced Google API call to its own module (or remove if dead).
  • Re-export from client.py for backward compat during the transition.
  • Resolves: 1 hard violation, the most-imported file in the repo, peer-review concern about mixed concerns.

PR-2: Shared dataclasses for common service returns

  • Add ResolvedProject, ServiceResult[T], BranchScope, etc. to models.py.
  • Convert the recurring tuple patterns first (covers ~30 of 51 sites).
  • Leave one-off tuples for follow-up PRs touching those files for other reasons.

PR-3: ty pass -- fix real bugs, annotate dynamic surfaces

  • Tag httpx response payloads with dict[str, Any] returns explicitly.
  • Add # ty: ignore[<rule>] ONLY for documented dynamic surfaces (MCP tool inputs, raw response.json()).
  • Fix real bugs uncovered (likely 30-80).
  • Flip pre-commit ty from warning-only to blocking when this lands.

PR-4: Split large commands + services

  • commands/storage.py (2494 LOC) -> by subcommand family (buckets, tables, files, descriptions).
  • commands/config.py (2352 LOC) -> (browse, mutate, metadata, rows, variables).
  • services/sync_service.py (2765 LOC) -> extract pull/push/diff into siblings.
  • services/storage_service.py (2180 LOC) -> extract table/file/bucket helpers.

PR-5 (lower priority): Soft-ceiling files + remaining tuples

  • Address as files are touched for other work, per the "split before crossing hard ceiling" rule.

Out of scope for this issue

  • Lambdas assigned to variables (11 sites) -- audit showed they are the established formatter.output(_, lambda c, d: c.print(...)) dual-output pattern and the client.py auth_fn=lambda closures for cloud download. These are not anti-patterns.
  • make check-error-codes already enforces ErrorCode enum usage. Nothing to do.
  • Ruff lint/format are clean.

Acceptance

This issue is done when:

  • All files under hard ceilings (file-size table in CONTRIBUTING.md).
  • tuple[...] returns in services/commands reduced to < 10 sites (genuine 2-element returns that don't warrant a dataclass).
  • make typecheck exits 0 (pre-commit ty switched from warning to blocking).
  • client.py no longer mixes Keboola endpoints with Google API / encryption / cloud download into a single file.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions