mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(transforms/code): coerce language aliases instead of raising (#1975)
## Description
`CodeAwareCompressor.compress()` picks the language for AST-based
compression like this
(`headroom/transforms/code_compressor.py`):
```python
if language:
detected_lang = CodeLanguage(language.lower()) # <-- raises on anything not an exact enum value
confidence = 1.0
elif self.config.language_hint:
detected_lang = CodeLanguage(self.config.language_hint.lower())
confidence = 1.0
else:
detected_lang, confidence = detect_language(code)
```
`CodeLanguage` only accepts
`python`/`javascript`/`typescript`/`go`/`rust`/`java`/`c`/`cpp`/`perl`.
The very common markdown fence tags and hints — `js`, `ts`, `py`, `jsx`,
`tsx`, `node`, `rs`,
`c++` — are **not** enum values, so `CodeLanguage("js")` raises
`ValueError`. That construction
is *above* the method's own `try/except`, so:
- **Direct callers** — `CodeAwareCompressor().compress(code,
language="js")` and the module-level
`compress_code(code, language="js")` — crash with an uncaught
`ValueError`.
- **In the router (mixed content):** `split_into_sections` extracts the
raw fence tag
(`_CODE_FENCE_PATTERN` captures `\w*`, e.g. `js`) into
`ContentSection.language`, and that string
is passed straight into `compress(...)`. The `ValueError` is swallowed
by the outer `try/except`
in the strategy dispatch, so a ` ```js ` / ` ```ts ` / ` ```py ` block
silently **skips
code-aware compression** even when `enable_code_aware=True`, falling
back to the generic path.
So the three most common web/scripting languages, written with their
usual fence tags, never get
the structure-aware compressor.
Closes: no issue filed — found while auditing the code-compression
language path.
## Fix
Add a `coerce_language()` helper that maps common aliases/fence tags to
the canonical
`CodeLanguage` and returns `CodeLanguage.UNKNOWN` (never raises) for
anything unrecognized.
`compress()` now coerces the hint and, when the result is `UNKNOWN`,
falls back to
content-based `detect_language(code)` instead of constructing the enum
directly:
```python
if language:
detected_lang = coerce_language(language)
if detected_lang == CodeLanguage.UNKNOWN:
detected_lang, confidence = detect_language(code)
else:
confidence = 1.0
```
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `headroom/transforms/code_compressor.py`: add `_LANGUAGE_ALIASES` and
`coerce_language()`; use them in `compress()` for both the `language`
argument and `config.language_hint`, with a content-detection fallback
on `UNKNOWN`.
- `tests/test_code_compressor_language_alias.py`: cover alias mapping,
canonical passthrough, case/whitespace handling, unknown-returns-UNKNOWN
(no `ValueError`), and that `compress(language="js")` no longer raises.
## Testing
- [x] New regression tests added
(`tests/test_code_compressor_language_alias.py`)
- [x] Linting passes (`ruff check`) and formatting is clean (`ruff
format --check`)
- [ ] Full `pytest` deferred to CI (local-OOM reason below).
```text
$ uv run ruff check headroom/transforms/code_compressor.py tests/test_code_compressor_language_alias.py
All checks passed!
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.10, headroom from this branch.
Importing `headroom` pulls in the torch/transformers stack and a full
`pytest` gets OOM-killed on this box, so I verified the coercion logic
with a dependency-free script (replicating the enum + helper) and left
the full pytest to CI.
- Exact command / steps: ran the common aliases and the canonical values
through both the old `CodeLanguage(value.lower())` construction and the
new `coerce_language()`.
- Observed result: the old construction raises `ValueError` on every
alias (the crash / silent-skip); the new helper maps them and never
raises:
```text
OK alias 'js': old raised ValueError -> new maps to javascript
OK alias 'ts': old raised ValueError -> new maps to typescript
OK alias 'py': old raised ValueError -> new maps to python
OK alias 'jsx': old raised ValueError -> new maps to javascript
OK alias 'node': old raised ValueError -> new maps to javascript
OK canonical values pass through
OK case-insensitive + trimmed
OK unknown -> UNKNOWN (no ValueError)
LANGUAGE COERCION VERIFIED
```
- Not tested: running a full mixed-content document with ` ```js `
fences through a booted compression pipeline (needs the heavy stack).
The unit tests exercise the coercion directly and the
`compress(language="js")` entry point. Full local `pytest` deferred to
CI (OOM, per above).
## Review Readiness
- [x] I have performed a self-review
- [x] This PR is ready for human review
## Checklist
- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes — ran
lint + a standalone logic check; full pytest deferred to CI (local OOM,
disclosed above)
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
- No new dependencies; a small lookup table plus a helper and a
call-site change.
- @JerrettDavis tagging you — this one silently disables code-aware
compression for the most common fence tags (`js`/`ts`/`py`), so it may
be worth a look when you have a moment.
This commit is contained in:
parent
69fd2189a3
commit
27ddde1f5e
3 changed files with 128 additions and 5 deletions
|
|
@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
* **install:** stop leaking a file descriptor on every `headroom install start`. `start_detached_agent()` opened the agent log file and handed it to `subprocess.Popen` but never closed the parent's copy, so each call leaked one fd (and pinned the log file open against rotation). The parent now closes its copy in a `try/finally` once the child has inherited it — the close also runs if `Popen` raises ([#1554](https://github.com/headroomlabs-ai/headroom/issues/1554)).
|
||||
* **memory/sync:** stop the Codex AGENTS.md sync adapter from erasing previously-synced memories on every export. `sync_export` hands each adapter only the *delta* (memories the agent lacks), but `CodexAdapter.write_memories` rebuilt its whole managed section from just that delta — so each sync overwrote the section with only the new items, thrashing the file between disjoint subsets and never accumulating. It now merges the delta into the facts already present (deduped), matching the additive contract the ClaudeCode adapter already follows.
|
||||
* **memory/sync:** stop the Claude Code sync adapter from clobbering distinct memories that share a first line. `write_memories` derived each file name from the first line of the content only (`headroom_{slug}.md`), so two different DB memories whose first lines slugify identically wrote to the same file and the second silently overwrote the first — and because the loser never landed on disk, the next sync re-exported it, ping-ponging the pair forever. When the slug is already taken by a *different* memory (distinct `headroom_id`) the file name is now disambiguated with a content-hash suffix; an update to the same memory still rewrites its slug file in place, so existing file names are unchanged.
|
||||
* **transforms/code:** stop raising `ValueError` on common language hints and fence tags. `CodeAwareCompressor.compress()` built the language with `CodeLanguage(language.lower())`, which only accepts the exact enum values (`python`/`javascript`/`typescript`/…). A markdown ` ```js ` / ` ```ts ` / ` ```py ` fence tag (or any caller passing an alias) raised `ValueError` — crashing direct callers, and inside the content router the error was swallowed so those blocks silently skipped code-aware compression. A new `coerce_language` helper maps the common aliases to their canonical language and returns `UNKNOWN` (never raises) for unrecognized tags, falling back to content-based detection.
|
||||
* **cli/proxy:** honor `HEADROOM_MIN_TOKENS=0` / `HEADROOM_MAX_ITEMS=0`. The Click `proxy` command built these with `_get_env_int_optional(name) or 500`/`or 50`, so an explicit `0` — a legitimate value (`min_tokens_to_crush=0` means "crush every item") — was treated as falsy and silently replaced with the default. The `headroom proxy` argparse path already preserved `0` via `_get_env_int`, so the two entry points disagreed. The Click path now uses the same None-checking helper.
|
||||
* **proxy:** strip the inbound `Content-Encoding`/`Transfer-Encoding` request headers on the Anthropic `/v1/messages` and OpenAI `/v1/chat/completions` paths before forwarding upstream. `read_request_json_with_bytes` already decompresses the inbound body (zstd/gzip/deflate/br), so the bytes forwarded upstream are plain JSON — but these two handlers left the original `content-encoding` header in place, so a client (or an edge proxy like a Cloudflare Worker) that sent a compressed body got its request rejected with upstream HTTP 400 because the provider tried to decompress already-decoded JSON. The `/v1/responses` handler already carried this fix (#1542); it is now applied to the messages and chat paths too.
|
||||
* **models:** fix the model registry's prefix fallback silently returning the wrong context window. `ModelRegistry.get` accepted any registered name as a `str.startswith` prefix and returned the *first* match, so `gpt-4-32k-0613` resolved to `gpt-4` (8192) instead of `gpt-4-32k` (32768), and unregistered ids like `gpt-4.1`/`gpt-4.5` inherited `gpt-4`'s 8192-token window — making the proxy think a nearly-empty context was almost full and compress far too aggressively. The fallback now requires the registered name to end at a version boundary in the query (so `gpt-4.1` no longer matches `gpt-4`) and picks the longest qualifying name (so `gpt-4-32k-0613` → `gpt-4-32k`).
|
||||
|
|
|
|||
|
|
@ -219,6 +219,49 @@ class CodeLanguage(Enum):
|
|||
UNKNOWN = "unknown"
|
||||
|
||||
|
||||
# Common language hints and markdown fence tags that are not the canonical
|
||||
# ``CodeLanguage`` value. Mapping them here keeps ``` ```js ``` / ``` ```ts ```
|
||||
# / ``` ```py ``` fenced blocks (and callers that pass an alias) on the
|
||||
# code-aware path instead of raising ValueError.
|
||||
_LANGUAGE_ALIASES: dict[str, CodeLanguage] = {
|
||||
"js": CodeLanguage.JAVASCRIPT,
|
||||
"jsx": CodeLanguage.JAVASCRIPT,
|
||||
"mjs": CodeLanguage.JAVASCRIPT,
|
||||
"cjs": CodeLanguage.JAVASCRIPT,
|
||||
"node": CodeLanguage.JAVASCRIPT,
|
||||
"ts": CodeLanguage.TYPESCRIPT,
|
||||
"tsx": CodeLanguage.TYPESCRIPT,
|
||||
"py": CodeLanguage.PYTHON,
|
||||
"python3": CodeLanguage.PYTHON,
|
||||
"golang": CodeLanguage.GO,
|
||||
"rs": CodeLanguage.RUST,
|
||||
"c++": CodeLanguage.CPP,
|
||||
"cxx": CodeLanguage.CPP,
|
||||
"cc": CodeLanguage.CPP,
|
||||
"hpp": CodeLanguage.CPP,
|
||||
"pl": CodeLanguage.PERL,
|
||||
}
|
||||
|
||||
|
||||
def coerce_language(value: str) -> CodeLanguage:
|
||||
"""Map a language hint or markdown fence tag to a ``CodeLanguage``.
|
||||
|
||||
Accepts the canonical enum values and common aliases/fence tags
|
||||
(``js``/``ts``/``py``/...). Unknown strings return ``CodeLanguage.UNKNOWN``
|
||||
instead of raising ``ValueError`` from ``CodeLanguage(value)``, so an
|
||||
unrecognized fence tag falls back to content-based detection rather than
|
||||
crashing the caller (or, inside the router, silently skipping code-aware
|
||||
compression because the ValueError is swallowed).
|
||||
"""
|
||||
key = (value or "").strip().lower()
|
||||
if not key:
|
||||
return CodeLanguage.UNKNOWN
|
||||
try:
|
||||
return CodeLanguage(key)
|
||||
except ValueError:
|
||||
return _LANGUAGE_ALIASES.get(key, CodeLanguage.UNKNOWN)
|
||||
|
||||
|
||||
class DocstringMode(Enum):
|
||||
"""How to handle docstrings."""
|
||||
|
||||
|
|
@ -1015,13 +1058,22 @@ class CodeAwareCompressor(Transform):
|
|||
syntax_valid=True,
|
||||
)
|
||||
|
||||
# Detect or use specified language
|
||||
# Detect or use specified language. An explicit hint or fence tag may be
|
||||
# an alias (js/ts/py/...) or something we don't recognize — coerce it
|
||||
# instead of constructing CodeLanguage() directly (which raises), and
|
||||
# fall back to content detection when the hint is unknown.
|
||||
if language:
|
||||
detected_lang = CodeLanguage(language.lower())
|
||||
confidence = 1.0
|
||||
detected_lang = coerce_language(language)
|
||||
if detected_lang == CodeLanguage.UNKNOWN:
|
||||
detected_lang, confidence = detect_language(code)
|
||||
else:
|
||||
confidence = 1.0
|
||||
elif self.config.language_hint:
|
||||
detected_lang = CodeLanguage(self.config.language_hint.lower())
|
||||
confidence = 1.0
|
||||
detected_lang = coerce_language(self.config.language_hint)
|
||||
if detected_lang == CodeLanguage.UNKNOWN:
|
||||
detected_lang, confidence = detect_language(code)
|
||||
else:
|
||||
confidence = 1.0
|
||||
else:
|
||||
detected_lang, confidence = detect_language(code)
|
||||
|
||||
|
|
|
|||
70
tests/test_code_compressor_language_alias.py
Normal file
70
tests/test_code_compressor_language_alias.py
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
"""Regression tests for language-hint / fence-tag coercion in code_compressor.
|
||||
|
||||
`CodeAwareCompressor.compress(code, language=...)` used to build the language
|
||||
with `CodeLanguage(language.lower())`, which raises `ValueError` for anything
|
||||
that is not an exact enum value. Common markdown fence tags and hints — `js`,
|
||||
`ts`, `py` — are not enum values, so:
|
||||
|
||||
* direct callers (`compress(code, language="js")`) crashed, and
|
||||
* inside the router the ValueError was swallowed, so ` ```js ` / ` ```ts ` /
|
||||
` ```py ` fenced blocks silently skipped code-aware compression.
|
||||
|
||||
`coerce_language` maps aliases to the canonical language and returns UNKNOWN
|
||||
(never raises) for unrecognized tags, letting the caller fall back to
|
||||
content-based detection.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from headroom.transforms.code_compressor import CodeLanguage, coerce_language
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"alias,expected",
|
||||
[
|
||||
("js", CodeLanguage.JAVASCRIPT),
|
||||
("jsx", CodeLanguage.JAVASCRIPT),
|
||||
("node", CodeLanguage.JAVASCRIPT),
|
||||
("ts", CodeLanguage.TYPESCRIPT),
|
||||
("tsx", CodeLanguage.TYPESCRIPT),
|
||||
("py", CodeLanguage.PYTHON),
|
||||
("python3", CodeLanguage.PYTHON),
|
||||
("golang", CodeLanguage.GO),
|
||||
("rs", CodeLanguage.RUST),
|
||||
("c++", CodeLanguage.CPP),
|
||||
],
|
||||
)
|
||||
def test_coerce_language_maps_common_aliases(alias, expected):
|
||||
assert coerce_language(alias) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"canonical",
|
||||
["python", "javascript", "typescript", "go", "rust", "java", "c", "cpp", "perl"],
|
||||
)
|
||||
def test_coerce_language_accepts_canonical_values(canonical):
|
||||
assert coerce_language(canonical) == CodeLanguage(canonical)
|
||||
|
||||
|
||||
def test_coerce_language_is_case_insensitive_and_trims():
|
||||
assert coerce_language(" JS ") == CodeLanguage.JAVASCRIPT
|
||||
assert coerce_language("Python") == CodeLanguage.PYTHON
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", ["", " ", "not-a-language", "brainfuck", "yaml"])
|
||||
def test_coerce_language_unknown_returns_unknown_not_valueerror(value):
|
||||
# The whole point: never raise, so an unrecognized fence tag can fall back
|
||||
# to content detection instead of crashing / being swallowed.
|
||||
assert coerce_language(value) == CodeLanguage.UNKNOWN
|
||||
|
||||
|
||||
def test_compress_with_alias_language_does_not_raise():
|
||||
"""The direct API path must not raise on a common alias."""
|
||||
from headroom.transforms.code_compressor import CodeAwareCompressor
|
||||
|
||||
code = "function add(a, b) {\n return a + b;\n}\n"
|
||||
compressor = CodeAwareCompressor()
|
||||
# Before the fix this raised ValueError: 'js' is not a valid CodeLanguage.
|
||||
result = compressor.compress(code, language="js")
|
||||
assert result is not None
|
||||
assert result.compressed is not None
|
||||
Loading…
Add table
Add a link
Reference in a new issue