Skip to content

fix(compression): honor per-style boundaries and add the ponytail safety carve-out - #13938

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
woodsonl:fix/output-style-safety-boundaries
Sep 29, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
woodsonl:fix/output-style-safety-boundaries

Conversation

@woodsonl

@woodsonl woodsonl commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two output-style defects, feature-only scope. This is split (1) of the three you proposed.

Rebased on the release/v3.8.51 tip 3ee283bb9. 1 commit, 5 files, merges clean.

The blocker you flagged is resolved

You were right that the base-reds block was obsolete and contradicted the tip. Verified on a pristine checkout of 3ee283bb9:

  • the 16 unit-test files that failed on earlier tips: 96/96 pass
  • check:api-typecheck: OK, 283 pre-existing, within baseline
  • check:env-doc-sync: in sync
  • generate-agent-skills: 46 skills, nothing to generate
  • mutation-test-coverage: no drift

Every check green on the tip stays green with this PR applied.

Defect 1 — the ponytail safety carve-out was omitted by the original port

git log -S"simplify away" --all agrees with your finding: the clause only ever lived in skills/ponytail/SKILL.md. It is an omission from port #7781. Ported as an exported SAFETY_BOUNDARIES constant on ponytail and less-code, translated across all 10 style languages.

Defect 2 — OutputStyle.boundaries declared but never read

Dead field at outputStyles/catalog.ts:19, now resolved in buildStyleBoundaries() (outputStyles/apply.ts).

Your swap-or-sum question: SUM. SHARED_BOUNDARIES stays the base clause and a style's own boundaries is appended on top, deduped, in catalog order, so a code-shaping style keeps "Code blocks, file paths, commands, errors, URLs: keep exact". A style declaring nothing (terse-prose) stays byte-identical to the legacy injection (D-A5, pinned by test). Per-(selection, language) output is static and prompt-cache stable (D-A4).

Title fixed too: "restore" is gone, since as you showed there was no deliberate removal to revert.

Scoped out at your request

My own carve-out, for your call

⚠️ One-time prompt-cache invalidation

Every style's effective instruction gains a boundary clause, so the first request after deploy re-pays the system-prompt cache per (style, level, language).

Testing

  • 72/72 across output-styles-*, ponytail-catalog and i-have-adhd-catalog on the rebased branch.
  • New output-styles-boundaries.test.ts: sum semantics, dedupe, catalog order, localization, determinism, byte-identity anchor. The injection helper reads the top-level system placement from fix(compression): place output-style instruction in top-level system, not messages[0] #13383.
  • npm run typecheck:core clean, check:mutation-test-coverage no drift.
  • CI on head 31a5008: Quality Gates, semgrep, API Route Typecheck all pass.
  • Note: tests/unit/compression/ runs under node:test, not vitest.

@diegosouzapw

Copy link
Copy Markdown
Owner

A feature é boa e vale ⭐4 isolada. O que segura é o que veio junto.

Sobre o título: rodei git log -S"simplify away" --all e a cláusula nunca esteve no catálogo de compressão — só existe em skills/ponytail/SKILL.md:100 e nos commits que a adicionaram lá. Não houve remoção deliberada a ser revertida; é omissão do port original (#7781). Isso na verdade é a favor da PR — não há decisão de ninguém sendo desfeita — mas o "restore" no título confunde quem for revisar.

O defeito 2 confere: outputStyles/catalog.ts:19 declara boundaries?: string e nada no repo lê meta.boundaries. Campo morto real.

Bloqueador — o bloco de base-reds está obsoleto e contradiz o tip. Os 8 arquivos em conflito são todos daquele bloco, e a base já consertou tudo por conta própria com valores diferentes: provider-node-reserved-prefix.test.ts no tip diz RESERVED_PREFIX_COUNT === 413 e a PR insiste em 414; providers/[id]/test/route.ts no tip já usa \u0000-\u001f e a PR propõe \x00-\x1f. Mergear esse bloco reintroduz vermelho.

Fora de escopo: config/quality/eslint-suppressions.json com poda de 36 entradas (rebaseline de qualidade é decisão do dono, não entra de carona), e src/sse/services/auth.ts:1471, que embute uma mudança de comportamento de autenticação (gatear blockedByKeyPolicy com && !forcedConnectionId, reconciliando #12080 com #13832/#13879). Pode até estar certa, mas precisa de PR própria com teste dos dois contratos.

Decisão do dono sobre a feature em si: os levels de ponytail e less-code embutem ${SHARED_BOUNDARIES} e buildStyleInstructions faz .replace(SHARED_BOUNDARIES,""). Com boundaries: SAFETY_BOUNDARIES, esses dois estilos perdem o clause compartilhado ("Code blocks, file paths, commands, errors, URLs: keep exact"). Para estilos que moldam código, perder "keep code blocks exact" é troca discutível — devem trocar ou somar os dois?

Sugestão: dividir em três. (1) a feature de boundaries, rebaseada no tip e sem nada do bloco de base-reds; (2) o auth.ts como fix próprio; (3) descartar o bloco de base-reds e a poda de suppressions. Os testes (output-styles-boundaries.test.ts, 11 casos) são a parte mais sólida e sobrevivem inteiros.

@woodsonl
woodsonl force-pushed the fix/output-style-safety-boundaries branch from 7ed2735 to f96c168 Compare September 18, 2026 03:58
@woodsonl woodsonl changed the title fix(compression): restore ponytail safety carve-out and wire per-style boundaries fix(compression): honor per-style boundaries and add the ponytail safety carve-out Sep 18, 2026
@woodsonl

Copy link
Copy Markdown
Contributor Author

Feito — reescopada exatamente como sugerido. Branch resetada no tip 1603c86e0, force-push; o PR agora tem 1 commit e 5 arquivos:

open-sse/services/compression/outputStyles/catalog.ts
open-sse/services/compression/outputStyles/apply.ts
tests/unit/compression/output-styles-boundaries.test.ts        (novo)
tests/unit/compression/output-styles-i18n-matrix.test.ts
changelog.d/fixes/13938-ponytail-safety-boundaries.md

Título corrigido — "restore" não se sustenta mesmo: o git log -S"simplify away" --all confirma que a cláusula só existiu em skills/ponytail/SKILL.md. Agora é fix(compression): honor per-style boundaries and add the ponytail safety carve-out (omissão do port #7781, não reversão).

Swap vs sum → SUM. buildStyleBoundaries() sempre emite SHARED_BOUNDARIES como cláusula base e soma, deduplicada, a cláusula de cada estilo que declara boundaries, em ordem de catálogo. Assim ponytail/less-code ganham o carve-out sem perder "Code blocks, file paths, commands, errors, URLs: keep exact" — que era a preocupação correta. terse-prose (sem boundaries) continua byte-idêntico ao injection legado (D-A5), pinado por teste.

Bloco de base-reds descartado inteiro (suppressions, stryker, NUL, SVG, fixtures, etc.). Confirmei no tip pristino 1603c86e0 que a base já resolveu tudo por conta própria, com os valores dela (413, \u0000-\u001f).

auth.ts não vira PR próprio — não é mais necessário. sse-auth.test.ts + chat-helpers.test.ts passam 97/97 no tip pristino (rodei num worktree limpo): a base já reconciliou #12080 com #13879/#13832 sem o gate !forcedConnectionId. Não há mudança de auth pendente.

Testes: 68/68 nas suítes output-styles-* + ponytail-catalog + i-have-adhd-catalog; typecheck:core limpo.

⚠️ Nota de transparência: o tip pristino está vermelho por conta própria em pontos fora deste escopo — npm run lint sai 2 (suppressions não podadas), ~13 falhas RTK no tests/unit/compression/, 2 de header-drop em chatcore-translation-paths.test.ts e o SVG do budget card desatualizado. Reproduzi num worktree limpo do tip; nada disso vem deste PR, e deixei fora do escopo como pedido.

@woodsonl
woodsonl force-pushed the fix/output-style-safety-boundaries branch from f96c168 to bb106e0 Compare September 18, 2026 23:16
@woodsonl

Copy link
Copy Markdown
Contributor Author

Rebasei no tip atual 4d1282be3 (#14101, que drenou os base-reds de 09-18). Continua 1 commit / 5 arquivos. A primeira rodada de CI estava vermelha porque eu tinha rebasado em 1603c86e0; no tip novo a maior parte some.

O tip também mudou o placement do instruction: #13383 põe o injection no system top-level, não em messages[0] — ajustei o helper do meu teste e as 11 asserções voltaram a passar (72/72 nas suítes output-styles-* + catálogos).

Confirmei no tip pristino 4d1282be3 o que ainda é herdado (nada disso vem deste PR):

  • check:api-typecheck: 2 erros TS2677 (src/app/api/v1/_shared/rerankProviderNodes.ts, src/mitm/handlers/antigravity.ts)
  • 6 testes unit: chat-route-coverage (emergency fallback ×2), i18n-pt-br (repository contract + drift do en.json), issue-7793-env-doc-sync-repro, rate-limit-maxwaitms-disable-execution
  • gate check:env-doc-sync

O file-size que falhava antes já está verde no tip novo.

…ety carve-out

The original port (diegosouzapw#7781) omitted the ponytail safety carve-out that
upstream ships ('Never simplify away: input validation at trust
boundaries, error handling that prevents data loss, security measures,
or accessibility basics'). It is not a removal — the clause never existed
in the compression catalog. Port it as an exported SAFETY_BOUNDARIES
constant on ponytail and less-code, with translations for the 10 style
languages.

OutputStyle.boundaries was declared but never read (dead field). Resolve
it in buildStyleBoundaries(): SHARED_BOUNDARIES is always the base clause
and a style that declares boundaries contributes its own clause on top,
deduped, in catalog order. The two ADD rather than replace so a
code-shaping style never loses the shared guarantee (e.g. 'keep code
blocks, file paths, commands, errors, URLs exact'). A style that declares
nothing (terse-prose) stays byte-identical to the legacy
instructions + SHARED_BOUNDARIES injection (D-A5); per-(selection,
language) output is static and prompt-cache stable (D-A4).
@woodsonl
woodsonl force-pushed the fix/output-style-safety-boundaries branch from bb106e0 to 31a5008 Compare September 22, 2026 02:08
@woodsonl

Copy link
Copy Markdown
Contributor Author

Rebaseado no tip atual 3ee283bb9 — a base finalmente está limpa. Continua 1 commit / 5 arquivos.

Confirmei num checkout pristino do tip novo que todos os base-reds que o PR herdava sumiram:

  • os 16 arquivos de teste unit que falhavam → 96/96 passam no tip
  • check:api-typecheck → OK (283 pré-existentes, dentro do baseline)
  • check:env-doc-sync → em sync
  • generate-agent-skills → 46 skills, nada a gerar
  • mutation-test-coverage → sem drift

Localmente no branch rebaseado: 72/72 nas suítes output-styles-* + catálogos, typecheck:core limpo, check:mutation-test-coverage sem drift. O CI deve ficar verde agora (fora o que ainda for base-red da release/v3.8.51, que segue NOT release-green na issue #13866).

placeSystemInstruction() appends the styles block after the existing
system text, or as a trailing system message, and never creates a new
messages[0]. The JSDoc said the styles were front-loaded into the
system prompt.
@diegosouzapw
diegosouzapw merged commit 5cb5593 into diegosouzapw:release/v3.8.51 Sep 29, 2026
9 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants