You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
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.
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.
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.
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
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.
Then a repo-declared allowlist with an equality assertion, rather than more built-in categories.
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.
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.
For step 2 (the repo-declared allowlist): colbymchenry/codegraph (MIT) uses a single optional repo-root codegraph.json (src/project-config.ts, cached by mtime), and it's a good shape for one file that carries more than the allowlist:
exclude: gitignore-style patterns. It keeps committed vendor code out on index, sync and watch.
include: forces gitignored first-party source back in. Built-in skips (node_modules, dist) are never re-included, and exclude still wins.
deprioritize: paths stay indexed but rank below first-party code. This is the "don't drop, just don't report first" idea, which fits orphans as well.
extensions: maps custom file extensions to a language.
Bad entries are warned about and skipped, never fatal. Their generated-file detection (src/extraction/generated-detection.ts) matches ours in spirit (path patterns plus a "Code generated … DO NOT EDIT" banner in the file head) and only lowers rank.
Suggestion: put the orphans allowlist into a [tool.cgis] section in pyproject.toml, or a cgis.toml, with exclude and deprioritize from day one, so step 2 doesn't create a single-purpose config.
Activity
fix(query): a prefix matching nothing is a misconfiguration, not a cl…
feat(query): orphan classes — what production no longer builds (#415) (…
zaebee commentedon Sep 6, 2026
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.Configunder "alive by metaclass". Their sweep does not report it for a different reason: it only ever considers top-levelClassDefs —ast.parse(...).body, notast.walk— because a nested class carries a different question. It is almost always a config block, aMeta, 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-api70c679b6, outsideapp/gen: their guard reports 2,cgis orphansreports 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:
--top-level-onlyflag matching the sweep's scope;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
KEPTlist 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
KEPTentries areChatAttachmentandChatModerationEvent: migrated chat tables whose writer is not built yet, and deleting the model would autogenerate aDROP 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, pydanticConfigandSQLModel(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
DO NOT EDIT/@generatedheader, or a path glob) and it is 16 of owner-api's 19 orphans on the current ref. Exclude by default,--include-generatedto opt back in.Also note: the numbers originally in this issue came from a checkout 14 merges behind owner-api main. On
70c679b6the report is 19 orphans — 16 undergen/, 2 chat models, 1 nestedConfig— not 43. Corrected here so the ratio in §1 is the current one.feat(orphans): generated classes are noise, not findings — hide them …
feat(orphans): generated classes are noise, not findings — hide them …
zaebee commentedon Sep 9, 2026
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 fromcgis orphansby default with--include-generatedto opt back in. Measured on owner-api atb7d02fe6: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 editwith the stemgeneratclassified 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_generatedhas 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 blanksfiles_statehashes (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.Configis 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/ChatModerationEventcategory, whose reason is repo knowledge.SQLModel(table=True)does not reproduce onb7d02fe6at all — those two chat models are no longer reported. It needs a repository where it recurs before it can be measured.[-]cgis orphans: generated stubs and framework-registered classes dilute the report (~40% of findings)[/-][+]cgis orphans: repo-declared allowlist + nested-class scope (generated-stub filter shipped)[/+]zaebee commentedon Sep 16, 2026
Retitled to steps 2–3: step 1 (generated-file filter,
--include-generated) shipped in #441.feat(orphans): consider module-level classes only by default (#432) (#…
zaebee commentedon Oct 2, 2026
For step 2 (the repo-declared allowlist): colbymchenry/codegraph (MIT) uses a single optional repo-root
codegraph.json(src/project-config.ts, cached by mtime), and it's a good shape for one file that carries more than the allowlist:exclude: gitignore-style patterns. It keeps committed vendor code out on index, sync and watch.include: forces gitignored first-party source back in. Built-in skips (node_modules,dist) are never re-included, andexcludestill wins.deprioritize: paths stay indexed but rank below first-party code. This is the "don't drop, just don't report first" idea, which fits orphans as well.extensions: maps custom file extensions to a language.Bad entries are warned about and skipped, never fatal. Their generated-file detection (
src/extraction/generated-detection.ts) matches ours in spirit (path patterns plus a "Code generated … DO NOT EDIT" banner in the file head) and only lowers rank.Suggestion: put the orphans allowlist into a
[tool.cgis]section inpyproject.toml, or acgis.toml, withexcludeanddeprioritizefrom day one, so step 2 doesn't create a single-purpose config.Notes:
research/competitor-codegraph.md(idea 8).Generated by Claude Code