fix(core): let an impairment outlive the flow record that implements it

CHANGELOG-INTERNAL.md

@@ -42,6 +42,68 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre
 - Help text and the flag tables in both READMEs now state that the flag is valid on its own.
 - Version bump deliberately NOT taken (convention 34): the owner closes it in `VERSION.txt`.
 
+### Fixed: two tooltips that described counters they do not describe (audit F8)
+
+Both in `lang/en.json` and `lang/pl.json`, both user-visible, neither catchable by a test - this is
+the failure mode rule 5 is about: true-sounding prose next to correct code.
+
+* `tips.stat_loss` said "Packets dropped because of the configured Loss (or link outages)". Link
+  outages have had their own counter (`drop_flap`) since they stopped inflating "Dropped", and
+  `tips.stat_flap` says so in the next cell: "Counted separately from loss". Two tooltips
+  contradicting each other, with the wrong one attached to the number a tester reads first.
+* `tips.data_down` promised "Hovering also shows how much the app tried to download".
+  `add_tooltip(widget, key)` renders one static translated string (`gui/tooltip.py`); there is no
+  dynamic path and never was. The offered figure exists only as `metrics.offered_mb` in the repro
+  report - and note it is the SUM of both directions, so it is not "how much the app tried to
+  download" either. Replaced with something true and useful in its place: dropped packets are not
+  counted in this figure, which is the difference between it and the connections table's bytes.
+
+No code change, so no new test; `test_i18n_coverage` already pins key parity (465 keys, identical
+sets) and `test_no_em_or_en_dashes_in_repo_text` the punctuation.
+
+### Fixed: an impairment no longer expires when its flow record does (audit F2, and F1's tail)
+
+`_reset_until` and `_flow_last` are `_FlowTable`s that retired a generation every `FLOW_ROTATE_S`
+(30 s), so a record survived 30-60 s - and both tables hold the state that IS an impairment. Two
+settings were therefore capped by a constant nobody had connected them to:
+
+* **`rst_cooldown`** accepts up to 3600 s. Measured at 120 s, the same flow was reset again after
+  30.3, then 60.8, then 60.8 s.
+* **NAT expiry** was worse than capped. A retired record reads back as "never seen", so the next
+  inbound packet reopens the mapping with nothing sent. Measured: a 5 s timeout blackholed for 20 s
+  and then passed traffic; **a 30 s and a 120 s timeout dropped ZERO packets**, because the record
+  died at the rotation just before the first inbound packet arrived.
+
+`_FlowTable.keep_for(seconds)` raises the age window; `set_rst` passes the cooldown, and `set_nat`
+switches ageing OFF for `_flow_last` (`float("inf")`) while NAT is on. Matching the window to the
+timeout is not enough for NAT and was measured failing exactly as above - the record has to outlive
+the timeout AND the blackhole after it, and that blackhole is meant to last until the application
+sends. The RST cooldown is a fixed span, so there the window is simply the cooldown.
+
+**Why raising the window is safe, and the property that says so:** the SIZE ceiling is enforced on
+every write, independently of the age window, so a longer window makes the table older and never
+bigger. Measured through the real setter: 250k flows with `nat_timeout=3600` peaked at 199,999
+against the 200,000 ceiling.
+
+After: cooldowns of 60 / 120 / 300 s reset at exactly those intervals; a NAT blackhole held for the
+full 900 s of the probe at 5, 30 and 120 s timeouts (1800 packets dropped) and ended on the first
+outbound packet, which is the documented contract.
+
+New tests in `tests/test_core.py`:
+`test_a_long_rst_cooldown_is_honoured_not_truncated_by_the_flow_table`,
+`test_an_expired_nat_mapping_stays_shut_until_the_application_sends` and
+`test_the_size_ceiling_holds_even_with_the_age_rotation_switched_off` - the last one drives the
+table through `set_nat` on purpose, because the existing churn test assigns `nat_timeout_s`
+directly and so never exercises the ageing-off path. The first two were verified by mutation:
+making `keep_for` a no-op reproduces the audit's numbers exactly (`gaps=[30.3, 60.8, 60.8, ...]`
+and `dropped=0, passed=720`).
+
+Prose corrected in the same commit (convention: a behaviour change updates the sentence that
+justified the old one): the `_FlowTable` docstring said eviction "can lose an impairment, never
+invent one" as if that covered both paths. It covers the SIZE path; on the AGE path it was not a
+trade but a silent cap. The step-3 comment added by the F1 fix said the blackhole ends at the first
+rotation - that is now what the fix prevents.
+
 ### Fixed: the injector asks Windows for a fine timer tick, and for it to be honoured (audit F3)
 
 `_inject_loop` holds a delayed packet with `Condition.wait(timeout=...)`, and Windows rounds that

CHANGELOG.md

@@ -36,6 +36,23 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol
 
 ### Fixed
 
+- **Two tooltips were telling you things that were not true.** "Dropped" said it also counted
+  link outages - those have had their own counter for a while, so the tooltip was sending you to
+  the wrong number when working out where your packets went. "Downloaded (MB)" promised that
+  hovering would also show how much the app tried to download; nothing of the sort ever appeared.
+  Both now describe what the counter actually holds.
+
+- **Long connection cut-offs and NAT blackouts now last as long as you set them.** Two impairments
+  quietly stopped early, because the tool forgets a connection it has not seen for a while and a
+  forgotten connection looks brand new. "Reset connections" with a cooldown above about half a
+  minute resumed traffic after roughly 30 seconds however long you had asked for - a scenario
+  written as "the connection is down for two minutes" simply did not happen. "NAT mapping expiry"
+  was worse: at a 30 second timeout it never blocked a single packet, because the connection was
+  forgotten just before the incoming traffic arrived. Both now hold for exactly as long as
+  configured - a 120 second cooldown resets every 120 seconds, and an expired NAT mapping stays
+  shut until the application sends something, which is the behaviour the setting describes. The
+  tool's memory limits are unchanged.
+
 - **The delay you set is the delay you get.** Every configured delay used to arrive with several
   milliseconds of padding on top, because of how Windows rounds up the wait the tool uses to hold
   a packet back. It was a fixed surcharge rather than a percentage, so it barely showed at 100 ms

beantester/core.py

@@ -73,6 +73,15 @@ class _FlowTable:
     Eviction can therefore cost a missed NAT-expiry drop (a false negative); it can
     never invent one (a false positive). Losing an impairment is acceptable; a
     frozen capture thread is not.
+
+    That trade is about the SIZE path. It used to apply to the AGE path as well,
+    and there it was not a trade at all but a silent cap: an impairment configured
+    to hold longer than the rotation window simply stopped early, with nothing said.
+    An ``rst_cooldown`` of 120 s resumed traffic after 30 s; a NAT blackhole ended at
+    the first rotation. The age window therefore follows the configuration now -
+    see :meth:`keep_for`, called from ``set_rst`` and ``set_nat``. The size ceiling
+    is unchanged and still checked on every write, so this can only make the table
+    older, never bigger.
     """
 
     __slots__ = ("_new", "_old", "_limit", "_half", "_rotate_s", "_next_rotate",
@@ -128,6 +137,23 @@ def _enforce_ceiling(self):
         if len(self._new) >= self._half:
             self._rotate()
 
+    def keep_for(self, seconds):
+        """Raise the AGE window so a record survives at least ``seconds``.
+
+        A record lives through one rotation and dies at the next, so the window is
+        the MINIMUM lifetime: with the default 30 s an entry survives 30-60 s. That
+        was fine while nothing configurable outlived it, and wrong the moment an
+        impairment could be set to hold longer than the table remembers - a
+        ``rst_cooldown`` of 120 s was measured resuming traffic after 30 s, because
+        the record that said "still cut" had been retired.
+
+        The SIZE ceiling is untouched and still enforced on every write, so a longer
+        window cannot make the table bigger, only older: under churn it rotates on
+        size long before the age window matters. That is why this is safe to raise
+        as far as the field bounds allow (``nat_timeout`` reaches 24 h).
+        """
+        self._rotate_s = max(FLOW_ROTATE_S, float(seconds or 0.0))
+
     def maybe_rotate(self, now):
         """Retire a generation on AGE. O(1): the old dict is handed to the watchdog."""
         if len(self._new) < self._half and now < self._next_rotate:
@@ -337,11 +363,33 @@ def set_spike(self, prob_pct, spike_ms):
     def set_nat(self, timeout_s):
         with self._lock:
             self.nat_timeout_s = max(0.0, timeout_s)
+            # While NAT is on, the flow record IS the impairment: an expired mapping
+            # stays shut only for as long as its record survives, because a retired
+            # record reads back as "never seen" and the next inbound packet reopens
+            # the mapping with nothing sent. Ageing records out on a timer therefore
+            # cannot be right at any setting - the record has to outlive the timeout
+            # AND the blackhole that follows it, and the blackhole is meant to last
+            # until the application sends. Matching the window to the timeout is not
+            # enough and was measured failing: with keep_for(nat_timeout) a 30 s and
+            # a 120 s timeout dropped ZERO packets, because the record died at the
+            # rotation just before the first inbound packet arrived.
+            #
+            # So while NAT is on the age rotation is off for this table and records
+            # are kept until the SIZE ceiling needs the space - which is the real
+            # memory bound anyway (see _FlowTable), is enforced on every write, and
+            # arrives quickly under the flow churn that would otherwise be the
+            # argument for ageing.
+            self._flow_last.keep_for(float("inf") if self.nat_timeout_s > 0 else 0.0)
 
     def set_rst(self, prob_pct, cooldown_s):
         with self._lock:
             self.rst_prob = clamp01(prob_pct / 100.0)
             self.rst_cooldown_s = max(0.1, cooldown_s)
+            # The cooldown deadline lives in the flow table, so the table has to
+            # remember it for at least that long. The field accepts up to 3600 s
+            # while the table retired records after 30; measured before this line,
+            # a 120 s cooldown reset the flow again after 30.3 / 60.8 / 60.8 s.
+            self._reset_until.keep_for(self.rst_cooldown_s)
 
     def reset_now(self, duration_s=2.0, now=None):
         """Manual reset: cut all active connections for the next ``duration_s``."""
@@ -489,19 +537,19 @@ def decide(self, size, is_outbound, local_port, now, rng,
             # cost: `touch` was a get plus this same write (measured 160 ns/op both
             # ways, difference below the noise floor at 200k iterations).
             #
-            # Bounded honesty: the stamp lives in a _FlowTable, which retires a
-            # generation on age (FLOW_ROTATE_S) and on size, and a retired record
-            # reads back as "never seen" - so the direction CAN reopen with no
-            # outbound packet at all. How long the blackhole really holds turns on
-            # whether other flows are moving, because the drop path returns above the
-            # _prune() call and so never rotates anything itself. Measured (timeout
-            # 5 s, drops from t=10): with this flow alone the blackhole was still
-            # holding at t=200; with one other flow driving _prune it ended at t=30,
-            # the first rotation. So it lasts at least one rotation window, and holds
-            # indefinitely only when nothing else is on the wire. That is the table's
-            # documented safe direction (it can lose an impairment, never invent one)
-            # and it is NOT the resurrection above - there the dropped packet did the
-            # reopening itself, once per timeout, for as long as traffic kept coming.
+            # Bounded honesty: the stamp lives in a _FlowTable, and a retired record
+            # reads back as "never seen", so the direction CAN reopen with no
+            # outbound packet at all. While NAT is on the age rotation is therefore
+            # switched off for this table (set_nat -> _FlowTable.keep_for), so the
+            # blackhole lasts until the application sends instead of ending at the
+            # next 30 s rotation - measured before that: a 5 s timeout blackholed for
+            # 20 s and then let traffic through at t=30, and a 30 s one never
+            # blackholed at all. What remains is the SIZE path: under enough flow churn
+            # the record is evicted early and the direction reopens. That is the
+            # table's documented safe direction (it can lose an impairment, never
+            # invent one), and it is NOT the resurrection above - there the dropped
+            # packet did the reopening itself, once per timeout, for as long as
+            # traffic kept coming.
             if self.nat_timeout_s > 0 and key is not None:
                 last = self._flow_last.get(key)
                 expired = last is not None and (now - last) > self.nat_timeout_s

lang/en.json

@@ -392,7 +392,7 @@
   "tips.conn_table": "Who (process) talks to whom (IP:port). 'time[s]' is the connection duration, 'idle[s]' is seconds since the last packet. Click a header to sort; the list scrolls vertically and horizontally.",
   "tips.copy_cli": "Copies to the clipboard a ready CLI command that reproduces these conditions (a frozen .exe uses its own name).",
   "tips.corrupt": "Percent of packets with one flipped data bit - tests resilience to corrupted data.",
-  "tips.data_down": "How much data actually passed downstream (download) since start - real usage. Hovering also shows how much the app tried to download.",
+  "tips.data_down": "How much data actually passed downstream (download) since start - real usage. Packets dropped by loss or a speed limit are not counted here.",
   "tips.data_total": "Sum of downloaded and uploaded data this session.",
   "tips.data_up": "How much data actually passed upstream (upload) since start.",
   "tips.delete_profile": "Delete the selected profile. Built-in presets cannot be deleted.",
@@ -445,7 +445,7 @@
   "tips.stat_duplicated": "Packets sent twice.",
   "tips.stat_flap": "Packets dropped by a link outage (flapping). Counted separately from loss.",
   "tips.stat_lan": "Packets to/from the internet dropped in LAN mode.",
-  "tips.stat_loss": "Packets dropped because of the configured Loss (or link outages).",
+  "tips.stat_loss": "Packets dropped because of the configured Loss. Link outages are counted separately, under Link outage.",
   "tips.stat_mtu": "Packets dropped as too large (MTU black hole).",
   "tips.stat_nat": "Packets dropped after the NAT mapping expired.",
   "tips.stat_overflow": "Packets dropped because the queue overflowed (heavy overload).",

lang/pl.json

@@ -392,7 +392,7 @@
   "tips.conn_table": "Kto (proces) z kim (IP:port) gada. 'czas[s]' to czas trwania połączenia, 'nieakt.[s]' to ile sekund minęło od ostatniego pakietu. Klik w nagłówek sortuje; lista przewija się pionowo i poziomo.",
   "tips.copy_cli": "Kopiuje do schowka gotową komendę CLI, która odtwarza te warunki (w zbudowanym .exe użyje jego nazwy).",
   "tips.corrupt": "Procent pakietów z przekłamanym jednym bitem danych - test odporności na uszkodzone dane.",
-  "tips.data_down": "Ile danych faktycznie przeszło w dół (pobieranie) od startu - realne zużycie. Po najechaniu widać też, ile aplikacja próbowała pobrać.",
+  "tips.data_down": "Ile danych faktycznie przeszło w dół (pobieranie) od startu - realne zużycie. Pakiety porzucone przez utratę albo limit prędkości nie są tu liczone.",
   "tips.data_total": "Suma pobranych i wysłanych danych w tej sesji.",
   "tips.data_up": "Ile danych faktycznie przeszło w górę (wysyłanie) od startu.",
   "tips.delete_profile": "Usuń wybrany profil. Presetów wbudowanych nie można usunąć.",
@@ -445,7 +445,7 @@
   "tips.stat_duplicated": "Pakiety wysłane podwójnie.",
   "tips.stat_flap": "Pakiety porzucone przez przerwę w łączu (flapping). Liczone osobno od strat.",
   "tips.stat_lan": "Pakiety do/od internetu odrzucone w trybie LAN.",
-  "tips.stat_loss": "Pakiety porzucone z powodu ustawionej Utraty (lub przerw w łączu).",
+  "tips.stat_loss": "Pakiety porzucone z powodu ustawionej Utraty. Przerwy w łączu mają własny licznik - Przerwa w łączu.",
   "tips.stat_mtu": "Pakiety odrzucone jako za duże (czarna dziura MTU).",
   "tips.stat_nat": "Pakiety odrzucone po wygaśnięciu mapowania NAT.",
   "tips.stat_overflow": "Pakiety porzucone, bo kolejka się przepełniła (silne przeciążenie).",