Hardening gemma4 - #31
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 270fb6d1d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _name_pattern = re.compile( | ||
| r"(?i)(?P<prefix>\b(?:my\s+name\s+is|full\s+name\s*[:=]|name\s*[:=])\s*)" | ||
| r"(?P<value>[A-Z][A-Za-z'-]+(?:\s+[A-Z][A-Za-z'-]+){1,3})" |
There was a problem hiding this comment.
Remove the global case-insensitive flag from the name value or otherwise enforce title casing. Because (?i) also makes [A-Z] case-insensitive, ordinary code and YAML such as name: npm install or name: Run tests is classified as a person's name. Extraction anonymizes all user and assistant text by default, so these common snippets are replaced with redacted_pii_* and the resulting training examples are corrupted.
Useful? React with 👍 / 👎.
| parts.append(text[cursor:match.end()]) | ||
| if not text.startswith(_GEMMA_THOUGHT_PREFIX, match.end()): | ||
| parts.append(suffix) |
There was a problem hiding this comment.
Identify model-turn boundaries from the message structure rather than scanning the rendered string for every _GEMMA_TURN_START_PATTERN match. For affected non-thinking Gemma 4 templates, user or system content containing the literal <|turn>model\n—for example, a discussion of the chat template—also reaches this insertion path, so Teich injects an empty thought block into the original content; downstream Gemma span inference can then additionally treat the remainder as an assistant target.
Useful? React with 👍 / 👎.
| marker = f"[redacted media {self._dummy_sequence(value, 16)}]".encode("utf-8") | ||
| self.counts["media"] += 1 | ||
| return base64.b64encode(marker).decode("ascii") |
There was a problem hiding this comment.
Replace media with a valid placeholder of the declared type, or remove the media block entirely. This encodes a plain-text marker as base64 while leaving fields such as media_type: image/png unchanged, so any multimodal loader that decodes the anonymized payload as the declared image, audio, or video format receives invalid bytes and can fail before the dataset is prepared.
Useful? React with 👍 / 👎.
| if queue.full(): | ||
| try: | ||
| queue.get_nowait() |
There was a problem hiding this comment.
Avoid silently removing the oldest chunk when a terminal produces output faster than the websocket can send it. Once this 256-item queue fills, get_nowait() permanently discards bytes from the live stream; large command output is therefore incomplete, and dropping a chunk in the middle of an ANSI escape sequence can also desynchronize the xterm display. Coalescing queued chunks or replaying from the terminal scrollback would bound memory without losing output.
Useful? React with 👍 / 👎.
2 participants
No description provided.