i've spent the last while sending small fixes to bigger repos: langchain, vite, bat, mistune, indicatif, a few others. different languages, different domains, maintainers who are way better at this than me. and the thing that surprised me is how few distinct bugs there actually were. it's not a hundred different mistakes. it's the same handful of shapes, wearing different clothes.
so this isn't a "look what i found" post. it's the shapes. once you can name them you start catching them before you go looking. here are five i keep running into, each with a real PR, why it survives code review, and the one check that would have caught it.
1. the argument the producer never sets
langchain's anthropic file tool crashed on every rename. _handle_rename read the source path like this:
def _handle_rename(self, args):
src = args["old_path"] # KeyError: 'old_path'
dst = args["new_path"]
but the code that builds args never puts an old_path in there. it stores the target under path and the destination under new_path. and every sibling handler, _handle_view, _handle_create, _handle_str_replace, _handle_insert, _handle_delete, already reads args["path"]. rename was the only one reaching for a key that doesn't exist, so any rename hard-crashed with KeyError: 'old_path'.
the fix is boring, read the key the tool actually produces:
src = args["path"]
dst = args["new_path"]
why it survives review: there was a passing unit test. it called the handler directly and handed it the phantom key by hand.
_handle_rename({"old_path": "a.txt", "new_path": "b.txt"}) # green, but the system never builds this
the test was green because it constructed the input the function wished it got, not the input the rest of the system actually sends. a happy-path test that builds its own inputs can hide a feature that has never once worked.
the check: when a function reads args["x"], go find where args is built. if nothing sets "x", that's your bug. and write the test against the real caller, not against the shape the function is hoping for.
(PR: langchain-ai/langchain#38488)
2. the lazy default that swallows a real value
this one has two faces. the classic is a truthiness check used where you meant "is it set":
// drops a legitimate DEFAULT 0
const clause = defaultValue ? `DEFAULT ${defaultValue}` : '';
0, '', and false are all real values, and all three skip the branch. DEFAULT 0 silently becomes no default at all. the fix is to ask the question you actually meant:
const clause = defaultValue !== undefined ? `DEFAULT ${defaultValue}` : '';
the other face is lossy formatting. indicatif's HumanFloatCount rendered with {:.0}, which rounds to zero decimal places and throws the fraction away. 1234.9 printed as 1,234. same disease as the truthiness one: a "tidy" default path that quietly discards information.
why it survives review: falsy is not the same as absent, and the convenient default looks correct on every value you happened to test. you tested 5, not 0. you tested 1200, not 1234.9. the bug lives in the one input you didn't bother typing.
the check: always test the falsy member of the set, 0, "", false, and at least one fractional number. if a code path can't tell "missing" from "present but zero", it has this bug.
(PR: console-rs/indicatif#816)
3. unsigned width math with no floor
bat lays out its language list with arithmetic like this:
let desired_width = config.term_width - longest - separator.len();
run bat --list-languages --terminal-width 1 and term_width is smaller than the things you're subtracting. on usize there is no negative side to land on. debug builds panic outright, release builds wrap to an enormous number and quietly lay out the wrong width. the fix is saturating_sub, which also happens to match what a previous fix already did for the sibling renderer right next door.
let desired_width = config.term_width
.saturating_sub(longest)
.saturating_sub(separator.len());
i hit the same u32 underflow shape in imageproc, in oriented_fast's edge_radius, just in image space instead of terminal columns. and a close cousin in the image crate's ICO decoder: it rejected images with more than 256 color planes even in lenient mode, while the sibling field had already been relaxed to strict-only. not arithmetic, but the same family, an edge case slipping past a guard because two parallel fields drifted apart.
why it survives review: "this width can't realistically be 1," plus unsigned types give you no warning when you're wrong. and when you add a strict/lenient knob it's easy to wire it into one check and miss the one sitting right beside it.
the check: any a - b on an unsigned type that touches a user-controlled size, reach for saturating_sub and test at 0 and 1. any new strictness flag, grep every validation in that struct and confirm each one actually reads it.
(PRs: sharkdp/bat#3812, image-rs/imageproc#793, image-rs/image#3056)
4. the unicode corner ascii never reaches
text rules look simple until someone hands you a character you don't type by hand.
mistune mis-rendered *foo***bar*. commonmark has a rule that emphasis delimiter runs whose combined length is a multiple of 3 need special handling, and the rewritten inline engine was checking that against the wrong run length. plain *foo* was fine, so it shipped. the broken case is the one with stacked delimiters that you'd never write on purpose but a generator will.
in wenmode, the flanking logic (deciding whether a * is allowed to open or close emphasis) treated unicode combining marks and format characters as punctuation. so emphasis wrapped around accented or composed text behaved as if there were punctuation in the middle of a word, and broke.
why it survives review: ascii passes, so it feels done. the spec's corner cases only bite on input outside the keys under your fingers.
the check: differential test against a reference implementation. for mistune i triangulated against markdown-it-py and commonmark.py, and a differential fuzz went from 26 mismatches down to 0. you don't need to re-derive the spec in your head, you need a second implementation that's willing to disagree with you.
(PRs: lepture/mistune#458, lepture/wenmode#1)
5. malformed input handed straight to a decoder
vite's html middleware decoded the request url with no guard:
const pathname = decodeURIComponent(url);
ask for /%c0.html and decodeURIComponent throws URIError: URI malformed, which takes down the middleware instead of falling through to a normal 404. the fix mirrors a guard that already existed on a sibling path, wrap it and move on:
let pathname;
try {
pathname = decodeURIComponent(url);
} catch {
return next();
}
why it survives review: "it's a url, urls are valid." anything that comes off the wire can be malformed, and decodeURIComponent, JSON.parse, and atob all throw on bad input rather than handing you back an error value you might remember to check.
the check: every decode or parse of input you didn't create gets a try/catch and a defined fallthrough. then throw a couple of deliberately broken strings at it and confirm it bends instead of breaking.
(PR: vitejs/vite#22781)
how i actually find these
honestly, mostly one boring habit. when a maintainer merges a fix, i look at the code right next to it, the other handler, the other field, the other renderer, the parallel middleware, because the fix usually lands in one spot while the same shape is still sitting in the one beside it. i wrote that method up on its own if you want the mechanics, but i think the patterns above are most of the value. the finding part gets easy once you can name the shape.
and to keep myself honest: some of these are merged (mistune, indicatif, image-rs, vite), some are still open (bat, imageproc, wenmode), one got auto-closed on a process technicality (langchain). the links are all there, so don't take my word for any of it, read the diff and decide whether the diagnosis holds.
— greymoth (@greymoth__)
