TimZander / TimZander/claude

/deep-review: flag comments describing peer-class internals

Open Beginner friendly
#118 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
0
Forks
1
Avg merge
1d 3h
Merged PRs (30d)
7

Description

Problem

/deep-review's Step 6 ("Leave It Better") covers comment quality generically but does not flag a specific subtype that causes repeated review churn: comments at DI registration sites, call sites, or peer-class xmldoc that describe how a different class works.

On a recent review of a .NET auth handler change, /deep-review found many real issues but missed three leaky-comment instances, each of which had to be flagged manually by the human reviewer in a separate feedback round:

  1. // Handler enforces membership by looking up admin email in DB at AddAuthorization(...)
  2. // Singleton because handler is stateless and deps IFoo/IBar are singleton-safe at AddSingleton<IAuthorizationHandler, Handler>()
  3. // ...via IMyService (which caches per-oid) in the handler's xmldoc summary

Each comment described a peer class's internals rather than the site's own concerns. The rationale belongs on the class it describes, not where the class is registered or called. These comments duplicate knowledge and rot the moment the peer class changes.

Proposal

Extend Step 6 of /deep-review to explicitly look for:

  • Comments at DI registration calls (AddSingleton, AddScoped, AddTransient, framework equivalents) that describe the registered type's internals (caching, dependency list, statefulness, error handling).
  • Comments at new Foo(...) or method call sites that describe Foo's internals.
  • xmldoc summaries on class A that describe peer class B's internals.

Surface as 💡 findings using the format:

💡 path/to/file.cs:L — describes <peer class> internals; move to <peer class>.cs or delete

The rule, stated concisely for the skill file: class internals belong on the class itself; registration sites, call sites, and peer summaries should document only their own concerns (lifetime, ordering, policy name, config keys being bound).

Acceptance Criteria

  • Step 6 in the /deep-review skill file includes this check with concrete avoid/rewrite examples
  • Findings use the rewrite-hint format above (pointing at the class file where the rationale belongs)
  • Existing Step 6 checks (clarifying comments, dead code, naming, performance, logging) are unchanged — this is additive
  • A short note in the skill prompt reminds the reviewer that opportunistic cleanup of leaky comments the author already wrote is in-scope for Step 6

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at Step 6 in the /deep-review skill file and review its existing checks for clarifying comments, dead code, naming, performance, and logging. Add the peer-internals check, concrete avoid/rewrite examples, the specified rewrite-hint format, and the note about opportunistic cleanup; done means the existing checks remain unchanged and all acceptance criteria are covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
developer-experience, tooling
Issue type
Feature
Difficulty
2/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.