feat: asymmetry - separate impairment values for uploads and downloads - #180
Merged
Conversation
`decide()` reads the seven values from step 8 down (loss, corruption, duplication, latency, jitter, spike probability and spike length) from one value set indexed by `is_outbound`, the way `_bucket` and `_loss_bad` are already indexed. Nothing above the core uses it yet: with the switch off both directions get the same object, so the packet path cannot read whether asymmetry is on and there is no state there to get wrong. The shape was measured rather than picked. Against bench_decide.py on this tree (4750 ns/packet, impairing mix, spread 6.6%) one index plus seven attribute reads costs ~39 ns/packet, while indexing seven separate per-direction tuples costs ~172 ns. Both sit under the spread the rig can resolve. `decide()` gains no branch, which matters because it sits exactly on the mccabe ceiling of 27 pinned in pyproject.toml. The burst-loss chain moves with the values, and so does its reset. `p` is derived from the loss, so two directions losing different amounts need two chains. While they shared one pair of probabilities, re-deriving it reset both runs, so raising the upload loss ended the run the download direction was in the middle of: a direction nobody touched, losing its run because the other one moved. The run length itself stays symmetric on purpose. Both mutations were run, not assumed. Restoring the shared reset fails the new guard on "the download run survives a change to the upload", and deriving both chains from the download loss fails it on "with the probability its own new loss implies". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One switch (`asym`) and seven upload values reach the field registry, the settings model, the parser, the reproduction command and the session summary. They live in a card of their own rather than beside each field they mirror: the latency and impairment cards pair their fields on purpose, "Loss" next to the run length that shapes it, and interleaving an upload value into those rows would break every one of those pairs. The upload fields declare both `impairs` and the new `Field.live_when`, and each half alone is a real defect. Without `impairs`, `--asym --loss-up 50` cuts half of everything this machine sends, with no target and no deadline, and starts in silence, because the blast-radius warning looks at the download loss and that is zero. Without `live_when`, a value left behind after the switch goes back off warns about a run that touches nothing, which is not a corner case but the ordinary path of trying the feature and changing your mind. Both mutations were run against the new guards and both fail them. The flag names cost 18 working abbreviations (`--lat`, `--jit`, `--j`, `--cor`, `--spike-p` and longer forms), measured before the names were chosen, the same cost `--loss-burst` was allowed to charge for `--los`. Every full flag survives, so no stored reproduction command and no documented example moves. `--up-latency` would have cost one prefix and was refused: every other modifier in this parser reads noun-then-modifier, and `--up-` would also read as belonging to `--up`, the upload speed limit. Two ratchets took the routine door rather than moving. `settings_to_cli` crossed into the complexity crowd band, so its seven identical boolean branches became a table. The upload half of the summary went into its own function instead of into `settings_summary`, which sits one step below the ceiling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A profile file names only the keys it carries and the store fills the rest from each field's default, so a profile written before asymmetry loads whatever the design is. The question was never whether it loads, it is what it then means, and the existing legacy guard could not see this class: it flags a field zero-filled against a non-zero default, and every upload value defaults to zero. The new guard checks that the stored numbers ARRIVE and that both directions then carry them. Mutation run: reading the upload values regardless of the switch, which is the rejected design where absent simply means zero, turns a stored 7% loss into 7% down and 0% up and fails it. A second guard covers `asym` being the first bool in the profile scope, where everything is stored through float(). Mutation run: taking the switch out of the profile scope fails it. The fixture is built from the keys that existed before the change rather than read from the stored corpus, so it keeps describing the pre-asymmetry shape however that corpus is regenerated later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ticking the switch copies each upload field from the download field it mirrors, which is the whole reason the card needs no new rule explained: the second set of boxes is never blank, so nobody has to work out what an empty one would have meant. It describes the same link it described a moment ago, and only an edit changes anything. Unticking leaves the values alone, since they are inert either way and wiping them would punish somebody comparing against a symmetric run. While the switch is off those fields are disabled and their labels greyed, through the same `apply_overrides` that already did this for a field another field had taken over. An editable box that changes nothing is a lie about what the tool is doing, whichever of the two reasons makes it dead. The pairs come from the registry (`Field.mirror_of`), so a value added to the card later cannot be left out of the copy, and a new guard checks that every gated field names a switch and a mirror that both exist and agree on kind, bounds and unit. This also fixes a defect it exposed, which is a class rather than one field: both loops that fill the form from a preset or profile called number_string on every value, which held only while every profile field was a number. The switch went in as the string "0", which real tkinter coerces back to False by luck, and a profile storing it on would have arrived as "1" and worked for the same wrong reason. The conversion now dispatches on the field kind. Mutation run: restoring the uniform call fails the new guard. Both size ratchets fired and both took the routine door. app.py went five lines over its ceiling, and lowering that ceiling was not available - the crowd band is 70% of it and engine.py sits one line under the band - so the helper moved to fields.py beside off_value and the existing import absorbed it. `_on_switch` crossed the nesting crowd count, so the copy moved out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The outbound-only and inbound-only traffic filters are applied in the driver, so the other direction never reaches this process. With asymmetry on, half the values the user just typed then describe traffic the session cannot see, the summary still lists them, and the counters read as the tool ignoring its own form. It is said once per apply rather than refused, which is the answer the two LAN switches and the address-family pair already give: impairing one direction on purpose is a legitimate thing to ask for. This also moves seven mutation proofs out of prose and into the registry. Leaving them as sentences would be exactly the unguarded claim that file exists to prevent, and the registry test caught two of its own entries going stale on this branch: the repro entry aimed at an `if` that became a table row when the complexity ratchet fired, and the burst-loss entry named `_recompute_burst`, which became `_recompute` when the value sets went per direction. Both are re-aimed at the same statements. Run on this branch with `--changed origin/master`: 73 mutations, 73 caught, none survived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two problems the full suite found that the per-chunk runs could not. The entry ran to 143 words against the 100-word cap in test_no_user_facing_entry_grows_into_an_essay: this file carries the effect for a tester, and the reasoning belongs in CHANGELOG-INTERNAL.md, where it already is. The Changed heading had also been inserted in the middle of the Added list, which quietly moved two entries from the previous session (the refused-block checkbox and the connections_refused column) under a heading that describes neither. Both are additions and are back under Added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Until now only throughput was per direction. Latency, jitter, latency spikes,
loss, corruption and duplication were one number applied both ways, so a link
that is fast down and slow up - which is most consumer and mobile links - could
not be described at all.
What this adds
A new "Asymmetry" card with a tick box, and seven upload values behind it:
latency, jitter, spike chance, spike size, loss, corruption and duplication.
On the command line:
--asym, then--loss-up,--corrupt-up,--dup-up,--latency-up,--jitter-up,--spike-prob-upand--spike-ms-up. Profilesstore all of it, the reproduction command carries it, and the session summary
says what the upload half is doing.
A switch, not an empty box
The obvious design - leave the upload box empty to mean "same as download" - was
rejected, and so was the other obvious one.
Separate fields defaulting to zero silently change what an existing profile
means. A profile file names only the keys it carries and the store fills the
rest from each field's default, so a profile saying "latency 200" would load
without any error as 200 ms down and 0 ms up. The existing legacy-file guard
could not have seen it: it flags a field zero-filled against a NON-ZERO default,
and every upload value's default is zero. Reproduced as a mutation:
down=0.07, up=0.0for a stored 7% loss.A sentinel is invisible where it matters. An empty numeric box means zero
everywhere else in this program, and in a reproduction command the inheritance
would not appear at all.
So the switch decides, and ticking it copies the download values across. The
second set of boxes is never blank, switching it on changes nothing until you
edit something, and an existing profile keeps its meaning by construction rather
than by a migration somebody has to remember.
The measured cost, in full
The upload flags make 18 currently-working abbreviations ambiguous:
--lat,--jit,--j,--cor,--spike-pand the longer forms of each. This wasmeasured on the real parser before the names were chosen, and it is the same
cost
--loss-burstwas allowed to charge for--los.Every FULL flag survives, because argparse prefers an exact match over a prefix
one - so no stored reproduction command and no documented example changes. Only
a hand-typed abbreviation needs writing out.
--up-latencywould have cost one abbreviation instead of eighteen and wasrefused on purpose: every other modifier in this parser reads
noun-then-modifier (
--loss-burst,--spike-prob,--rst-cooldown,--flap-down,--nat-timeout), and--up-would also read as belonging to--up, the upload speed limit.Safety
--asym --loss-up 50cuts half of everything the machine sends. With no targetand no deadline that is a machine-wide impairment, and the blast-radius warning
looks at the download loss, which is zero - so the upload fields declare
impairsand would otherwise have started in silence. They also declare the newField.live_when, without which a value left behind after the switch goes backoff would warn about a run that touches nothing. Both halves are load-bearing
and each was proven by mutation.
A one-way traffic filter under asymmetry is now said out loud:
outandinare applied in the driver, so the other direction never reaches the tool and
half the values would describe traffic the session cannot see.
Verification
are new and cover this change; two existing registry entries had gone stale on
this branch and the registry caught both.
the switch honoured (0.00% both ways),
up_only49.91% up and 0.00% down,down_only0.03% up and 50.79% down - the pair that catches a swappedmapping - and round trips of +401.9 ms symmetric against +221.8 ms and
+221.3 ms for the two asymmetric shapes.
decide()gains no branch and sits where it did on the complexity ceiling.Against the packet-path benchmark on this tree (4750 ns/packet, impairing
mix, spread 6.6%), the per-direction read costs about 39 ns/packet, which is
under what that benchmark can resolve. Retention per packet is unchanged.
Deliberately not in this change
knowledge, but no number goes into a preset without a measurement or a source.
"Dropped" figure, so the asymmetry cannot be read off the screen directly
(delivered bytes are already per direction). That is a separate change.
🤖 Generated with Claude Code