Skip to content

test: realign two stale assertions with product behavior (#13313) - #13315

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
huuhungn:fix/test-assertions-vs-product
Sep 18, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
huuhungn:fix/test-assertions-vs-product

Conversation

@anhtahaylove

Copy link
Copy Markdown
Contributor

Two test files assert things the product does not do. Both fail on
release/v3.8.51 before this branch, and neither is a product bug — the
expectations are simply stale. Split out of #13292 so the teardown work can be
reviewed sepa...[truncated]

@anhtahaylove

Copy link
Copy Markdown
Contributor Author

@diegosouzapw — still open and MERGEABLE, test-only (git diff origin/release/v3.8.51 -- src/ is empty). Grouped context for this PR and the other two left over from the 10–11 Sep batch is on #13292: #13292 (comment)

Short version: #13187 has merged, so the concurrency blocker is gone; the red checks here are the repo-wide ones that also fail on release/v3.8.51 and on other contributors' PRs, not something specific to this branch. Happy to rebase or close on request.

The custom-model assertions expected ids prefixed `jina-ai/`, but the catalog
prefixes model ids with the provider alias, which is `jina`. The synced-model
test directly above asserts `jina/` and passes, so the two cases contradicted
each other within the same file.

The expectation predates the alias: the test was written in v3.7.9 (2026-05-04)
and `alias: "jina"` was added in v3.8.36 (2026-06-25).

Aligns the two assertions with the sibling test and with the product. The file
now passes 44/44 (was 43 with 1 failure).

Confirmed the assertions still bite: renaming the alias to `jina-XX` fails
exactly these two cases.
Both tests assert the Portuguese output-style string but configured
languageConfig with enabled:false and defaultLanguage:"en".
resolveOutputStyleLanguage returns "en" on its first line when enabled is not
true, so the English pack was injected and the assertion could never hold.

autoDetect:true would not have helped either: the user turns in these fixtures
are English, so the detector resolves back to "en". The pack under test has to
be pinned, hence autoDetect:false with an explicit defaultLanguage.

This restores coverage rather than just turning the suite green. Mutating the
pt-BR pack string in outputMode.ts now fails exactly these two tests; with the
old fixture the file reported 9 pass / 2 fail whether the pack was intact or
mutated, so it detected nothing. File is 11/11 (was 9 + 2 failures).

The third languageConfig fixture in this file belongs to an rtk test that makes
no language assertion and is left untouched.
@anhtahaylove
anhtahaylove force-pushed the fix/test-assertions-vs-product branch from 7a89889 to 1894077 Compare September 13, 2026 13:36
@anhtahaylove

Copy link
Copy Markdown
Contributor Author

Correction to my previous comment.

I called this PR test-only and backed that with git diff origin/release/v3.8.51 -- src/ being empty. That check was wrong: src/ exists (2270 non-test .ts files), but it is not the only product tree — open-sse/ holds another 1558. Scoping the diff to src/ alone silently hid every change under open-sse/. I should have diffed the whole tree.

Re-checked properly, this PR really is test-only — both files are tests:

  • tests/integration/chatcore-compression-integration.test.ts
  • tests/unit/models-catalog-route.test.ts

So the conclusion holds here, but it was reached by a broken check, and the same sentence was wrong on #13289 — that one changes open-sse/services/adobeFireflySession.ts, which is product code. Correction posted there too.

Everything else stands: this branch is rebased onto current release/v3.8.51 (1894077e8, behind = 0), tests/unit/models-catalog-route.test.ts reports 44 pass / 0 fail locally, and the red checks are the repo-wide ones that also fail on the base branch.

@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for digging into this. I reproduced both failures on the release tip, and I think the pt-BR half of the diagnosis stops one step short. Evidence below.

The fixture's global languageConfig is not what the combo path reads. applyCompressionComboConfig in open-sse/handlers/chatCore.ts rewrites it from the compression combo before output styles run. With languagePacks: ["pt-BR"] it produces enabled: true, defaultLanguage: "pt-BR", enabledPacks: ["pt-BR"], and it keeps autoDetect: true from settings. So resolveOutputStyleLanguage does not return "en" on its first line. It reaches the auto-detect branch, which gives:

resolveOutputStyleLanguage({ enabled: true, defaultLanguage: "pt-BR", autoDetect: true,  enabledPacks: ["pt-BR"] }, body)  // "en"
resolveOutputStyleLanguage({ enabled: true, defaultLanguage: "pt-BR", autoDetect: false, enabledPacks: ["pt-BR"] }, body)  // "pt-BR"

The user turn is "Resuma esta implementacao.", which is Portuguese but matches none of the LANGUAGE_HINTS words. detectCompressionLanguage returns "en" whenever nothing scores ("zero hits → English"). That fallback then overrides the pack the combo explicitly assigned.

Setting the global config to pt-BR with autoDetect: false makes the test pass. It does so by bypassing the combo-pack path this test was written to cover, not by exercising it. Whether a zero-evidence detection should defer to the combo's defaultLanguage is a product call, so I've left it with the maintainer (noted in #13678). The Jina catalog-prefix half looks right to me.

@diegosouzapw

Copy link
Copy Markdown
Owner

Nice, focused split from #13292 — both stale-assertion fixes check out. Ran
models-catalog-route.test.ts (44/44) and chatcore-compression-integration.test.ts
(11/12, the 1 skip is pre-existing) at your branch head and both are green. Looks
merge-ready to me.

Reverts 8d371d4. The catalog file holds two Jina cases that are not the
same scenario: the custom-model/alias case legitimately expects the
`jina/` alias prefix, while the specialty-model case expects the
canonical provider id `jina-ai/`. Aligning the second to the first reads
like a fix but changes a passing assertion into a failing one.

Verified against the current release tip by mutation, both directions:
the specialty case passes as `jina-ai/...` and fails as `jina/...`, with
the runner reporting `actual: 'jina-ai/jina-embeddings-v5-text-small'`.
The canonical ids are what the source declares — `embeddingRegistry.ts`
and `rerankRegistry.ts` both key the provider as `jina-ai`, as does
EMBEDDING_RERANK_PROVIDER_IDS in src/shared/constants/providers.ts.

The pt-BR language-pack commit on this branch is untouched: that one is a
real fix and repairs two genuinely failing assertions.
@diegosouzapw
diegosouzapw merged commit 1603c86 into diegosouzapw:release/v3.8.51 Sep 18, 2026
10 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…w#13313) (diegosouzapw#13315)

* test: expect the jina alias prefix in the custom-model catalog case

The custom-model assertions expected ids prefixed `jina-ai/`, but the catalog
prefixes model ids with the provider alias, which is `jina`. The synced-model
test directly above asserts `jina/` and passes, so the two cases contradicted
each other within the same file.

The expectation predates the alias: the test was written in v3.7.9 (2026-05-04)
and `alias: "jina"` was added in v3.8.36 (2026-06-25).

Aligns the two assertions with the sibling test and with the product. The file
now passes 44/44 (was 43 with 1 failure).

Confirmed the assertions still bite: renaming the alias to `jina-XX` fails
exactly these two cases.

* test: pin the pt-BR pack in the two language-pack fixtures

Both tests assert the Portuguese output-style string but configured
languageConfig with enabled:false and defaultLanguage:"en".
resolveOutputStyleLanguage returns "en" on its first line when enabled is not
true, so the English pack was injected and the assertion could never hold.

autoDetect:true would not have helped either: the user turns in these fixtures
are English, so the detector resolves back to "en". The pack under test has to
be pinned, hence autoDetect:false with an explicit defaultLanguage.

This restores coverage rather than just turning the suite green. Mutating the
pt-BR pack string in outputMode.ts now fails exactly these two tests; with the
old fixture the file reported 9 pass / 2 fail whether the pack was intact or
mutated, so it detected nothing. File is 11/11 (was 9 + 2 failures).

The third languageConfig fixture in this file belongs to an rtk test that makes
no language assertion and is left untouched.

* test: keep the canonical jina-ai prefix in the catalog case

Reverts 8d371d4. The catalog file holds two Jina cases that are not the
same scenario: the custom-model/alias case legitimately expects the
`jina/` alias prefix, while the specialty-model case expects the
canonical provider id `jina-ai/`. Aligning the second to the first reads
like a fix but changes a passing assertion into a failing one.

Verified against the current release tip by mutation, both directions:
the specialty case passes as `jina-ai/...` and fails as `jina/...`, with
the runner reporting `actual: 'jina-ai/jina-embeddings-v5-text-small'`.
The canonical ids are what the source declares — `embeddingRegistry.ts`
and `rerankRegistry.ts` both key the provider as `jina-ai`, as does
EMBEDDING_RERANK_PROVIDER_IDS in src/shared/constants/providers.ts.

The pt-BR language-pack commit on this branch is untouched: that one is a
real fix and repairs two genuinely failing assertions.

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
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