The bug that is not on the screen

A lesson from Engineering Judgment When AI Writes the Code. Play it above, or read it through below.

• Both approvals were right. The run's output contradicts both of them.

  The common read: Each file is correct on its own, so the behaviour the two of them produce together is correct.

  Neither approval was a mistake. cache.mjs is correct for the contract its own docstring states, and pricing.mjs reads the currency from request context, validates it, and refuses a currency it cannot price. Try to point at the line to change in either file: there is no such line. The five numbers came from the way the two files work together, a kind of defect you have met once before, inside one file. Here the line that combines them is in a third file, and neither review opened that file.

  memoize builds its key from the argument list it receives. quote takes one argument and reads two inputs, so the key contains the SKU and not the currency. The first request for seat-pro arrived in USD, ran quote once, and put 4900 into the Map under ["seat-pro"], and every later request for that SKU received the stored 4900 from the Map.

  Nothing on that run is an error, and nothing about the output looks unusual: five plausible prices for five requests. Eventually a ticket reports that currency conversion is wrong for European customers, and the engineer assigned to it spends the first hour reading pricing.mjs, which is the one file in the service that converts currency correctly, on every input it is ever given, and which has not changed since the day it was written.

• A cache key assumes it lists everything the function reads.

  The common read: A function's inputs are the values in its parameter list, so a cache keyed on the arguments is keyed on everything the function reads.

  Name every input quote reads, then count how many of them appear in its signature. Two are read and one appears. sku selects the list price and is a parameter. currency selects the rate and comes from the store that currentRequest reads, which the caller set for this request and never passed as an argument. The pull request that added the wrapper had one review comment: whether the Map should be bounded, a question about memory use and not about the input the key leaves out.

  The signature lists the inputs somebody decided to pass, not every input, and a generic wrapper cannot see the inputs nobody passes: request context, module-level state, the environment, the clock, the locale, and whichever tenant is current. Any wrapper keyed on an argument list has the same limit and cannot detect it, because a function that reads a value outside its parameter list has the same signature and the same type as one that does not.

  The docstring on memoize asks for a pure function. That requirement is the contract memoize states, and the function passed to it does not meet it: quote does no I/O, allocates nothing and reads no database, and it still returns two different values for one argument. The comment that would have caught this bug fits on one line, and it asks what the function reads outside its parameter list. Ambient input is this course's term for the inputs outside the parameter list, and it is not documented vocabulary.

• An issue stream is a list of what threw. It is not a list of what went wrong.

  The throw happened on the request whose SKU nothing had cached. seat-pro returned 4900 for a currency the service cannot price and seat-team threw, and what decided which one threw is whether an earlier request had asked for that SKU. A tracker captures two kinds of event: uncaught exceptions and unhandled promise rejections, which it catches automatically, and anything the code reports by hand. A wrong number returned with no throw and no manual report does not produce a smaller event than a crash; it produces no event.

  The run made five requests and returned five prices, and a counter on every throw handler reads zero. The dashboard was accurate all week: it reported what threw, and nothing threw. The postmortem, filed under gaps in monitoring coverage, says the error rate stayed flat through the incident and lists an action item: add alerting on the pricing path. The error rate could not change, because the service responded to every request it received, in well-formed JSON, with a number that looked like a price.

• The diff that caused this bug is one line, and it is in neither file you read.

  The common read: The diff is the change, so a careful reading of the diff is a careful reading of the change.

  Read this diff and name the behaviour it changes. It adds an import, a comment and one binding, and it replaces quote(sku) with cachedQuote(sku), a call with the same argument and return type. Nothing in it mentions currency, request context, validation or correctness, and every claim it makes is true: pricing is hot on that path, the table does change at most daily, and memoize returns the same value for the same arguments. A careful review and a careless review of this diff both find nothing to change.

  The behaviour change is not in the diff. A wrapper's effect depends on the function it wraps, so the shorter the wiring line, the less behaviour the reviewed text shows, and the two files that show the risk are unchanged and not in the diff. The incident doc records that the change was generated rather than typed, which is true and explains nothing: the same line typed by hand behaves the same. The wiring diff is this course's name for this kind of change, not documented vocabulary.

• Every dimension you record says the two workers run the same build.

  The common read: Two processes of one build are interchangeable, so a price that differs between them means the data differs.

  The recorded fields show which worker answered, but none of them shows why the two answers differ. A server-side tracker SDK can attach the host name to every event, so a dashboard can put 4508 next to worker-a and 4900 next to worker-b. The release, environment, commit, rate table and request are equal on both workers. The prices differ because each process stored the currency of the first request it served, and no log or event records that order.

  An event carries the dimensions somebody named, and you can filter on those and on nothing else. Someone chooses them before the incident, without knowing which incident will happen, so the most useful dimensions are the ones that describe inputs rather than outcomes: the currency in scope, the tenant in scope, the flag state, the process that answered. A dashboard cannot group by a value that never left the process.

  The team splits the dashboard by release, sees both workers on the same release, and concludes that the deploy is clean, which is true. The review comment on the eventual fix asks for more logging, and more logging of the same fields would have produced twice as much evidence for the same wrong conclusion, because the field that explains the difference, which currency each process served first, is not among the fields anyone is logging.

• When the service does throw, the stack shows the code that detected the mismatch.

  The common read: The top of the stack is where the fault is, so the file named on the issue is the file to fix.

  Name the files you expect on the stack when the nightly re-pricing job refuses to settle a row it computes as mispriced. The job re-prices every charged row from the rate table and throws when a row disagrees. The stack trace lists settlement.mjs, then reconcile.mjs, then Node's own module loader. cache.mjs, pricing.mjs and handlers.mjs appear in none of the frames, and the call that produced 4900 returned before the job printed the ledger it is now checking.

  The stack trace is not defective. A stack lists the calls open when the throw happened, and the call that produced the number returned long before any code detected the mismatch. Any mechanism that stores a value for later use in other code has the same effect: a cache, a queue, a scheduled job. The trace shows where the wrong value was detected, not where it was produced, and the issue is opened against the one file in the chain that checked the value.

• Grouping reads each event alone, the way the two reviews read each file.

  The common read: One issue is one cause, so the issue count is the count of things wrong.

  Predict how many issues a week of that settlement job opens when three currencies are mispriced across two SKUs. When an event has a stack trace, default grouping uses the exception type and the stack frames and ignores the message. Every one of those rows throws AmountMismatch from the same line of settlement.mjs through the same calls, so they land in one issue with one title and a count. Without a stack trace, the tracker groups by type and message, and a varying message splits the issue.

  The single issue is correct under the rule grouping applies, which is whether two events look alike at the point where they were thrown. Grouping performs the same local reading the two file reviews performed at the start, by machine, over every event, and it is right for the same reasons and misses the same input. The currency is not on the issue unless somebody attached it, so the title reports that an amount disagreed and the count reports how often.

  Key name arguments. Function read more than arguments. Key wrong, no error. Two file good. Two file together, not good. Nobody write that down. Stack show who check number. Stack never show who make number. Rule stay same: input you read, input you name. In key. On event.

• The question that finds this bug fits on one line.

  The common read: The bug is a memoize problem, so not memoizing avoids it.

  If you wrap a function whose inputs you have not listed, the wrapper's key assumes an input list you never checked. The retro's action item is a lint rule banning memoize on the checkout path, which bans the wrapper and leaves the key unexamined. The question that finds this bug is what the function reads that is not in its parameter list, and you can answer it by opening the file and looking at what is in scope. In quote, the first line gives the answer.

  Look for two patterns. Both are relationships between pieces of code, not defects in one piece. The first is a cache, memo, dedupe or batch key built from an argument list over a function that reads request context, module state, the environment, the clock or the current tenant, where whichever caller arrives first decides the value every later caller receives. The second is any change whose diff is one line of wiring, because one line of wiring can change behaviour that lives in files the diff does not touch.

  The repair is to name the input. Giving quote a currency parameter and having the handler read it from context and pass it puts both values in the key, and the same five requests price at 4900, 4508, 769300, 2668 and 2900. A sixth request, in GBP, now reaches quote, and the guard throws. The cost is one parameter and one line in the caller for every wrapper you write, and the change prevents a defect that no local reading of either file can find.

• Nothing in this lesson was a defect in a file.

  The common read: These numbers were wrong, so there is a defective file to find.

  Name the file to fix and the fix that goes inside it. There is no such file, and every reading this lesson performed on one file at a time was correct, including the two file reviews at the start. The incident doc names pricing.mjs as the cause. You can now predict what the second currency receives, that a guard stops running once something is cached, that two workers of one build answer one request with two numbers, and that the issue names the frame that detected the mismatch.