When a memorised rule predicts the wrong result

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

• The pattern is shared, and so is the position where its last match ended.

  The common read: A regex literal is a value that describes a text pattern, so the result of test depends on the string it receives and on nothing from the call before it.

  The prediction failed on a version that matches the pattern. VERSION has a lastIndex field. A regex with the g flag reads that number before it scans and writes a new value after, so the first call matched to the end of 2.4.1 and left lastIndex at 5. The second call then started reading its five-character string at index 5, where the ^ anchor cannot match. The failure path resets lastIndex to 0, and that reset is why the third version returns true.

  If you write the rule down now, you will probably write it as a fact about one method: repeated calls on a global regex alternate. That sentence is true of these four calls, it predicts this transcript exactly, and it is the rule this screen gives you. In this example the compatibility check ran for four months. Nobody compared the versions it dropped against the manifest, and the ticket someone eventually filed blamed the check's ordering.

• The rule was about a method. The position belongs to the object.

  The common read: replace takes a line and returns a new line, so adding a redaction call next to an audit cannot change what the audit reports.

  The audit got worse when a correct function ran in the same loop. replace on a global regex sets lastIndex to 0 before it scans, and its own scan loop runs until the pattern fails, which sets the field to 0 a second time, so every redact call in the second loop leaves lastIndex at 0 for the next hasSecret call. Lines two and three therefore audit correctly, and line one does not.

  The audit of line one started at the 13 left by the first loop. Index 13 is the end of a thirteen-character string, where no key can start, so the audit returned false on a line holding a key. No code between the two loops touches SECRET. The rule the previous screen gave you matched the four versions, and it was a rule about test. lastIndex is not part of test. It is a field on the regex object, and both functions in this file write to it.

  The report from that run shows zero findings for the first line. That is a correct report of what hasSecret returned, and hasSecret is a correct wrapper around test. The review comment on this file asked whether the pattern belonged in a shared constants module.

• Adding a call that changes no audited line changed the audit result.

  The common read: The audit missed a line because the audit is wrong, so the fault is inside the function that missed it.

  Name what the audit prints for the third line when redact runs in the same loop. redact takes a string and returns a string. Its body contains no assignment and mutates no argument, and a reviewer can prove both of those from the one line it is written on.

  With redact interleaved the third line reads true; without it, false. The third line's result changed because the redact call changed a number that neither function names: lastIndex, the same field that made the compatibility check drop two versions at the start of this lesson. The audit code is the same in both runs. All three lines hold a key and no pass in either run reports all three.

  The postmortem for this miss says that the scan passed and the reviewer approved a one-line function, and both statements are accurate.

• Both functions are correct, and the file that combines them is wrong.

  The common read: Every defect is inside one function, so a review that reads every function carefully finds every defect in the diff.

  Name the function you would reject in review, then estimate what changing it would cost. hasSecret is a correct predicate: it tests the line against the shared pattern and returns the result, which is what its name says and all its body does. redact is a correct transform: it masks the pattern and returns a new string. Read either one alone and you write no review comment. Writing no comment is the accurate result of reading a correct function, not a failure of attention.

  The fault is in the pair, and the pair is twelve lines in one file with no other file involved: the flag, the two call sites, the loop order and the shared binding are all on the screen and the reader has seen every one of them. A reviewer who read every line correctly still cannot produce false, true, true from the lines alone, because the sequence depends on a value that no line of this file prints, names or assigns.

  A review is organised around functions and the diff, and the value that decides this file's behaviour is in neither. The retrospective blamed the process, and its action item was a lint rule against module-level regex literals, which would also flag regexes that are correct.

• The repair fixed the failure you had seen.

  The common read: The carried position is the fault, so removing the g flag removes the fault and the file is correct again.

  Removing the flag does repair the audit, for the reason you made the repair. Removing the flag also removes the replacement loop: under g the specification makes replace repeat until the pattern stops matching, and without g replace stops after one match, so a line containing two keys is returned with the first key masked and the second printed in full. One character decides which of the two failures the file has.

  Neither version reports an error. The change that introduces this bug is a one-character diff with a title like "fix regex statefulness". A reviewer approves the diff in under a minute, because the diff is one character and the reviewer can see all of it.

• A rule learned from a symptom is not a model, and it predicts the wrong result with confidence.

  The common read: I know the mechanism now, so I will catch the next file that has this bug.

  A rule learned from a first encounter describes the symptom that encounter showed. The symptom on the four schema versions was alternation, so the rule reads: repeated calls alternate. The loop that audits and then redacts broke that alternation on its third line, and the one-character repair failed inside replace, which the rule never mentions. A rule matches the symptom it was learned from, not the state that produced the symptom.

  A held model is smaller than the rules it replaces. One mutable field is stored on the regex object, both operations write to it, and the value at any call site is whatever the last write left. That sentence predicts the compatibility check and both audit loops, and it fits on one line. It does not predict the one-character repair. The repair needs one more fact about the same flag: under g, replace repeats until the pattern stops matching, and without g it stops after the first match.

  A partial model is worse than no model in one way only. With no model the reader is unsure and runs the code. With the rule, the reader predicts alternation, does not run the code, and is confident and wrong, and a reviewer who is confident and wrong approves the change. The postmortem note says the team had seen this class of bug before. The note is accurate, and that earlier encounter is why the team trusted the rule and did not run the code.

• The question that finds this bug is asked before you read the code.

  The common read: I missed the bug because I read too fast, so the repair is another pass over the same lines.

  Name the state before you read the lines. One question produced every answer about the audit, and it is one sentence long: which values here outlive a call, and what writes to them. SECRET binds one module-level regex object, lastIndex is a mutable field on that object, and both test and replace write to that field. Answering the question takes about as long as reading the file once, and you can answer it before you read the first line of either function.

  Another pass over the lines does not produce that answer, because the answer is not in a line: every regex object has a lastIndex field, the g in the literal makes test and replace read and write it, the two call sites depend on its value, and no statement in this file names the field, assigns to it or prints it. The reading that finds the bug starts from the regex object and asks how its fields change over time.

  Carried state is this course's phrase and not a specification term: a value that outlives the call that touched it. The review comment that would have caught this bug reads: what does lastIndex hold when the second loop starts? That comment is a question rather than a finding, and a review interface has no field for it.

• Predicting costs one line, and it is the only step whose result can prove you wrong.

  The common read: Reading a diff carefully is how I check it, so a careful read and a prediction are the same act done at different speeds.

  Estimate what the check costs before deciding where to use it. You already know how to do the check: write down what the code prints, run it, and read the difference. These twelve lines show where the check is worth the minute it takes, because a careful read of them produced a confident wrong answer on the one-character repair, and the prediction is the only step in this review whose result can show you are wrong. Reading cannot show that, so the two acts differ in kind, not in speed.

  Predicting the output of every diff costs more minutes than a review has. Use it where a wrong answer is expensive and where the code carries state across calls: a shared module-level value, anything holding a position or a cache, anything whose second call could differ from its first. Everywhere else, read the code and continue. The recommendation on this file was more time per review, and more time on these twelve lines produces the same wrong answer.

  The ticket was titled "intermittent scanner false negatives". Every word of that title is accurate except "intermittent".