Repository navigation
test(compression): make output-style checks able to fail - #15614
Merged
diegosouzapw merged 3 commits intoOct 6, 2026
Merged
diegosouzapw merged 3 commits into
diegosouzapw merged 3 commits into
Conversation
diegosouzapw#13383 moved the output-style instruction out of messages[0]. On an OpenAI-shaped body with no system turn, placeSystemInstruction now appends a trailing system message. diegosouzapw#14423 repointed the five assertions that failed after that change. Five tests still carried checks that could not fail: - output-styles-backcompat golden: it read messages[0] from both injectors' results, which is the untouched user turn. Once strip() drops the first line, both sides are "", so it passed even when the two instruction texts diverged. - "caveman output mode skipped when compression is globally disabled": it checked that messages[0] is the user turn, which still holds with the config.enabled gate removed, because the instruction lands at the end of the array. - The diegosouzapw#6240 compression:off test: its messages[0] assertion can no longer fail for the same reason, and its marker scan read only message text. - The two compression-combo tests: they pinned the global language default to pt-BR, so they passed even when chatCore ignored the combo's language packs. The golden test now reads each injector's system message, checks that it starts with its own marker, and compares the text below the marker. It also covers a bypass turn, "Explain this security vulnerability in detail." With Auto-Clarity off, both injectors apply the same text. With Auto-Clarity on, both skip the turn as security_warning. The disabled-compression and compression:off tests deep-equal the untouched messages and assert that the [OmniRoute Output Styles] marker appears nowhere in the upstream body. The disabled-compression test moves to test-model-compression-off-6240.test.ts and runs through that file's runChatCore helper, which keeps chatcore-compression-integration.test.ts under its 1,214-line freeze. Its explicit-any suppression count in config/quality/eslint-suppressions.json drops from 9 to 8 with it. The 6240 file's storage reset now waits up to 30 seconds for the previous test's background call-log save and fails if it does not land, so the diegosouzapw#2101 test's save cannot race the moved test's reset. Its teardown closes the call-log writer, so the file exits on its own, and its temporary data directory now comes from the shared createTempDataDir helper in tests/_setup/tempDataDir.ts. The combo tests set the global default to English, so pt-BR output has to come from the combo. tests/unit/compression/output-styles-anthropic-placement.test.ts is new. It sends Claude Code-style /v1/messages requests through chatCore with three system shapes: a string, content blocks, and none. The upstream system field must carry the full instruction exactly once, in blocks whose text is a string, next to the client's own prompt, and messages[] must keep the client's turns. The two shapes with a system prompt also carry a later system turn, which must stay in messages[]. That shows chatCore took the mid-conversation-system path and did not hoist every system turn. The file takes its temporary data directory from the shared createTempDataDir helper. Each test waits up to 30 seconds for the call-log writer to finish before the next one resets storage, and fails if it does not. The file closes the writer at the end, so it exits on its own. It lives with the unit tests so the unit shards that gate pull requests into release branches run it.
…mmit missed The pass-2 review of 824ceda (five specialist lenses, adversarial pass, red-team sweep, independent refuter votes) upheld nine advisory findings; this applies the in-scope ones with the drain pattern that commit already established in its two sibling files: - chatcore-compression-integration.test.ts now uses the shared createTempDataDir helper, asserts waitForCallLogSaves before every resetDbInstance, and drains + closes the call-log writer in test.after. Every green run of the two tests this diff edits logged 'The database connection is not open' (measured twice per run by the review); the file also kept the racy teardown the moved test was fixed for. - test-model-compression-off-6240.test.ts and output-styles-anthropic-placement.test.ts drain the last test's save before closeCallLogSaves, so the final in-flight save lands instead of erroring during close (1 and 3 residual lines per green run, now 0). - the two pt-BR combo-test comments stated as present fact what the autoDetect detector 'finds' while the fixture sets autoDetect: false; they now state the actual mechanism (detector off, global default 'en'). - the golden back-compat and placement tests pin their scope: the legacy-vs-unified identity and placement contracts are asserted for the English instruction only. - file-size-baseline.json: the chatcore freeze tracked the pre-split size (1214); the file gate-counts 1145 now, so the entry is tightened to the measured size per the baseline's exact-LOC convention. - eslint-suppressions.json: drop six entries whose violations no longer occur on this tree (stale-enforcement fails a cold-cache lint run). Verification: all four changed test files pass with zero residual call-log error lines (chatcore 13, 6240 3, placement 3, backcompat 6); npm run lint, Prettier and check:file-size are clean. The 20 production mutations banked on the parent commit all remain caught: no assertion changed, only teardown, comments and scope notes.
…t-style-placement-asserts
diegosouzapw
merged commit Oct 6, 2026
f07c521
into
diegosouzapw:release/v3.8.52
41 of 51 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
messages[0]. This makes them able to fail, and (second commit) finishes the call-log drain the pre-landing review found missing on the paths the first commit touched.tests/unit/compression/output-styles-backcompat.test.ts): it readmessages[0]from both injectors' results, which is the untouched user turn. Oncestrip()drops the first line, both sides were"", so it passed even when the two instruction texts diverged. It now compares the injected instruction text itself.chatcore-compression-integration.test.tsintotests/integration/test-model-compression-off-6240.test.ts. Itsmessages[0]check still held with the config gate removed, because the instruction lands at the end of the array.tests/unit/compression/output-styles-anthropic-placement.test.ts): driveshandleChatCorewith Claude Code-style/v1/messagesrequests and asserts exactly-once placement in the top-levelsystemfield with string-typed text blocks, plus the mid-conversation-system path.c84434577a):chatcore-compression-integration.test.tsnow uses the sharedcreateTempDataDirhelper and assertswaitForCallLogSavesbefore everyresetDbInstance(every green run logged "The database connection is not open" before this); the 6240 and placement files drain the final save beforecloseCallLogSaves; the two pt-BR comments no longer narrateautoDetectdetector output the fixture disables withautoDetect: false; the golden and placement tests state their en-only scope.file-size-baseline.jsontightens the chatcore freeze from 1214 to the measured 1145;eslint-suppressions.jsondrops six entries whose violations no longer occur on this tree (a cold-cachenpm run lintfails stale-enforcement otherwise).Related Issues
hukey in the terse-prose i18n map; extra space beforeSHARED_BOUNDARIESfor ja/zh) is tracked for a separate follow-up PR.Validation
node --import tsx/esm --testruns)npm run lint(cold cache, exit 0)npm run check:file-size(exit 0)origin/release/v3.8.52merged in (235e02e715), focused checks re-run on the merged treeTests Added Or Updated
tests/unit/compression/output-styles-backcompat.test.ts(golden checks rewritten so they can fail)tests/integration/test-model-compression-off-6240.test.ts(gained the moved disabled-compression test, the drain, the shared temp dir)tests/unit/compression/output-styles-anthropic-placement.test.ts(new: Anthropic top-level system placement)tests/integration/chatcore-compression-integration.test.ts(drain, shared temp dir, comment accuracy)src/,open-sse/,electron/,bin/untouched).Coverage Notes
Reviewer Notes
c84434577a. Declined with reason: extracting the drain lifecycle into a shared helper (three call sites of a three-line block; revisit if a fourth appears), the bare-run exit-latency claims (refuted 0/2 by the verifiers), and the hu/ja/zh production divergence (outside this diff).eslint-suppressions.json: the release tip already pruned three stale entries; this branch pruned two of the same plus four that were stale at its older base. The reconciled tree lints clean with the merged file (exit 0, cold cache).