[no-ticket] Have reviews catch renames of names persisted outside the codebase - #47
Merged
Merged
Conversation
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
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.
no-ticket
A 204-file rename of
Billing::BusinessEntitlementtoBilling::OrganizationEntitlementrenamed a Sidekiq job along with it. The queue and retry set store a job's class as a string, so every instance already in Redis at deploy time would have raisedNameErroron constantize and gone on retrying for weeks. The review approved the PR; the miss only surfaced from a manual second look.The general shape is a name that lives somewhere other than the codebase — Redis, a database column, a third party's records — written by code that is already running and read back by the new code after the deploy. The diff shows the rename and nothing about the data still using the old name, so there is nothing on the page to react to unless the reviewer is looking for it.
prompts/_shared/rename-compatibility-rules.mdasks for that check and lists where the old name typically survives: Sidekiq/ActiveJob classes, maintenance task names inmaintenance_tasks_runs, STI and polymorphic type columns, GlobalIDs, ActiveHash ids, Flipper keys, cache and Redis keys, experiment names, and keys already sent to a third party such as Stripe subscription metadata. It also says plainly that renaming the Ruby constant while leaving the persisted name alone is correct, so a table name or metadata key that deliberately stays put does not turn into a finding.Wired into all three prompts —
codex-first-pass,claude-synthesize, andclaude-synthesize-thesis-first— so both experiment arms get it.