refactor: give each duplicated rule one home - #104
Merged
Conversation
Tags were downcased in four places and stripped in only one, so the same hashtag reached three of them as a different string. The username rule was two regexes that disagreed on case and length with nothing saying whether that was intended — it is, and both are now derived from one character class with the reason written down. The ranking formula was two copies bound by a comment; it moved to a plain module the benchmark script can require without booting Rails, which is why the copy existed. Nine guards, each checked by mutation rather than trusted: dropping strip, removing digits from the username characters and changing the decay exponent all fail an example. Finding 8 stays open on the review's own judgement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes finding 9 of the 2026-08-18 architecture review, tracked in #90. Three rules, each written more than once — and each needing a different answer, which is why this wasn't mechanical.
Tags are lowercase — four copies, one of them different
Tag,Post#sync_tags,ApplicationHelperandTagsControllereach downcased, and onlyTagalso stripped whitespace. So"#Ruby "reached three of them as a different string from the one the fourth stored.Tag.normalizeis now the rule; the other three call it.What a username is — two definitions that disagreed
Nothing said whether the disagreement was intended. It is — and that's the interesting part: the route is deliberately looser so
/@ADAreaches the controller, which downcases before looking up. A stricter route would 404 on a handle someone capitalised, which is worse than finding it.So this isn't "delete one". Both are now derived from one character class and one length range, with that reasoning written where they're defined. Verified
/@adaand/@ADAboth return 200 against a running server.The ranking formula — two copies bound by a comment
RankedFeed#scoreandscript/scaling-curveeach held the expression, joined only by the words "the ranking, as RankedFeed#score computes it". A comment cannot fail.It moved to
Ranking— a plain module with no Rails dependency, which is precisely what lets the benchmark script require it directly. That dependency was the reason the copy existed.Cost, stated
config/routes.rbnow references a model constant, so the routes file autoloadsUserat boot. Supported, and verified (bin/rails routesand a live/@adaboth work) — but it's a coupling the previous literal didn't have. The alternative, a spec asserting the two regexes agree, would have left two definitions plus a test holding them together, which is the shape this finding is about.Measured
The guards were checked by mutation, not trusted. Dropping
strip, removing digits from the username character class, and changing the decay exponent each failed an example — because nine specs written green prove nothing on their own. Suite 380 → 389, RuboCop clean.Finding 8 is deliberately not done
Like and Repost are one concept written twice across twelve files. The review's own judgement is that the duplication is stable and worth doing "when a third engagement type appears". Building a shared abstraction for two cases with no third in sight is exactly the scaffolding-ahead this repo forbids — so #90 stays open for finding 8, with that reasoning recorded in the findings table rather than closed by a premature module.
Generated by Claude Code