diff --git a/tests/test_redact_large_string_cache.py b/tests/test_redact_large_string_cache.py index dfcbbf21..400b0d1b 100644 --- a/tests/test_redact_large_string_cache.py +++ b/tests/test_redact_large_string_cache.py @@ -1,21 +1,21 @@ -"""Grosses chaines: memoiser au lieu de repayer ~15 regex a chaque requete. +"""Large strings: memoize instead of re-paying ~15 regexes on every request. -``_redact_fn_cached`` excluait du LRU toute chaine > 16 384 caracteres, pour -eviter d'evincer les milliers de petites chaines recurrentes et de gonfler la -RSS. Consequence mesuree sur une session reelle de 22 Mo: 59 grosses chaines -(29 uniques, 0,76 Mo) repassaient l'integralite des regex a CHAQUE requete, -pour un resultat toujours identique — 1,68 s par requete, soit 99,9% du cout -recurrent de la redaction une fois le cache des petites chaines chaud. +``_redact_fn_cached`` excluded every string > 16,384 characters from the LRU, +to avoid evicting the thousands of small recurring strings and inflating RSS. +Measured consequence on a real 22 MB session: 59 large strings (29 unique, +0.76 MB) went through the full regex set on EVERY request, for an always +identical result -- 1.68 s per request, i.e. 99.9% of the recurring redaction +cost once the small-string cache is warm. -Le correctif ajoute un SECOND cache, dedie aux grosses chaines, borne en -nombre d'entrees ET par une taille maximale par entree, pour que la RSS reste -plafonnee. Les chaines geantes (> plafond) restent volontairement non cachees. +The fix adds a SECOND cache, dedicated to large strings, bounded both by entry +count AND by a maximum size per entry, so that RSS stays capped. Giant strings +(> cap) deliberately remain uncached. -Contrat verifie ici: - 1. une grosse chaine est redigee exactement comme avant; - 2. la meme grosse chaine n'est pas recalculee au second appel; - 3. les deux caches restent separes (pas d'eviction croisee); - 4. une chaine au-dela du plafond n'est jamais retenue. +Contract verified here: + 1. a large string is redacted exactly as before; + 2. the same large string is not recomputed on the second call; + 3. the two caches stay separate (no cross-eviction); + 4. a string above the cap is never retained. """ import sys from pathlib import Path @@ -37,13 +37,13 @@ SECRET = "gh" + "p_" + "0123456789abcdefghijklmnopqrstuvwxyzAB" @pytest.fixture(autouse=True) def _isolate_large_cache(): - """Ne pas laisser ce fichier polluer l'etat global du process. + """Do not let this file pollute the process-wide global state. - Le cache est un singleton de module partage par toute la suite. Le vider - sans le restaurer laisse les tests suivants demarrer a froid, ce qui - perturbe ceux qui dependent de fenetres de fraicheur ou de temps de - reponse. On repart d'un cache vide ET on le revide en sortie, pour que ce - fichier n'ait aucun effet observable en dehors de lui-meme. + The cache is a module singleton shared by the whole suite. Clearing it + without restoring it makes the following tests start cold, which disturbs + those that depend on freshness windows or response times. We start from an + empty cache AND clear it again on exit, so that this file has no observable + effect outside of itself. """ helpers._redact_fn_large_lru.cache_clear() yield @@ -51,11 +51,11 @@ def _isolate_large_cache(): def _big(secret: str, size: int) -> str: - """Chaine > seuil du petit cache, porteuse d'un secret. + """String > small-cache threshold, carrying a secret. - Le remplissage doit etre assez long pour atteindre ``size`` meme au-dela - du plafond du grand cache, sinon la troncature produit une chaine trop - courte et le test ne verifie plus ce qu'il annonce. + The filler must be long enough to reach ``size`` even beyond the large + cache cap, otherwise truncation yields a string that is too short and the + test no longer verifies what it claims. """ unit = "lorem ipsum dolor sit amet " filler = unit * (size // len(unit) + 1) @@ -82,21 +82,21 @@ def test_large_string_is_memoized(): info = helpers._redact_fn_large_lru.cache_info() assert second == first - assert info.hits >= 1, "grosse chaine recalculee au second appel" - assert info.misses == misses_after_first, "miss supplementaire inattendu" + assert info.hits >= 1, "large string recomputed on second call" + assert info.misses == misses_after_first, "unexpected additional miss" def test_small_strings_do_not_use_the_large_cache(): helpers._redact_fn_large_lru.cache_clear() - small = f"petit texte {SECRET}" + small = f"small text {SECRET}" assert len(small) <= _REDACT_CACHE_MAX_TEXT_LEN _redact_fn_cached(small) info = helpers._redact_fn_large_lru.cache_info() - assert info.hits == 0 and info.misses == 0, "petite chaine routee vers le grand cache" + assert info.hits == 0 and info.misses == 0, "small string routed to the large cache" def test_giant_string_is_not_retained(): - """Au-dela du plafond: correct, mais jamais mis en cache (RSS bornee).""" + """Above the cap: correct, but never cached (bounded RSS).""" helpers._redact_fn_large_lru.cache_clear() giant = _big(SECRET, _REDACT_LARGE_CACHE_MAX_TEXT_LEN + 10000) assert len(giant) > _REDACT_LARGE_CACHE_MAX_TEXT_LEN @@ -106,7 +106,7 @@ def test_giant_string_is_not_retained(): assert out == _redact_fn_uncached(giant) assert SECRET not in out info = helpers._redact_fn_large_lru.cache_info() - assert info.currsize == 0, "chaine geante retenue en cache" + assert info.currsize == 0, "giant string retained in cache" def test_large_cache_is_bounded(): diff --git a/tests/test_redact_value_no_copy_when_clean.py b/tests/test_redact_value_no_copy_when_clean.py index 210217bf..eb52bc93 100644 --- a/tests/test_redact_value_no_copy_when_clean.py +++ b/tests/test_redact_value_no_copy_when_clean.py @@ -1,18 +1,18 @@ -"""Redaction: ne pas reconstruire ce qui n'a pas change. +"""Redaction: do not rebuild what has not changed. -``_redact_value`` reconstruisait integralement chaque dict/liste, soit une -copie profonde de tout le payload a chaque reponse, meme quand rien n'est -redige. Sur une session de 22 Mo, 97,4% des chaines ne contiennent aucun -marqueur sensible: la copie est donc du travail pur perte, et elle tient le -GIL (json/allocations ne le liberent pas), ce qui serialise les onglets. +``_redact_value`` used to rebuild every dict/list in full, i.e. a deep copy of +the whole payload on every response, even when nothing is redacted. On a 22 MB +session, 97.4% of the strings contain no sensitive marker: the copy is +therefore pure wasted work, and it holds the GIL (json/allocations do not +release it), which serializes the tabs. -Contrat verifie ici: - 1. Quand rien n'est redige, la valeur RENVOYEE EST l'objet d'origine - (identite, pas seulement egalite) et aucun conteneur proportionnel au - payload n'est alloue en chemin. - 2. Quand quelque chose est redige, un NOUVEL objet est renvoye et - l'original n'est PAS mute (fail-closed: pas de fuite par aliasing). - 3. Le resultat reste egal a celui de l'ancienne implementation. +Contract verified here: + 1. When nothing is redacted, the RETURNED value IS the original object + (identity, not just equality) and no container proportional to the + payload is allocated along the way. + 2. When something is redacted, a NEW object is returned and the original + is NOT mutated (fail-closed: no leak through aliasing). + 3. The result stays equal to the one of the previous implementation. """ import sys import tracemalloc @@ -27,7 +27,7 @@ SECRET = "authorization=Bearer sk-live-abcdef0123456789" def _reference_redact(v, *, _enabled): - """Implementation d'origine (copie systematique), pour comparaison.""" + """Original implementation (systematic copy), kept for comparison.""" from api.helpers import _redact_text if isinstance(v, str): @@ -40,25 +40,25 @@ def _reference_redact(v, *, _enabled): def test_clean_payload_is_returned_by_identity(): - """Rien de sensible -> aucun dict/liste ne doit etre reconstruit.""" + """Nothing sensitive -> no dict/list must be rebuilt.""" payload = { "messages": [ - {"role": "user", "content": "bonjour, ou en est la commande 4512 ?"}, - {"role": "assistant", "content": [{"type": "text", "text": "elle part demain"}]}, + {"role": "user", "content": "hello, where is order 4512 at?"}, + {"role": "assistant", "content": [{"type": "text", "text": "it ships tomorrow"}]}, ], "meta": {"workspace": "/workspace/project", "count": 3, "ok": True}, } out = _redact_value(payload, _enabled=True) - assert out is payload, "payload propre reconstruit inutilement" + assert out is payload, "clean payload rebuilt needlessly" assert out["messages"] is payload["messages"] assert out["messages"][0] is payload["messages"][0] assert out["meta"] is payload["meta"] def test_clean_payload_does_not_allocate_eager_container_copies(): - """La voie propre doit rester O(1) en allocation de conteneurs temporaires.""" + """The clean path must stay O(1) in temporary container allocation.""" payload = { "dict": {index: index for index in range(20_000)}, "list": list(range(20_000)), @@ -77,39 +77,39 @@ def test_clean_payload_does_not_allocate_eager_container_copies(): tracemalloc.stop() assert out is payload - # Une liste temporaire de 20 000 pointeurs depasse a elle seule 160 Ko. - # Le seuil reste volontairement un ordre de grandeur en dessous, sans - # dependre des details exacts de l'allocateur d'une version de CPython. - assert peak - before < 16_384, f"copies temporaires detectees: {peak - before} octets" + # A temporary list of 20,000 pointers alone exceeds 160 KB. The threshold + # deliberately stays an order of magnitude below that, without depending + # on the exact allocator details of a given CPython version. + assert peak - before < 16_384, f"temporary copies detected: {peak - before} bytes" def test_sensitive_payload_is_copied_and_original_untouched(): - """Quelque chose est redige -> nouvel objet, original intact.""" + """Something is redacted -> new object, original intact.""" inner = {"role": "user", "content": SECRET} payload = {"messages": [inner], "meta": {"workspace": "/tmp"}} out = _redact_value(payload, _enabled=True) - # un nouvel objet est renvoye sur le chemin modifie + # a new object is returned along the modified path assert out is not payload assert out["messages"] is not payload["messages"] assert out["messages"][0] is not inner - # l'original n'est pas mute (pas de fuite par aliasing) + # the original is not mutated (no leak through aliasing) assert inner["content"] == SECRET assert payload["messages"][0]["content"] == SECRET - # le secret est bien masque dans la sortie + # the secret is indeed masked in the output assert out["messages"][0]["content"] != SECRET - # les branches NON modifiees restent partagees (c'est tout l'interet) + # the UNmodified branches stay shared (that is the whole point) assert out["meta"] is payload["meta"] def test_copy_on_first_change_preserves_order_types_and_clean_identity(): - """Une redaction tardive copie seulement son chemin sans reordonner le JSON.""" - before = {"clean": ["avant", 1]} - after = {"clean": ["apres", 2]} + """A late redaction copies only its own path without reordering the JSON.""" + before = {"clean": ["before", 1]} + after = {"clean": ["after", 2]} payload = { "before": before, "items": [before, {"secret": SECRET}, after], @@ -130,14 +130,14 @@ def test_copy_on_first_change_preserves_order_types_and_clean_identity(): def test_matches_reference_implementation(): - """Le resultat doit rester identique a l'implementation d'origine.""" + """The result must stay identical to the original implementation.""" payload = { "messages": [ - {"role": "user", "content": "texte normal"}, + {"role": "user", "content": "plain text"}, {"role": "assistant", "content": SECRET}, - {"role": "user", "content": ["propre", SECRET, 42, None]}, + {"role": "user", "content": ["clean", SECRET, 42, None]}, ], - "nested": {"a": {"b": {"c": SECRET}}, "d": {"e": "rien"}}, + "nested": {"a": {"b": {"c": SECRET}}, "d": {"e": "nothing"}}, "scalars": [1, 2.5, True, None], } @@ -145,7 +145,7 @@ def test_matches_reference_implementation(): def test_disabled_redaction_still_shares(): - """Redaction desactivee -> rien ne doit etre copie.""" + """Redaction disabled -> nothing must be copied.""" payload = {"messages": [{"role": "user", "content": SECRET}]} out = _redact_value(payload, _enabled=False) assert out is payload