Skip to content

Skills dataclasses have overlapping responsibilities and repetitive transformations #413

@tunahorse

Description

@tunahorse

Summary

The skills dataclass layer feels structurally off. The current model stack works, but the responsibilities and data flow are muddled enough that the code is hard to reason about and easy to drift.

Current shape

The skills pipeline currently passes through several near-overlapping dataclasses:

  • src/tunacode/skills/discovery.py
    • DiscoveredSkillPath
    • SkillRoot
  • src/tunacode/skills/models.py
    • SkillSummary
    • LoadedSkill
    • SelectedSkill
  • src/tunacode/skills/loader.py
    • converts DiscoveredSkillPath into SkillSummary and LoadedSkill
  • src/tunacode/skills/selection.py
    • converts LoadedSkill into SelectedSkill

What feels wonky

1. Repeated identity/path data across multiple dataclasses

name, source, skill_dir, and skill_path are repeated across:

  • DiscoveredSkillPath
  • SkillSummary
  • LoadedSkill
  • SelectedSkill

That makes the model stack feel more like a series of snapshots of the same concept than a set of distinct domain types.

2. Each transition copies almost the same object forward

The pipeline repeatedly reconstructs a new dataclass with mostly the same fields:

  • discovery produces DiscoveredSkillPath
  • loader turns that into SkillSummary
  • loader turns that into LoadedSkill
  • selection turns that into SelectedSkill

Most of those transitions are copy-forward transformations with one or two extra fields added at each stage.

3. SelectedSkill mixes domains

SelectedSkill currently bundles together:

  • filesystem identity
  • loaded content
  • referenced/related file bookkeeping
  • session attachment ordering via attachment_index
  • prompt-rendering input

That makes it unclear whether SelectedSkill is supposed to represent:

  • a loaded skill document
  • a session attachment
  • prompt materialization state
  • all of the above

4. Boundary between discovery, loading, caching, and selection is fuzzy

registry.py caches SkillSummary and LoadedSkill, while selection.py rediscover-skills-by-name and then rebuilds another shape for prompt rendering. The same underlying concept is being rediscovered, reloaded, and rematerialized in multiple places.

5. The model layering is harder to validate than the rendered prompt output

The existing test coverage I found (tests/unit/core/test_agent_skills_prompt_injection.py) verifies the rendered prompt content, but it does not really pin down the intended invariants between the discovery/load/select dataclass layers.

Impact

This is mostly maintainability and correctness risk rather than an obvious crash bug:

  • harder to tell which shape is authoritative at each phase
  • easy to add a field to one dataclass and forget one of the later copy-forward steps
  • harder to extend the skills subsystem without adding another translation hop
  • caching and selection logic have to care about several subtly different representations of the same skill

Evidence map

Relevant files:

  • src/tunacode/skills/discovery.py
  • src/tunacode/skills/models.py
  • src/tunacode/skills/loader.py
  • src/tunacode/skills/registry.py
  • src/tunacode/skills/selection.py
  • src/tunacode/skills/prompting.py
  • tests/unit/core/test_agent_skills_prompt_injection.py

Scope of this issue

This issue is only meant to map the problem shape and the awkwardness in the current dataclass layering. It is not proposing a fix yet.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions