实习日志 / English / Code Audit / How many failure scenarios are there, really? Cleaning up silent fallbacks

How many failure scenarios are there, really? Cleaning up silent fallbacks

中文 | English

On 3 October, the same day the CI audit finished, I asked the Agent to audit origin/main for redundancy and dead code. The first version of the list had five groups: code with no references at all, code called only by tests, old versions replaced by new ones but still on the main path, duplicate implementations, and tests that were already broken without anyone noticing.

The third group stood out most. All 4 profiles in the live config/model-runtime-bindings.json pointed to northwind-fnol-claimant-v8, yet in the code v6 and v7 were still the defaults:

# backend/core/config.py
model_prompt_version = 'northwind-fnol-claimant-v6'   # when MODEL_PROMPT_VERSION is not set

# backend/services/prompt_composer.py
'v8' if prompt_version == CLAIMANT_V8_PROMPT_ID else 'v7'   # any value other than v8 silently picks the v7 pack

model_agent.py also still held a v6 branch of about 320 lines, unreachable in production and not guarded by any assertion. The concrete consequence: leave one line out of the local .env and the backend still starts, the page still chats, but the model gets the v6 instructions from mid-September and behaves completely differently from the demo, and whoever is debugging is unlikely to think of this layer.

Backward compatibility is a bad habit of coding Agents like Codex. How many failure scenarios are there, really? And these fallbacks were silent on top of that.

Evidence can only come from code

The first version judged one script not to be dead code because "the README describes it", and based several items on "not mentioned anywhere except one doc". That does not hold. An Agent changing code is just as free to change docs; in a repository where everyone uses an Agent, docs and code are written by the same hands, and a doc saying something is still in use only shows that someone wrote so in some commit. Whether code is still used can only be answered by the code itself; for anything to be deleted, trace it to the PR that introduced it and confirm that deleting it is really safe.

The second group, "production code called only by tests", could not simply be deleted either. If code that only serves tests cannot just be moved into a test file, it may have some other role. Take assert_retry_is_permitted, a 93-line "is this retry allowed" check that the real retry path never called. Either the retry path was missing a check, or the check had been rewritten elsewhere, and both had to be understood before touching anything. So the second version set this whole group aside and left it out of this cleanup.

Searching again by responsibility

The second version changed method. Every item was first traced to the PR that introduced it and then to the commit where its last caller disappeared, to confirm deleting it was safe, with docs no longer used as evidence. The first version had only searched for strings with version numbers; the second searched by responsibility: an AST structural comparison of every backend function to find responsibilities written twice; a line-by-line read of 238 places marked fallback|legacy|compat|shadow|deprecated|backward|predate|degrad, excluding normal features such as configuration rollback and "S3-compatible"; a scan of every except that neither raised nor logged; and a check of every runtime configuration environment variable with a default.

The most dangerous kind at runtime was "quietly use the old implementation when nothing is configured":

Some of these defaults had been there since the prototype stage in August; others were left behind by the successive changes from v6 to v8: at each change, the previous version was casually kept as "the fallback when nothing is set".

The model path itself fails closed: FeatureControlledAgent falls back to UnavailableAgent and never quietly downgrades. The whole problem was "what runs when nothing is configured". The current start script start-dev.ps1, .env.example and the compose file all set configuration explicitly, so these defaults normally never take effect, and precisely because they normally never take effect, nobody notices when they do.

Version changes exist to replace the old; falling back to the previous version when configuration is missing just hides the problem. The fix is simple: refuse to start when configuration is missing, with an error that says which item is missing and what it should be. A configuration problem exposed at the moment of startup is more robust than one hidden by falling back to an old version.

The upstream of compatibility layers

The second kind was compatibility layers: accepting old-shaped model output, translation tables for old action codes and old tool names, and aliases kept only for old import names. My judgement then was that the compatibility layers were still useful in themselves, but they existed because old code was still emitting old fields. So the way to clean up was to find the upstream emitting the old fields:

Where the upstream was gone, delete directly. Where it was still there, change or retire the upstream first and delete the compatibility layer in the same PR; deleting only the layer just causes errors. ControlledAgent was a nuisance: almost every conversation test used it as a fake model. My decision was "keep the keyword bot for now", and its set of compatibility layers stayed with it, untouched this time.

Old APIs followed the same idea. POST /api/claims/message, which always replied "I received your claim.", was used by none of the three front ends and was deleted directly. The lenient path that allowed profile edits without a revision could not be deleted directly: the client's useProfileController.js itself said "submit without a version if the account has no revision". As long as the front end kept the old path, the backend had to keep that door open, and two people editing a profile at once could overwrite each other. So the front end was changed first to always send the version, and then the backend path was deleted.

See what it does before deleting it

The function prompt_projection was judged an orphan, and the first round's explanation was: after #1092, "which fields can be filled this turn" was no longer written into the prompt but into the schema the reply must follow, so the model could only fill fields listed in the schema; the new approach was more reliable, and the old function was not worth fixing. That explanation did not survive checking. Going back through the code and commits, the timeline was: on 29 September #1092 removed prompt_projection from the prompt, and from that day the model could not see each field's type and allowed values; on 1 October #1159 found the problem, with the commit title "give v8 turns their enum values", and wrote a new writable_fields_projection to put the list back into the prompt; the same PR changed the reply format to a fixed answer sheet, where the schema lists every writable field but no longer narrows them per turn, and which fields may be filled this turn, and whether their values are right, is checked item by item by the Runtime after receiving the reply, with any invalid item dropped and recorded.

So the real division of labour is: the prompt tells the model what it can fill, and the model judges; the schema only guarantees a complete structure; the Runtime checks afterwards. The verdict on prompt_projection changed from "delete" to "deduplicate": it was an older duplicate of writable_fields_projection.

For the front end computing progress on its own, the first round suggested deleting it outright: on the claimant page, "x of y completed" was counted by the front end from the form whenever the backend did not provide it, which amounts to a second set of rules for "what counts as complete". The direction was right, but deleting the front-end code presumes the backend already has the matching rule; delete it while the backend still has a gap, and the gap lands right in front of the claimant. Checking showed the backend did have a gap. Field evaluation ran after more than twenty kinds of event, sending a message, editing the form, uploading evidence and so on, but searching the repository for recomputation_reason found no claim_created. Between a new claim being created and its first message, the backend could not provide progress, and the front end's own count was filling exactly that gap. So the order became: first have the backend run field evaluation once at claim creation too, then delete the front-end code.

In the admin console's "create release configuration" form, the "runtime environment" field opened with fixture preselected. Leaving it empty to force a choice was not right either; the default should be whatever the server is running right now. The backend's GET /health/ready already returns the current data_runtime_profile, so the page just reads it once on opening.

A parameter changed by hand

Fixtures were originally simulations designed for tests and the prototype stage, and later were used for a long time as if they were the real implementation; apart from tests, they should not exist any more. That covered the in-memory store, the in-memory identity store, the in-memory knowledge base, and branches in app.py like if profile is not FIXTURE that let fixtures skip real components.

Developer mode was the same kind of problem: at startup, a hand-edited parameter decided between development and business environments. The switch bundled three unrelated things: accepting a few fixed fake tokens (Bearer synthetic-claimant, synthetic-staff, synthetic-admin) without login; storing customer and staff accounts in memory; and storing all 7 of the admin console's stores in memory. It could only be turned on when NORTHWIND_ENVIRONMENT was development or test, but that variable defaulted to development when not set. Forget to set the environment on a server, turn on developer mode by mistake, and the fixed fake tokens could log straight in. This was the same problem as "silently use a default when configuration is missing", just landing on identity.

Startup actually had four switches, each with a default:

NORTHWIND_ENVIRONMENT    defaults to development
NORTHWIND_IDENTITY_MODE  developer enables fake tokens and in-memory storage
DATA_RUNTIME_PROFILE     defaults to fixture
AGENT_RUNTIME_PROFILE    defaults to controlled

Leave any of them out and it silently takes effect. The environment does have to be specified explicitly, since a program cannot guess where it is running; but there should be only one switch, with no default, and the program should refuse to start without it. Test-only things such as fake tokens and in-memory storage are injected by the tests themselves in conftest.py, which already injects FixtureRepository.

Staff accounts were handled at the same time. We had not built staff registration and only allowed existing accounts to log in, so staff accounts needed to persist in the database and be printed at startup. In normal mode staff accounts were in fact already stored in SQLite; the gap was that a freshly cloned environment had no staff account at all, and startup did not print which accounts could be used. The plan: keep a staff seed list in the repository and create the accounts at startup if the database does not have them; keep passwords out of the repository, reading from .env, generated by start-dev.ps1 on first start; print email, display name and role at startup, but not the password itself, because the database holds only the hash and logs end up in CI output and terminal history.

In the end it was split into three PRs, from lowest risk to highest: first delete compatibility layers whose upstream was gone and the dead code, archiving the old prompts and bindings; then remove every fallback and refuse to start when configuration is missing; finally move developer mode and fixtures out of the runtime, adding staff seed accounts and field evaluation at claim creation. The last two changed how things start, so after merging everyone's local .env needed the new required items.

Review is the same

The same day dealt with another fallback. Reply review had by then switched to a small model acting as judge (Haiku 4.5, through OpenRouter), checking whether this turn's reply said anything wrong, such as verbally pushing a service or claiming a handoff to a person. When the judge did not run, the system fell back to a set of English regexes in reply_consistency.py. Three conditions triggered it: no judge model configured; more than 20 seconds already used in the turn, leaving too little for a review call; or the review call erroring or returning incomplete output.

The review on #1192 questioned this path, and the claim at the time was that lookup turns often fell back to regexes for lack of time. We split all backend logs from the local real-model runs of 1 to 3 October by turn and counted, per turn, the requests sent to OpenRouter and the main model calls:

The fallback to regexes never happened once, and "often" had no basis. By the logic of "how many failure scenarios are there, really", this fallback was almost never needed; but whenever it did trigger, it replaced semantic judgement with literal matching, so that "I've passed your question to the claims team" would be read as "claims to have handed over to a person". Rarely triggering is no reason to keep it.

So it was settled: review is no longer skipped for lack of time; it has its own 20-second timeout and runs as normal after the main call completes. When the review model is unavailable, the reply goes through, with a note "not reviewed this turn" and the reason, no rejection and no recovery. The fallback regexes were deleted as a group. Every turn records reviewed, skipped or failed, so future statistics need not be inferred indirectly from logs.

Time parsing was another face of the same illness. At first the model had no access to the current time, so "20 minutes ago" was stored as PT20M or current_approx, and the Runtime wrote a regex to parse it itself. Later the reporting time reported_at_local went into the model's context, but the regex stayed. Getting the time is the job of one small function and needs no regex and no review. The regex was deleted, the model converts the time itself, and the Runtime only checks format and range: not later than now, not earlier than two years ago. If the check fails, the customer's own words are stored with precision marked "approximate".

Into the rules

The audit ended by recommending three rules for AGENT.md, applying to every person and every Agent:

• no default fallback when runtime configuration is missing;
• when a versioned artefact is replaced, the old version must be archived and code may not reference it;
• no aliases or compatibility layers kept for "old callers", unless the PR states who the caller is and when it will be removed.

The three guard against the same habit. When an Agent changes code it always wants the old thing to keep working, so every change leaves a way back, and each way back looks considerate on its own. But with five people on the team each using an Agent, old versions, old fields and old APIs pile up layer by layer, and nobody knows what the system is actually running when "nothing is configured". Version changes exist to replace the old; when something fails, let it fail loudly, and you can find where the problem is.