cgis orphans: generated stubs and framework-registered classes dilute the report (~40% of findings)

[zaebee]

Found while reviewing #431 (the cgis orphans query, closing #415).

The gap

The query is correct about these classes — nothing constructs, extends or names them — but a reader will not act on any of them, so they dilute the signal. Hand-checking 14 of owner-api's 43 reported orphans:

category	count (of 43)	example	why it is noise
generated code	16	api.dependencies.grpc.services.ConfigServiceStub	betterproto stubs under api/dependencies/grpc/, header says DO NOT EDIT. Genuinely unused, never hand-deleted.
alive by metaclass	1+	domains.rating.schemas.RatingResponse.Config	a pydantic inner class Config is consumed by the model's metaclass and never named
alive by registration	1+	models.chat.ChatAttachment	SQLModel, table=True — the class is the table definition; importing it is the point

Usable signal on that repository is roughly 25 of 43. On a repo with a larger generated surface the ratio would be worse.

Why each needs a marker the graph does not have

• Generated: needs an is_generated notion. The conventions are real and narrow — a DO NOT EDIT / @generated marker in the first few lines, or a configured path glob. This is the highest-value one: 16 of 43.
• Metaclass-consumed nested class: Config inside a pydantic model is the common case, but "a nested class the enclosing class's metaclass reads" is not visible in the AST at all. A narrower rule — a nested class whose owner is live — would also hide genuinely dead nested classes, so it needs measuring before choosing.
• Registration side-effect: table=True is a keyword argument in the class definition's bases. Detectable, but it is one framework's convention; the general shape is "a class whose existence is its purpose", which also covers Django models, enum registries, and plugin subclasses.

Suggested order

1. is_generated from a header marker, and exclude by default with a --include-generated opt-out. Measure the drop on owner-api (expect 43 → ~27) and on the other four repos in the D10 test set.
2. Decide the nested-class question with numbers rather than a rule: count how many nested classes the query reports across the five repos, and how many of those are genuinely dead.
3. table=True only if step 2 leaves it looking common.

The related but distinct limitation — a class named only inside a decorator — is #429, and that one is under-reporting rather than noise.

[zaebee]

Two design constraints from the owner-api maintainer, whose hand-written sweep this query replaces. Both change what a good fix looks like here.

1. The nested-class case is a scope decision, not noise

I filed domains.rating.schemas.RatingResponse.Config under "alive by metaclass". Their sweep does not report it for a different reason: it only ever considers top-level ClassDefs — ast.parse(...).body, not ast.walk — because a nested class carries a different question. It is almost always a config block, a Meta, or an enum body the enclosing class's metaclass consumes.

So the two tools do not disagree about Config; one looks at nested classes and the other does not. On the same scope they agree exactly — calibrating on owner-api 70c679b6, outside app/gen: their guard reports 2, cgis orphans reports the same 2 plus this one nested class.

That reframes the item: the choice is which scope the query answers, not which classes to filter. Options worth measuring rather than guessing:

• report nested classes but mark them, so the reader can tell the question apart;
• a --top-level-only flag matching the sweep's scope;
• report a nested class only when its enclosing class is also unreferenced.

Count first: how many nested classes does the query report across the five-repo set, and how many of those are genuinely dead?

2. A suppression list rots; an equality assertion does not

The stronger point. Their KEPT list works not because it is a list but because the test asserts equality: it fails when an entry is stale — the class was deleted or became referenced — as well as when a new orphan appears. A scanner's exclude-list only fails in one direction, so it silently accretes entries that stopped being true, and ends up as the thing it replaced.

Whatever mechanism lands here should keep both directions. Concretely, if the repo gets a way to say "this class is unreferenced on purpose, here is why", the check must also fail when that justification no longer applies.

3. The reason belongs next to the code

Their KEPT entries are ChatAttachment and ChatModerationEvent: migrated chat tables whose writer is not built yet, and deleting the model would autogenerate a DROP TABLE. That reason cannot live in a scanner's config — it is repo knowledge, and it is exactly the same shape as the three categories in this issue. Generated code, pydantic Config and SQLModel(table=True) are all "unreferenced and nobody deletes them", and the why differs per repo.

Which argues against a built-in filter list as the primary mechanism, and for something the repository writes down — with the tool reading it and asserting it stays true.

Revised suggestion

1. Still start with generated code: it is the one category with a portable marker (DO NOT EDIT / @generated header, or a path glob) and it is 16 of owner-api's 19 orphans on the current ref. Exclude by default, --include-generated to opt back in.
2. Then a repo-declared allowlist with an equality assertion, rather than more built-in categories.
3. Decide the nested-class scope with counts, per §1.

Also note: the numbers originally in this issue came from a checkout 14 merges behind owner-api main. On 70c679b6 the report is 19 orphans — 16 under gen/, 2 chat models, 1 nested Config — not 43. Corrected here so the ratio in §1 is the current one.

[zaebee]

Step 1 landed in #441 (52e0de1); this issue now tracks steps 2 and 3 only.

What shipped

Node.is_generated, stamped once per file by the pipeline from a header self-declaration, and excluded from cgis orphans by default with --include-generated to opt back in. Measured on owner-api at b7d02fe6:

cgis orphans      6 of 699  ->  1 of 488   (211 generated classes not considered)

Note the numbers moved again from this issue's own history — 43, then corrected to 19, and 6 on b7d02fe6. Anything measured here needs re-measuring before it is quoted.

Corrections to what this issue assumed

The header window had to be 15 lines, not 5. openapi-generator and swagger-codegen put both markers inside the module docstring, at roughly lines 10 and 12; a 5-line window missed every one of their files. The 5-line version was justified in review by domains/chat/schemas.py "mentioning generation in its docstring" — that file contains no "do not edit" anywhere, and its only generation phrase is at line 244. The justification was a misread grep; the file was never at risk.

"Generated" as a stem is not a marker. Pairing do not edit with the stem generat classified a code-generation project's own template ("Template for the generated client. DO NOT EDIT the output") as generated, hiding every class in it. The rule is now a self-declaration — generated by / auto-generated / generated with / @generated.

The stale-graph signal needed a real remedy. is_generated has no backfill, and the report says re-ingest — but incremental ingest short-circuits on a content hash, so that advice could not be followed. The migration now blanks files_state hashes (blanks, not deletes: deleting breaks stale-file cleanup for files removed before the upgrade).

Still open

Step 3 — the nested-class scope decision. On the current ref domains.rating.schemas.RatingResponse.Config is now the only row on owner-api, so this is no longer a dilution question but the whole remaining report. Your §1 comment frames it correctly as a scope choice, and it still needs the cross-repo count it asks for.

Step 2 — the repo-declared allowlist with an equality assertion. Untouched. Note that generated-code detection deliberately did not use a suppression list, for the reason in your comment — a header marker stops being true when the file stops being generated, so there is nothing to go stale. The allowlist is still the right mechanism for the ChatAttachment / ChatModerationEvent category, whose reason is repo knowledge.

SQLModel(table=True) does not reproduce on b7d02fe6 at all — those two chat models are no longer reported. It needs a repository where it recurs before it can be measured.