Skip to content

Step subclasses are discovered by accident, and not at all outside a whole-repo run #5

Description

@borisdev

naming.naming_drift is structurally blind to Step subclasses — the vocabulary this package is about.

The motivating example, run

Three Step classes doing one job — sum a list — differing only in which Variable they read. This is the mis-decomposition that actually happened in examples/ during the Pydantic Graph comparison:

class Total(Step):
    """Sum the values."""
    inputs, outputs = (squares,), (total,)

class TotalKept(Step):
    """Sum the values."""
    inputs, outputs = (kept,), (total,)

class TotalWeighted(Step):
    """Sum the values."""
    inputs, outputs = (weighted,), (total,)

Three names, one concept — the textbook case for naming.naming_drift, whose own docstring says "a coding agent eventually reads an alias as a new architectural concept and writes a second implementation of it."

$ conceptlint <that module>
$ echo $?
0

Silent. Not a judgement call it got wrong — it never saw them.

Why

plan_types/naming/records.py:

MODEL_BASES = frozenset({"BaseModel", "RootModel"})

and _no_drift requires two corroborating signals — name overlap and (field overlap or definition overlap). A Step subclass has no Pydantic fields, so the second signal can never fire on one. The two-signal rule is right and should stay; it just has no second signal available here.

⚠️ Note the asymmetry — this is not "the linter can't see Steps"

naming.ambiguous_reference does work on Steps, and proved it by catching a real duplicate of mine within the hour:

naming.ambiguous_reference: Square is declared twice with different shapes
  concepts : Square (examples/.../stage2_strategies.py:34), Square (examples/.../stage3_map_join.py:43)

So one name, many concepts is covered for Steps. Many names, one concept is not. Half the "two laws" the README leads with is unenforced on the package's own central type.

Proposed fix

A Step already has the exact analogue of a field set: its ports. Step.shape() returns (input types, output types), and two Steps with equal shapes are declared substitutable elsewhere in this package (check_arms, AIEvalTrial). That is a shape signal, and it is the one near_duplicates is missing.

So: for classes whose base resolves to Step, use shape() where fields is used today. Total/TotalKept/TotalWeighted all have shape ((list,), (int,)) and share a head noun — two signals, fires.

Not proposing dropping the two-signal rule. A checker that fires on names alone is one people turn off, and that reasoning is already written down in records.py.

What would close this

  • a test asserting drift fires on the Total/TotalKept/TotalWeighted module above
  • a test asserting it does not fire on two Steps that share a head noun and have genuinely different shapes (UserRequest/SearchRequest is the existing prose example; the Step equivalent)
  • a decision on whether map_over participates in the shape — a mapped Step's declared ports are the ITEM types, so Square: int -> int and a non-mapped Double: int -> int would look identical. Probably wired_inputs/wired_outputs is the right basis, not inputs/outputs.

How it went unnoticed, which is the transferable part

Nothing reported it because nothing was looking: CI linted plan_types/ only, and the duplicate was in examples/. Fixed separately — the gate now lints . with the deliberate evals/minimal/ pairs carried in the baseline number rather than behind a path exclusion.

That failure has now happened twice in this repo with two different directories, and .conceptlint-baseline warned about the first one in a comment that was still true when the second one shipped: "a gate pointed at the wrong directory reads exactly like a clean codebase."

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions