docs(guidelines): record the HashMap iteration-order defect class

TECHNICAL_REPORTS/1290-hashmap-order-guideline-20260822.en.md

@@ -0,0 +1,70 @@
+# Technical Report: PR #1290 - Recording the HashMap iteration order class, and declining to gate it
+
+## Executive Summary
+
+Seven instances of one defect class were found and fixed in a three-day sweep. This PR writes the class down in `docs/code-guidelines.md` and records, with numbers, the decision not to enforce it with a static check. No Rust changed.
+
+The class: a `HashMap` iteration result becomes ordered state or feeds an order-sensitive consumer. Nothing in the toolchain flags it. The types are correct, `cargo check` and clippy are clean, and the tests pass most of the time.
+
+## 1. Problem Statement
+
+Seven instances across five modules, none caught by review or CI: #1265 (four test fixtures), #1267 (`lang_bias.rs`), #1276 (RT-DETRv2 layout sniffing), #1277 (four registry accessors) and #1286 (three eviction paths). Two of them reached primary request paths, and one silently double-transposed conv weights into a shape-valid tensor nothing downstream could flag.
+
+Recurrence at that rate is the argument for writing the rule down rather than relying on each reviewer rediscovering it.
+
+## 2. Technical Decisions
+
+### 2.1 What the guideline emphasizes
+
+Two points get the most space, because they are the ones that cost the most to rediscover.
+
+**`RandomState` seeds per map instance, not per process.** The common mental model, "iteration order is randomized per run", implies a fixed binary on a fixed machine sees a fixed order. It does not. Measured here: build ten maps from the same five keys inside one process, over 200 processes; 127 runs produced 10 distinct orders out of 10, 62 produced 9, 11 produced 8. Not one of the 200 had the ten maps agree. The split moves between measurement runs because the experiment is itself random, and the guideline says so rather than presenting one split as a constant.
+
+**A sort is not automatically a fix.** `sort_by_key` and `sort_by` are stable, so ties keep input order, and when the input came from a `HashMap` that retained order is hash order. Three of the seven sorted their candidate list and were still nondeterministic. `sort_unstable_by` is not the remedy either: it substitutes a different arbitrary tie order rather than defining one. The remedy is a key that is a total order.
+
+The guideline also records the testing rule, because every fix in the family needed it and each of its three points is a way to write a test that passes against unfixed code: rebuild the map inside each iteration and loop 32 or more times; construct an actual tie, since distinct keys already give a total order; and write equal `Instant` values in deliberately, since `Instant::now()` never collides at nanosecond resolution. PR #1281's test failed on 27 of 64 freshly built maps, which is why a single-shot version would have passed and shipped nothing.
+
+### 2.2 Declining the static check, and the number that decided it
+
+Two candidate checks were prototyped in the shape of `check_kernel_dtype_keys.py` and run over all 1,248 `.rs` files, against the current tree and against the reconstructed pre-fix tree of each fix.
+
+The `min_by_key` / `max_by_key` on an unordered-map receiver rule looked strongest on its face: 4 flags, 0 false positives. Its catch rate against the seven is **0 of 7**, a number the issue never computed. None of the seven used either method. Gating on it would mean suppressing four of four findings on the day it landed, and a suppression comment does not re-arm on the change that would make the pattern real, which is the key becoming coarser than `Instant`. Those four hits are true positives for the lint and zero live defects: every key is an `Instant` stamped once per call, so the tie is unreachable today. Reporting them as four bugs would have been false and would have cost the rule its credibility on first inspection.
+
+The `Vec` from a map view rule fails for a different and more decisive reason: it flags all three #1286 sites **after** PR #1288 fixed them. The fix keeps the shape and changes only whether the sort key is total, a distinction no regex can draw. Its 3-of-7 held only against pre-fix trees, so on any fixed tree those three become permanent false positives and it can never be a gate.
+
+The general form is undecidable from source alone, since whether an order matters depends on a consumer that is frequently in another module. The guideline says the rule is enforced by review and by the testing rule, and to re-open the question if a future instance takes a shape either candidate would have caught.
+
+One improvement over the issue's analysis survives: a tree-wide scan for `type X = HashMap<...>` removes the type-alias blind spot that `WeightMap` creates, and it resolves `Memo` and `DetachedMap` as well. That limit is fixable. The other three misses are not.
+
+## 3. Change Summary
+
+| File | Change |
+| --- | --- |
+| `docs/code-guidelines.md` | New `## HashMap Iteration Order` section: the rule with wrong and right examples, the per-instance measurement, order-sensitive consumers, the stable-sort subsection, the total-order remedies, the testing rule, the `BTreeMap` alternative, the seven instances, and the enforcement decision with its numbers |
+| `docs/README.md` | Index entry |
+
+## 4. Review Findings
+
+The implementation corrected the issue that specified it, in ways worth keeping.
+
+The issue described #1286 as open with its three instances unfixed on `main`. That was stale by roughly an hour: PR #1288 merged before the work started. It is not cosmetic, because it inverts the `Vec` rule's evaluation, turning three catches into three false positives. The stale text is a process error on the filing side: #1287 was filed to record a class while instances of that class were still being fixed, and it was not refreshed before implementation.
+
+The `RandomState` split did not reproduce exactly (127/62/11 measured against 133/52/15 in the issue). Correctly treated as the experiment being random rather than as an error, with the invariant that the ten maps never agree holding in both runs, and the guideline states the caveat.
+
+The reconstruction recipe in the issue, `git show <fix>^:<one file>`, understates any tree-wide rule, since both file-local type detection and the alias table need the whole tree. `git archive <fix>^ src` is the working form.
+
+## 5. Validation
+
+No cargo gate was run and none applies: the diff is two Markdown files with zero Rust changed, so `make verify-test-cuda` would exercise nothing this PR touches. CI's `changes` path filter reached the same conclusion and skipped every Rust job while `crate versions`, `kernel dtype keys` and `cross-repo refs` passed.
+
+Checks that do apply, all exit 0: `make verify-fmt`; the new section's relative link to `scripts/ci/check_kernel_dtype_keys.py` resolves; `docs/README.md` indexes it; and the added lines carry no em dash and no AI attribution.
+
+Every in-tree citation the section makes was verified rather than carried over: the four test-loop constants and their line numbers, the 27-of-64 figure in PR #1281, the withdrawal at `Makefile:604-616`, and the seven issue and PR pairs.
+
+## 6. Related Work
+
+- #1287: the issue this closes.
+- #1265 and PR #1266, #1267 and PR #1269, #1276 and PR #1281, #1277 and PR #1284, #1286 and PR #1288: the seven instances the guideline documents.
+- #1283 and PR #1285: found in the same window. A different class, an `err_expect` that reddened the lint gate for every contributor, but the same underlying gap, which is that nothing ran the check at PR time.
+
+Left for a separate decision: the four `min_by_key` LRU eviction sites are genuinely fragile and now have no mechanical guard. They are safe only because every key is an `Instant` stamped once per call. Pinning them with a total-order tie-break on the map key is small, and `src/server/responses_store.rs` and `src/server/conversation_store.rs` are already being touched by #1248.

TECHNICAL_REPORTS/1290-hashmap-order-guideline-20260822.ko.md

@@ -0,0 +1,70 @@
+# 기술 보고서: PR #1290 - HashMap 순회 순서 계열의 기록, 그리고 정적 검사를 두지 않기로 한 결정
+
+## 요약
+
+사흘간의 스윕에서 같은 결함 계열 인스턴스 일곱 개를 찾아 고쳤다. 이 PR은 그 계열을 `docs/code-guidelines.md`에 적고, 정적 검사로 강제하지 않기로 한 결정을 **숫자와 함께** 기록한다. Rust 변경은 없다.
+
+계열은 이것이다. `HashMap` 순회 결과가 순서 있는 상태가 되거나 순서에 민감한 소비자로 흘러간다. 툴체인 어느 것도 잡지 못한다. 타입은 맞고, `cargo check`도 clippy도 깨끗하며, 테스트는 대체로 통과한다.
+
+## 1. 문제
+
+다섯 모듈에 걸친 일곱 인스턴스, 리뷰도 CI도 못 잡았다. #1265(테스트 픽스처 4개), #1267(`lang_bias.rs`), #1276(RT-DETRv2 레이아웃 판별), #1277(레지스트리 접근자 4개), #1286(축출 경로 3개). 그중 둘은 주 요청 경로에 닿았고, 하나는 conv 가중치를 조용히 이중 transpose해 하류에서 아무도 못 잡는 형상상 유효한 텐서로 만들 수 있었다.
+
+이 빈도의 재발이 곧 "리뷰어마다 다시 발견하게 두지 말고 규칙으로 적자"는 근거다.
+
+## 2. 기술적 판단
+
+### 2.1 가이드라인이 무게를 싣는 두 지점
+
+다시 발견하는 비용이 가장 큰 둘에 가장 많은 지면을 줬다.
+
+**`RandomState`는 프로세스가 아니라 맵 인스턴스마다 시드된다.** "순회 순서는 실행마다 무작위"라는 흔한 모델은 고정된 기계의 고정된 바이너리가 고정된 순서를 본다는 뜻을 함축한다. 아니다. 여기서 실측: 한 프로세스 안에서 같은 다섯 키로 맵 열 개를 만들고, 200 프로세스에 걸쳐 세었다. 127회가 10개 중 고유 순서 10개, 62회가 9개, 11회가 8개였다. **200회 중 열 개가 일치한 경우는 한 번도 없었다.** 실험 자체가 무작위라 분포는 측정마다 움직이며, 가이드라인은 한 분포를 상수처럼 제시하지 않고 그 단서를 적는다.
+
+**정렬이 자동으로 수정이 되지는 않는다.** `sort_by_key`와 `sort_by`는 안정 정렬이라 동점이 입력 순서를 유지하고, 입력이 `HashMap`에서 왔다면 그 유지된 순서가 곧 해시 순서다. 일곱 중 셋이 후보 목록을 정렬하고도 비결정적이었다. `sort_unstable_by`도 해법이 아니다. 순서를 정의하지 않고 다른 임의 순서로 바꿀 뿐이다. 해법은 키를 전순서로 만드는 것이다.
+
+테스트 규칙도 적었다. 이 계열의 모든 수정이 필요로 했고, 세 항목 각각이 미수정 코드에서도 통과하는 테스트를 쓰는 방법이기 때문이다. 반복마다 맵을 새로 만들고 32회 이상 돌 것, 실제 동점을 만들 것(서로 다른 키는 이미 전순서다), 같은 `Instant` 값을 의도적으로 써 넣을 것(`Instant::now()`는 나노초 해상도에서 충돌하지 않는다). PR #1281의 테스트는 새로 만든 맵 64개 중 27개에서 실패했고, 그래서 단발 버전이었다면 통과하고 아무것도 못 실었을 것이다.
+
+### 2.2 정적 검사를 두지 않기로 한 결정과 그것을 가른 숫자
+
+`check_kernel_dtype_keys.py` 형태로 후보 검사 둘을 프로토타이핑해 `.rs` 파일 1,248개 전체에, 현재 트리와 각 수정의 재구성된 수정 전 트리에 대해 돌렸다.
+
+"맵 수신자에 대한 `min_by_key`/`max_by_key`" 규칙은 겉보기에 가장 강했다. 플래그 4건, 오탐 0. 그런데 일곱 인스턴스에 대한 검출률이 **0 of 7**이다. 이슈가 계산하지 않은 숫자다. 일곱 중 어느 것도 그 두 메서드를 쓰지 않는다. 이걸로 게이트를 걸면 도입 당일에 4건 중 4건을 억제해야 하고, 억제 주석은 그 패턴을 실재하게 만들 변경(키가 `Instant`보다 거칠어지는 것)에 다시 무장하지 않는다. 그 4건은 린트 참양성이자 live 결함 0건이다. 키가 전부 호출당 한 번 찍히는 `Instant`라 동점이 지금은 도달 불가다. 이를 "버그 4개를 찾았다"고 보고했다면 거짓이고, 처음 확인하는 사람에게 규칙의 신뢰를 잃었을 것이다.
+
+"맵 뷰에서 만든 `Vec`을 순서 민감하게 소비" 규칙은 더 결정적인 이유로 탈락했다. **PR #1288이 고친 뒤의 #1286 세 지점을 그대로 플래그한다.** 수정이 형태는 유지하고 정렬 키가 전순서인지만 바꾸는데, 그 구분은 어떤 정규식도 그릴 수 없다. 3-of-7은 수정 전 트리에서만 성립했고, 계열이 고쳐진 트리에서는 그 셋이 영구 오탐이 되므로 게이트가 될 수 없다.
+
+일반형은 소스만으로 판정 불가다. 순서가 중요한지가 종종 다른 모듈에 있는 소비자에 달려 있기 때문이다. 가이드라인은 이 규칙이 리뷰와 위 테스트 규칙으로 강제된다고 적고, 두 후보 중 하나가 잡았을 형태의 인스턴스가 나오면 질문을 다시 열라고 닫는다.
+
+이슈 분석보다 나아진 것 하나는 남는다. `type X = HashMap<...>`을 트리 전역으로 훑으면 `WeightMap`이 만드는 타입 별칭 사각지대가 사라지고, `Memo`와 `DetachedMap`도 함께 풀린다. 그 한계는 고칠 수 있다. 나머지 셋은 아니다.
+
+## 3. 변경 요약
+
+| 파일 | 변경 |
+| --- | --- |
+| `docs/code-guidelines.md` | 신규 `## HashMap Iteration Order` 절: 규칙과 wrong/right 예시, 인스턴스별 무작위화 실측, 순서 민감 소비자 목록, 안정 정렬 하위절, 전순서 해법 3형태, 테스트 규칙, `BTreeMap` 대안, 일곱 인스턴스, 그리고 숫자를 동반한 강제 결정 |
+| `docs/README.md` | 색인 항목 |
+
+## 4. 리뷰 지적사항
+
+구현이 그것을 지시한 이슈를 정정했고, 남길 가치가 있다.
+
+이슈는 #1286을 열려 있고 세 인스턴스가 `main`에서 미수정이라고 서술했다. 약 한 시간 낙후였다. PR #1288이 작업 시작 전에 머지됐다. 표면적 문제가 아니다. `Vec` 규칙의 평가를 뒤집어 검출 3건을 오탐 3건으로 만들기 때문이다. 이 낙후는 발행 쪽의 절차 오류다. 그 계열의 인스턴스를 아직 고치는 중에 계열을 기록하는 이슈를 냈고, 구현 전에 갱신하지 않았다.
+
+`RandomState` 분포는 정확히 재현되지 않았다(측정 127/62/11 대 이슈 133/52/15). 오류가 아니라 실험이 무작위인 것으로 옳게 처리했고, 열 개가 결코 일치하지 않는다는 불변식은 양쪽에서 유지됐으며, 가이드라인이 그 단서를 적는다.
+
+이슈의 재구성 레시피 `git show <fix>^:<파일 하나>`는 트리 전역 규칙을 과소평가한다. 파일 지역 타입 탐지와 별칭 표 둘 다 트리 전체가 필요하다. `git archive <fix>^ src`가 동작하는 형태다.
+
+## 5. 검증
+
+cargo 게이트는 돌리지 않았고 해당되지도 않는다. diff가 마크다운 두 파일이고 Rust 변경이 0이라 `make verify-test-cuda`는 이 PR이 건드린 것을 아무것도 실행하지 않는다. CI의 `changes` 경로 필터도 같은 결론에 도달해 Rust 잡을 전부 건너뛰었고 `crate versions`, `kernel dtype keys`, `cross-repo refs`가 통과했다.
+
+해당되는 검사는 전부 exit 0이다. `make verify-fmt`, 신규 절의 `scripts/ci/check_kernel_dtype_keys.py` 상대 링크 해석, `docs/README.md` 색인 등록, 그리고 추가된 줄에 em dash와 AI attribution 없음.
+
+절이 인용하는 트리 내 사실은 옮겨 적지 않고 전부 확인했다. 테스트 루프 상수 4개와 그 줄 번호, PR #1281의 27-of-64 수치, `Makefile:604-616`의 철회, 일곱 개 이슈/PR 쌍.
+
+## 6. 관련 작업
+
+- #1287: 이 PR이 닫는 이슈.
+- #1265와 PR #1266, #1267과 PR #1269, #1276과 PR #1281, #1277과 PR #1284, #1286과 PR #1288: 가이드라인이 문서화하는 일곱 인스턴스.
+- #1283과 PR #1285: 같은 기간에 나왔다. 계열은 다르지만(모든 기여자의 린트 게이트를 red로 만든 `err_expect`) 밑에 깔린 구멍은 같다. PR 시점에 그 검사를 도는 것이 없었다는 것.
+
+별도 판단으로 남긴 것: `min_by_key` LRU 축출 4곳은 실제로 취약하고 이제 기계적 가드가 없다. 키가 전부 호출당 한 번 찍히는 `Instant`라서만 안전하다. 맵 키로 전순서 타이브레이크를 거는 것은 작은 작업이고, `src/server/responses_store.rs`와 `src/server/conversation_store.rs`는 #1248이 이미 건드리고 있다.

docs/README.md

@@ -33,7 +33,7 @@ Current GitHub-facing docs:
 17. `cascade-attention.md` — shared-prompt-prefix (cascade) decode: computing a prefix shared by several concurrent sequences once per step instead of once per sequence, the two-level decomposition, the flags, and how to tell which path a launch took.
 18. `sparse-paged-decode.md` — sparse attention reduced to page indirection over the fused v2 decode kernel: the addressing argument, the MiniMax-M3 routing, why DeepSeek Sparse Attention is not routed, where the dispatch floor sits, the kill switch, and the benchmark harness.
 19. `mtp-policy-api.md`: the supported read interface for the adaptive B=1 MTP verdict (`GET /v1/internal/mtp-policy`): the response body, the four states, the unavailable reasons, and the schema versioning and compatibility policy.
-20. `code-guidelines.md`: the file-size and module-split thresholds, including when to extract a `<name>_helpers.rs` and when inline tests move to a sibling `_tests.rs`, the `// Used by:` annotation convention for shared functions, and the JIT kernel rule that every varying input dtype must appear in `template_args` because CUDA keys the compiled-module cache on it.
+20. `code-guidelines.md`: the file-size and module-split thresholds, including when to extract a `<name>_helpers.rs` and when inline tests move to a sibling `_tests.rs`, the `// Used by:` annotation convention for shared functions, the JIT kernel rule that every varying input dtype must appear in `template_args` because CUDA keys the compiled-module cache on it, and the `HashMap` iteration-order rule covering what counts as an order-sensitive consumer, why a stable sort on a non-total key does not pin the result, the fresh-map-per-iteration testing requirement, and why no static check enforces it.
 
 ## Architecture Decision Records