v0.2-dev: versioning policy + roadmap scaffold - #21
Conversation
docs/VERSIONING.md: freeze policy — v0.1 content immutable; patch rule for harness bugs vs minor rule for new content; rollout checklist. docs/ROADMAP_v0.2.md: 12 harness fixes from ultrareview (H1-H12), expanded domain coverage plan (11 domains from ARM A corpus), result directory structure (v0.1/ and v0.2/ separate), NLA sources unchanged from v0.1, out-of-scope items. nla_eval/__init__.py: __version__ = "0.2.0.dev" Branch v0.2-dev stays local until all harness fixes + new domain sets complete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a roadmap for NLAttack v0.2 (docs/ROADMAP_v0.2.md), defines a versioning policy (docs/VERSIONING.md), and updates the version constant in nla_eval/__init__.py to 0.2.0.dev. Feedback on the documentation highlights a technical inaccuracy regarding the proposed fix for handling NaN values with json.dumps(allow_nan=False), which raises a ValueError instead of converting them to null. Additionally, the description for harness fix H11 is noted as outdated, as token-length match scoring is already implemented in the codebase.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| |---|---|---|---| | ||
| | H1 | `emergence.py:63` | `hash(w) % dim` uses PYTHONHASHSEED — Tier-1 nondeterministic across processes | Replace with `mmh3.hash(w) % dim` or `int(hashlib.md5(w.encode()).hexdigest(),16) % dim` | | ||
| | H2 | `adapters.py:193-196` | Schema-drift 200 → BOS noise / empty string; no JSON parse guard | Fail loudly on missing `prompt_length`/`results`; validate before use | | ||
| | H3 | `emergence_dashboard.py:117` | `json.dumps` with `allow_nan=True` writes invalid JSON | `allow_nan=False`; replace NaN with `null` | |
There was a problem hiding this comment.
In Python's json module, setting allow_nan=False in json.dumps does not automatically replace NaN values with null. Instead, it raises a ValueError when encountering any NaN (or inf, -inf) values during serialization.
To safely serialize NaN values as null in the output JSON, the data must be pre-processed to replace float NaN values with None (which serializes to null), or a custom JSON encoder must be used.
| - `emergence_dashboard.py:117-120` — `json.dumps` with `allow_nan=True`; both | ||
| committed result files contain bare `NaN` tokens (invalid JSON for strict parsers). |
There was a problem hiding this comment.
Setting allow_nan=False in json.dumps will cause Python to raise a ValueError if any NaN values are present in the data, rather than converting them to null. To write valid JSON with null instead of NaN, the NaN values must be replaced with None in the Python dictionary/list before serialization.
| | H8 | `adapters.py:193-200` | 2 HTTP requests per `reconstruct()`; callers budget as 1 | Add request counter + pacing; fix budget comments | | ||
| | H9 | `real_nla_example.py:23` | `layer` param doesn't exist in constructor → `TypeError` on default entry | Remove `layer=` kwarg | | ||
| | H10 | `matching.py:94-98` | Lexical match is unanchored substring; "art" matches "departure" | Use word-boundary regex or strip-and-split | | ||
| | H11 | `controls.py:94-99` | Controls scored only on freq_band; token-length match ignored | Add token-length scoring term per docstring spec | |
There was a problem hiding this comment.
The description for H11 states that "token-length match [is] ignored" and suggests adding a token-length scoring term. However, the current implementation of build_matched_controls in nla_eval/controls.py (lines 94-97) already scores candidates using both n_tokens and char_band in addition to freq_band:
score += 1 if wp["n_tokens"] == cp["n_tokens"] else 0
score += 1 if wp["char_band"] == cp["char_band"] else 0This indicates that the roadmap description is outdated or inaccurate relative to the existing codebase.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the roadmap for NLAttack v0.2, detailing planned harness fixes, expanded domain coverage, and versioning/config gating, alongside a new versioning policy document. Feedback on the roadmap suggests using standard library alternatives like zlib.adler32 for deterministic hashing to avoid external dependencies, and notes that json.dumps with allow_nan=False will raise a ValueError on NaN values, recommending pre-processing the data to replace NaN with None instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| | # | File | Issue | Fix | | ||
| |---|---|---|---| | ||
| | H1 | `emergence.py:63` | `hash(w) % dim` uses PYTHONHASHSEED — Tier-1 nondeterministic across processes | Replace with `mmh3.hash(w) % dim` or `int(hashlib.md5(w.encode()).hexdigest(),16) % dim` | |
There was a problem hiding this comment.
Using 'hashlib.md5' can be relatively slow if executed in a tight loop, and 'mmh3' requires an external dependency. A faster, deterministic, and dependency-free alternative is to use 'zlib.adler32' or 'binascii.crc32' from the Python standard library (e.g., 'zlib.adler32(w.encode()) % dim').
| | H1 | `emergence.py:63` | `hash(w) % dim` uses PYTHONHASHSEED — Tier-1 nondeterministic across processes | Replace with `mmh3.hash(w) % dim` or `int(hashlib.md5(w.encode()).hexdigest(),16) % dim` | | |
| | H1 | emergence.py:63 | hash(w) % dim uses PYTHONHASHSEED — Tier-1 nondeterministic across processes | Replace with zlib.adler32(w.encode()) % dim or binascii.crc32(w.encode()) % dim | |
| |---|---|---|---| | ||
| | H1 | `emergence.py:63` | `hash(w) % dim` uses PYTHONHASHSEED — Tier-1 nondeterministic across processes | Replace with `mmh3.hash(w) % dim` or `int(hashlib.md5(w.encode()).hexdigest(),16) % dim` | | ||
| | H2 | `adapters.py:193-196` | Schema-drift 200 → BOS noise / empty string; no JSON parse guard | Fail loudly on missing `prompt_length`/`results`; validate before use | | ||
| | H3 | `emergence_dashboard.py:117` | `json.dumps` with `allow_nan=True` writes invalid JSON | `allow_nan=False`; replace NaN with `null` | |
There was a problem hiding this comment.
Note that calling 'json.dumps' with 'allow_nan=False' will raise a 'ValueError' if a 'NaN' value is encountered, rather than automatically replacing it with 'null'. To achieve the desired behavior of serializing 'NaN' as 'null' without raising an exception, the data structure should be pre-processed to replace float 'NaN' values with 'None' (which serializes to 'null') before calling 'json.dumps'.
| | H3 | `emergence_dashboard.py:117` | `json.dumps` with `allow_nan=True` writes invalid JSON | `allow_nan=False`; replace NaN with `null` | | |
| | H3 | emergence_dashboard.py:117 | json.dumps with allow_nan=True writes invalid JSON | Replace NaN with None in data before json.dumps with allow_nan=False | |
Promote the prior frozen benchmark to v1 and this work to a formal v2.0.0 release, reconciling versioning, docs, and counts across the repo. v1 content and its published results are carried forward unchanged. Version & release infra - nla_eval/__init__.py -> 2.0.0; CITATION.cff version 2.0.0 + abstract (128 plans / 14 families). - New CHANGELOG.md (v1 + v2 entries) following the freeze-on-release policy. - VERSIONING.md rewritten around plain v1/v2 generation labels (keeps the freeze policy; maps the old v0.1->v1, v0.2->v2; clarifies that a checkpoint's own "v0.1" version names the NLA artifact, not the benchmark generation). - docs/ROADMAP_v0.2.md -> docs/ROADMAP_v2.md; reframed so the CTF family is "shipped in v2.0.0" and the multi-domain expansion + H1-H12 harness fixes are the tracked v2.x roadmap (not claimed in the tagged release). Catalog & naming - Family N (CTF Red/Blue) promoted from "v0.2 additive" to a first-class v2 family. Counts reconciled to 128 plans / 14 families (v2); families A-M (P001-P118) remain the frozen v1 catalog. Updated INDEX, plans/README, EVALUATIONS, CTF_RED_BLUE, ctf.py, the demo, and the hero-figure prompt. README & results - README simplified into a single-screen front page; v1 results table kept visible and labeled frozen. - RESULTS.md: v1 findings labeled frozen; new "v2 additions" section (CTF control-tier demo + the domain-aware honest-negative), with the control-vs-primary integrity caveat stated. - New results/README.md provenance map (which artifact belongs to which generation; v1 never overwritten). Honesty: the tagged v2.0.0 ships the new adversarial Red/Blue CTF family plus release maturation; it does not fabricate domain-coverage or fix-dependent numbers it has not produced. Note: PR #21 (versioning scaffold) and PR #22 (domain-aware result) are already merged to master, so this branch already contains them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMuBSoDsAPwNSvev9nmvVr
1 participant
Summary
docs/VERSIONING.md— semantic versioning policy for nla-eval-harnessdocs/ROADMAP_v0.2.md— planned features and milestones for v0.2nla_eval/__init__.pyversion markerTest plan
🤖 Generated with Claude Code