Skip to content

cgis orphans: repo-declared allowlist + nested-class scope (generated-stub filter shipped) #432

Open

Description

@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.

Activity

  1. zaebee commented on Sep 6, 2026

    @zaebee
    OwnerAuthor

    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.

  2. zaebee commented on Sep 9, 2026

    @zaebee
    OwnerAuthor

    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.

  3. changed the title [-]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)[/+] on Sep 16, 2026
  4. zaebee commented on Sep 16, 2026

    @zaebee
    OwnerAuthor

    Retitled to steps 2–3: step 1 (generated-file filter, --include-generated) shipped in #441.

  5. zaebee commented on Oct 2, 2026

    @zaebee
    OwnerAuthor

    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.

    Notes: research/competitor-codegraph.md (idea 8).


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    could-haveNice for AI agents and UXenhancementNew feature or request

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions