untangle-codebase
Compare original and translation side by side
🇺🇸
Original
English🇨🇳
Translation
ChineseStart a thorough refactor of this codebase and complete it to the end.
Treat this as a goal.
Why do this at all?
- Better output: agents write better code in a well-structured codebase.
- Cheaper & faster changes: agents spend fewer tokens searching and more on the problem.
- Fewer silent breaks: agents break less without noticing.
- Faster debugging: agents find root causes faster.
1. Ask
Besides untangling the codebase, there are two other things you may do:
| Addition | What it does |
|---|---|
| Comments | Remove comments that only restate the code. |
| Tests | Remove tests that wouldn't catch a real bug, and fix the ones that fail without one. |
List these (with their exact descriptions) for the user and ask which ones to do. Then do only those.
2. List every problem
Don't change anything yet.
Go through the codebase asking one thing: should these pieces change together? Write down every place the code gets the answer wrong, as in the decision log example. There are only three ways to get it wrong.
Hidden links: pieces that must change together, where the code doesn't show it.
- Copies of logic or values: change one and the others must change too, but nothing says so.
- A module that reads a global or file set by another module: change that module and this one breaks, but nothing shows it (self-contained code).
- A feature split across folders by layer, or related code in different folders: one change needs all of them, but the layout hides that (domain-driven code, colocation).
- A call order or convention callers just have to know: change it and callers break, but nothing warns them (explicit over implicit).
Needless links: pieces tied together that change for different reasons.
- Callers that depend on internals: the internals change for their own reasons, and the callers break anyway (encapsulation).
- Stable code that imports something more volatile: the stable code has to change whenever the volatile code does (dependency direction).
- A general function with a parameter only one workflow understands: the function has to change whenever that workflow does (caller vocabulary).
- A shallow module: its interface changes whenever its internals do, so its callers change with it (deep modules).
Surplus: pieces with no reason of their own to change.
- Wrappers: they only change when the thing they wrap does (pass-through wrappers).
- Extension points and config for things that never vary: nothing will ever change them (speculative generality).
- Dead code: nothing uses it, so nothing will ever need it to change.
Also write down every special case: code that exists for one situation instead of following from how the system works. Each one is a requirement the design never accounted for, so use them to work out what the system needs to be.
- An for one value
if - A parameter for a specific caller
- A swallowed exception
- A retry or sleep
- A disabled test
3. Redesign from first principles
Work out the fixes as if the system had known today's requirements from the start. Look for the one fix behind several problems, which often looks nothing like them. Don't fix special cases one by one.
Ask the same thing of every fix, even one that looks better in the code: pieces that change together end up together, pieces that don't aren't tied, and everything depends on what changes less.
Write each fix above the problems it solves, as in the decision log example.
4. Fix everything on the list
Remove the special cases the redesign makes unnecessary, and keep the ones the system still needs. Don't go looking for bugs separately. A structural fix may fix some along the way, which is fine.
If the user chose Comments:
- Comments that restate the implementation, like , only change when it does, so they have no reason of their own to change. Remove them, even if they're phrased as a why.
// sort first so the binary search works - Comments about something outside the code, like , change when that does, and the code can't show it. Keep them.
// the proxy drops websockets, so we poll - Comments that explain a workaround om the codebase, like , only change when the problem behind the workaround does. They go when the redesign removes that problem.
// wait 100ms because the cache isn't ready yet
If the user chose Tests, do this last, because tests are your regression safety net. A test should fail on a regression, and only then:
- Tests that repeat the implementation only change when it does, so they have no reason of their own to change. They check the same interpretation against itself, so remove them.
- Tests tied to internals break when the code changes but behavior doesn't, so they change for the wrong reason. Rewrite them to depend on behavior, which changes less.
- Tests that can't fail pass whatever the code does, so no change ever affects them. They aren't checking anything, so remove them.
5. Check, and repeat until done
Have subagents search for special cases, hidden links, needless links and surplus. Do not stop until there is nothing worth fixing left.
You're done when:
- As few special cases are left in the system as possible. How many remain is the best measure of how close you are.
- A typical change lands in one place.
- Every import is reasonable, and nothing imports anything more volatile than itself.
- Subagents have no issues left that are worth fixing.
- Intended behavior is the same as when you started, apart from bugs the refactor fixed. Test this at the end.
Tips
- Start from a skeleton: script away the function bodies, so the whole codebase fits in context.
- Summarize with subagents: have them summarize parts of the codebase, so you can reason across all of it at once.
- Check the dependency graph often: script one that shows what imports what, and rerun it as you go.
- Enter where leverage is highest: start looking there first.
- Keep a decision log: compaction is less of a problem that way.
- Use Git as evidence: for how often pieces change, and what changes together.
<a id="decision-log-example"></a>Decision log example
The list of problems:
md
- `ui/LogoutDialog.tsx:30` waits 100ms after closing and hopes the message input can take focus by then.
- `ui/Toolbar.tsx:40` and `ui/Menu.tsx:22` call the same actions, each with its own check for when they're disabled.
- `checkout/submit.ts:9` depends directly on `payments/stripe.ts`.
- `shortcuts/keymap.ts:18` has a switch with one case per action, separate from the menu's list.
- `ui/CommandPalette.tsx:55` keeps its own list of actions, and it's missing the newer ones.
- `ui/SettingsDialog.tsx:61` sets a global `skipRefocus` flag to stop the logout dialog it opens from sending focus to the message input.
- `history/undo.ts:30` has a special case for every action that can be undone.
- `refunds/refund.ts:22` wraps every Stripe call in `if (order.provider === "paypal")`.
- Every action handler has to call `track()` by hand, and some don't.
- `ui/Toolbar.test.tsx` and `ui/Menu.test.tsx` test the same actions separately.Then with the fixes:
md
- Make the general `Dialog` give focus back to whatever had it before it opened, so the wait and the flag can both go.
- `ui/LogoutDialog.tsx:30` waits 100ms after closing and hopes the message input can take focus by then.
- `ui/SettingsDialog.tsx:61` sets a global `skipRefocus` flag to stop the logout dialog it opens from sending focus to the message input.
- Define a `PaymentProvider` interface that Stripe and PayPal both implement, so checkout and refunds no longer know which one they're using.
- `checkout/submit.ts:9` depends directly on `payments/stripe.ts`.
- `refunds/refund.ts:22` wraps every Stripe call in `if (order.provider === "paypal")`.
- Make every user action a command with a name, a handler and an undo, so the toolbar, menu, shortcuts, palette and undo history all run from one list.
- `ui/Toolbar.tsx:40` and `ui/Menu.tsx:22` call the same actions, each with its own check for when they're disabled.
- `shortcuts/keymap.ts:18` has a switch with one case per action, separate from the menu's list.
- `ui/CommandPalette.tsx:55` keeps its own list of actions, and it's missing the newer ones.
- `history/undo.ts:30` has a special case for every action that can be undone.
- Every action handler has to call `track()` by hand, and some don't.
- `ui/Toolbar.test.tsx` and `ui/Menu.test.tsx` test the same actions separately.对这个代码库进行一次彻底的重构,并将其完整执行到底。
将此视为一个目标。
为什么要做这件事?
- 输出质量更高: Agent 在结构良好的代码库中能编写出更好的代码。
- 变更成本更低、速度更快: Agent 用更少 token 搜索,把更多 token 投入问题本身。
- 更少悄无声息的破坏: Agent 更不容易在不知情的情况下造成破坏。
- 调试速度更快: Agent 能更快找到根本原因。
1. 询问
除了理清代码库之外,你还可以做以下两件事:
| 添加项 | 作用 |
|---|---|
| 注释 | 删除只是在复述代码的注释。 |
| 测试 | 删除无法发现真实 bug 的测试,并修复那些并非因真实 bug 而失败的测试。 |
列出这些选项(连同其确切描述)供用户选择,并询问要执行哪些。然后只执行选中的项目。
2. 列出所有问题
暂时不要做任何更改。
检查整个代码库时始终只问一件事:这些部分是否应该一起变更? 按照决策日志示例中的方式,记录代码中所有判断错误的地方。判断错误只有三种方式。
隐藏关联: 必须一起变更,但代码中看不出这种关系的部分。
- 逻辑或值的复制:修改其中一处后,其他地方也必须跟着修改,但代码中没有任何说明。
- 某个模块读取由另一个模块设置的全局变量或文件集:修改后者会使前者失效,但代码中看不出这种关系(自包含代码)。
- 按层拆分到不同目录中的功能,或分散在不同目录中的相关代码:一次变更需要同时涉及所有这些代码,但目录结构隐藏了这种关系(领域驱动代码、代码同置)。
- 调用方必须知晓的调用顺序或约定:修改后调用方就会失效,但没有任何警告(显式优于隐式)。
不必要的关联: 因为不同原因而被迫绑定在一起的部分。
- 调用方依赖内部实现:内部实现因自身原因发生变化时,调用方仍然会失效(封装)。
- 稳定代码导入了更易变化的内容:每当易变内容发生变化时,稳定代码也必须跟着改变(依赖方向)。
- 通用函数包含只有某一种工作流才理解的参数:每当该工作流发生变化时,函数都必须跟着改变(调用方词汇)。
- 浅模块:其接口会随内部实现一起变化,因此调用方也会跟着变化(深模块)。
多余部分: 自身没有变更理由的部分。
- 包装器:只有被包装对象发生变化时才会改变(透传包装器)。
- 为永远不会变化的事物提供的扩展点和配置:这些内容永远不会改变(推测性通用设计)。
- 死代码:没有任何地方使用它,因此永远不需要修改。
还要记录每一个特殊情况:没有遵循系统运行方式、仅为某一种场景而存在的代码。每一种特殊情况都是设计未曾考虑的需求,因此可以利用它们推导系统真正需要什么。
- 针对某个特定值的
if - 专供某个特定调用方使用的参数
- 被吞掉的异常
- 重试或 sleep
- 被禁用的测试
3. 从第一性原理重新设计
像系统从一开始就掌握当前所有需求一样推导修复方案。寻找能够解决多个问题的同一个修复方案,它通常与表面上的各个问题截然不同。不要逐个修复特殊情况。
对每个修复方案都要提出同样的问题,即使它看起来让代码更美观:应该一起变更的部分最终会放在一起,不应一起变更的部分不会被绑定,而所有依赖关系都应指向变化较少的内容。
按照决策日志示例中的方式,将每项修复方案写在它所解决的问题上方。
4. 修复清单上的所有问题
删除重新设计后不再需要的特殊情况,并保留系统仍然需要的情况。不要单独寻找 bug。结构性修复可能会顺带解决部分 bug,这没有问题。
如果用户选择了注释:
- 只是在复述实现的注释,例如 ,只会随实现一起变化,因此自身没有变更理由。即使注释以解释原因的方式表述,也要删除它们。
// sort first so the binary search works - 描述代码外部情况的注释,例如 ,会随外部情况一起变化,而代码无法呈现这种关系,因此应保留。
// the proxy drops websockets, so we poll - 解释代码库中某项变通方案的注释,例如 ,只会随变通方案背后的变化而变化。当重新设计解决了相应问题后,这些注释也应删除。
// wait 100ms because the cache isn't ready yet
如果用户选择了测试,最后再执行,因为测试是回归安全网。测试应当在发生回归时失败,而且只应在此种情况下失败:
- 重复实现过程的测试只会随实现一起变化,因此自身没有变更理由。它们只是用同一种解释验证自身,应当删除。
- 与内部实现绑定的测试会在代码发生变化、但行为没有变化时失效,因此它们因为错误的原因而需要变更。应将它们重写为依赖行为,而行为的变化频率更低。
- 无论代码如何执行都不会失败的测试,任何代码变更都不会影响它们。它们没有检查任何东西,因此应删除。
5. 检查并重复,直到完成
让 subagent 搜索特殊情况、隐藏关联、不必要的关联和多余部分。在没有任何值得修复的内容之前,不要停止。
满足以下条件后,即视为完成:
- 系统中剩余的特殊情况尽可能少。 剩余数量是衡量完成度的最佳指标。
- 一次典型变更只需落在一处。
- 每个 import 都合理,并且没有任何内容导入比自身更易变化的部分。
- subagent 没有留下任何值得修复的问题。
- 除修复的 bug 外,预期行为与开始时保持一致。最后验证这一点。
提示
- 从骨架入手: 用脚本抽掉函数体,让整个代码库都能装入上下文。
- 使用 subagent 总结: 让它们总结代码库的不同部分,以便同时对整个代码库进行推理。
- 经常检查依赖图: 编写脚本展示各项内容之间的 import 关系,并在推进过程中反复运行。
- 从杠杆最高处切入: 先从那里开始查找。
- 维护决策日志: 这样就不会太受上下文压缩影响。
- 把 Git 当作证据: 用它判断各部分多久变更一次,以及哪些部分总是一起变更。
<a id="decision-log-example"></a>决策日志示例
问题清单:
md
- `ui/LogoutDialog.tsx:30` waits 100ms after closing and hopes the message input can take focus by then.
- `ui/Toolbar.tsx:40` and `ui/Menu.tsx:22` call the same actions, each with its own check for when they're disabled.
- `checkout/submit.ts:9` depends directly on `payments/stripe.ts`.
- `shortcuts/keymap.ts:18` has a switch with one case per action, separate from the menu's list.
- `ui/CommandPalette.tsx:55` keeps its own list of actions, and it's missing the newer ones.
- `ui/SettingsDialog.tsx:61` sets a global `skipRefocus` flag to stop the logout dialog it opens from sending focus to the message input.
- `history/undo.ts:30` has a special case for every action that can be undone.
- `refunds/refund.ts:22` wraps every Stripe call in `if (order.provider === "paypal")`.
- Every action handler has to call `track()` by hand, and some don't.
- `ui/Toolbar.test.tsx` and `ui/Menu.test.tsx` test the same actions separately.然后列出相应的修复方案:
md
- Make the general `Dialog` give focus back to whatever had it before it opened, so the wait and the flag can both go.
- `ui/LogoutDialog.tsx:30` waits 100ms after closing and hopes the message input can take focus by then.
- `ui/SettingsDialog.tsx:61` sets a global `skipRefocus` flag to stop the logout dialog it opens from sending focus to the message input.
- Define a `PaymentProvider` interface that Stripe and PayPal both implement, so checkout and refunds no longer know which one they're using.
- `checkout/submit.ts:9` depends directly on `payments/stripe.ts`.
- `refunds/refund.ts:22` wraps every Stripe call in `if (order.provider === "paypal")`.
- Make every user action a command with a name, a handler and an undo, so the toolbar, menu, shortcuts, palette and undo history all run from one list.
- `ui/Toolbar.tsx:40` and `ui/Menu.tsx:22` call the same actions, each with its own check for when they're disabled.
- `shortcuts/keymap.ts:18` has a switch with one case per action, separate from the menu's list.
- `ui/CommandPalette.tsx:55` keeps its own list of actions, and it's missing the newer ones.
- `history/undo.ts:30` has a special case for every action that can be undone.
- Every action handler has to call `track()` by hand, and some don't.
- `ui/Toolbar.test.tsx` and `ui/Menu.test.tsx` test the same actions separately.