Skip to content

docs(reviewer): flag cross-module structural duplication, propose a shared generic#194

Draft
gregorydemay wants to merge 2 commits into
mainfrom
gdemay/sharpen-reviewer-cross-module-dry
Draft

docs(reviewer): flag cross-module structural duplication, propose a shared generic#194
gregorydemay wants to merge 2 commits into
mainfrom
gdemay/sharpen-reviewer-cross-module-dry

Conversation

@gregorydemay

Copy link
Copy Markdown
Contributor

Extends the reviewer's Maintainability rules so duplication detection looks ACROSS modules and PRs, not just at copy-paste within a single diff.

The reviewer should now recognize when a new type faithfully mirrors an existing sibling — same field/map shape and near-identical methods — and treat that parallelism itself as the finding, proposing a shared generic core (generics over copy-paste and over macros) rather than accepting a duplicated structure.

…hared generic

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 1, 2026 05:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the reviewer agent’s maintainability guidance to treat “parallel” types (new structures mirroring existing siblings) as actionable duplication, encouraging refactors toward shared generic abstractions across the codebase.

Changes:

  • Adds a new maintainability rule to flag cross-module structural duplication (not only copy/paste within the diff).
  • Guides reviewers to propose extracting shared generic cores (traits + generics) instead of duplicating mirrored types.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .claude/agents/reviewer.md Outdated
not a one-line repeat; as a rule of thumb, roughly 10+ near-identical lines or the
same multi-line block repeated at 2+ sites — is at least 🟠 Medium and gates the verdict;
name the parameterization, helper, or proptest that removes it.
- Flag structural duplication ACROSS modules/PRs, not just copy-paste within the diff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Reworded in a9b5d83. The bullet now scopes the duplication search to the repository codebase (including code outside the current diff/PR) and explicitly excludes other open PRs, so it no longer invites cross-PR comparison the agent cannot reliably access. It also now requires the finding to cite the concrete existing sibling being mirrored (the actual type/module, no speculation). Dropped the "/PRs" phrasing entirely; the rest of the bullet (mirror ⇒ shared generic core, generics over copy-paste and macros, ≥🟠 Medium when substantial, name the unifying generic) is unchanged.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

canbench 🏋 (dir: canister) 3b4ce3e 2026-07-01 05:57:40 UTC

canister/canbench_results.yml is up to date
📦 canbench_results_benchmark.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 14 | regressed 0 | improved 0 | new 0 | unchanged 14]
    change:   [max +12.50K | p75 0 | median 0 | p25 0 | min 0]
    change %: [max +0.02% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 14 | regressed 0 | improved 0 | new 0 | unchanged 14]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 14 | regressed 0 | improved 0 | new 0 | unchanged 14]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

…her PRs

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gregorydemay

Copy link
Copy Markdown
Contributor Author

🤖 Ready for your review. Standalone tooling change — leaving it DRAFT per our flow.

Sharpens the reviewer agent's Maintainability rules with a cross-module heuristic: when a new type closely mirrors an existing sibling (same field/map shape, near-identical methods), the reviewer should treat the parallelism itself as a finding and propose extracting a shared generic core — preferring generics over copy-paste and over macros — rather than only catching within-diff copy-paste. Per Copilot's feedback, the search is scoped to the repository codebase (not other open PRs, which the reviewer can't reliably access) and must cite the concrete sibling being mirrored (no speculation).

Docs-only (.claude/agents/reviewer.md); all CI green; mergeable: MERGEABLE. No formal reviewer pass since there's no spec and it only edits the reviewer's own rules.

Motivation: the trade store in #193 was initially built as a near-line-for-line parallel of OrderHistory and neither the implementer nor the reviewer flagged the DRY-via-generic opportunity — this rule is so that gets caught next time.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants