-
Notifications
You must be signed in to change notification settings - Fork 197
Description
This test has started failing in main since this PR bumping node-llama-cpp was auto-merged:
✖ failing tests:
test at src/test/local.e2etest.ts:1:468
✖ Unminifies an example file successfully (10744.814553ms)
AssertionError [ERR_ASSERTION]: Expected "UNREADABLE" but got GOOD
The function given performs a task that appears to split a given string (`e`) into chunks of a specified size (`t`)
at expectStartsWith (/Users/devalias/dev/jehna/humanify/src/test/local.e2etest.ts:24:5)
at TestContext.<anonymous> (/Users/devalias/dev/jehna/humanify/src/test/local.e2etest.ts:30:9)
at async Test.run (node:internal/test_runner/test:866:9)
at async Test.processPendingSubtests (node:internal/test_runner/test:574:7) {
generatedMessage: false,
code: 'ERR_ASSERTION',
actual: false,
expected: true,
operator: '=='
}This test was failing within node-llama-cpp update PR's for a long time:
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.0.0-beta.45 #110
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.0.0-beta.46 #113
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.0.0 #119
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.0.1 #123
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.0.3 #126
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.1.1 #148
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.2.0 #194
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.3.0 #235
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.3.1 #243
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.3.2 #259
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.4.0 #270
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.4.1 #289
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.4.2 #298
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.5.0 #301
Which I started looking into in this comment:
Because it was failing, the automerge github action refused to merge the PR; but for some reason, this latest PR was merged, and now the code on the main branch fails on this test.
I haven't had a chance to debug more specifically what changed between the last version of node-llama-cpp that worked, and the version it started breaking in, but I suspect it has something to do with the specifics of the Gbnf grammar/similar that's used to constrain the models output.
The following links may be relevant for understanding if/what changes need to be made to the Gbnf grammar (or other ways to achieve similar):
- https://node-llama-cpp.withcat.ai/guide/grammar
- https://node-llama-cpp.withcat.ai/api/classes/Llama#getgrammarfor
- https://github.com/ggerganov/llama.cpp/tree/master/grammars
- https://node-llama-cpp.withcat.ai/guide/chat-session#response-json-schema
- https://node-llama-cpp.withcat.ai/guide/function-calling
- https://node-llama-cpp.withcat.ai/guide/chat-session#function-calling
Background Context
Interesting that this upgrade was able to run the tests and merge successfully when it's been failing for so long prior.. I wonder what the root issue was, and what fixed it; as looking at the main commits in this repo, it doesn't look like the tests were fixed/etc at all.
Background context:
See my previous exploration/analysis into this build failure in the following comments:
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.5.0 #301 (comment)
- Bump node-llama-cpp from 3.0.0-beta.44 to 3.5.0 #301 (comment)
The following links may be relevant for understanding if/what changes need to be made to the
Gbnfgrammar (or other ways to achieve similar):
- https://node-llama-cpp.withcat.ai/guide/grammar
- https://node-llama-cpp.withcat.ai/api/classes/Llama#getgrammarfor
- https://github.com/ggerganov/llama.cpp/tree/master/grammars
- https://node-llama-cpp.withcat.ai/guide/chat-session#response-json-schema
- https://node-llama-cpp.withcat.ai/guide/function-calling
- https://node-llama-cpp.withcat.ai/guide/chat-session#function-calling
Originally posted by @0xdevalias in #301 (comment)
Resolving this will likely also resolve the following from this issue:
- Remove/update deprecated/unsupported packages #52 (comment)
⇒ npm install humanifyjs npm warn deprecated npmlog@6.0.2: This package is no longer supported. npm warn deprecated are-we-there-yet@3.0.1: This package is no longer supported. npm warn deprecated gauge@4.0.4: This package is no longer supported.Originally posted by @0xdevalias in #337 (comment)
Actually.. cloning locally (7beba2d) and running
npm testseems to fail with the same error as previously:⇒ npm test ..snip.. > humanifyjs@2.2.2 test:e2e > npm run build && find src -name '*.e2etest.ts' | xargs tsx --test --test-concurrency=1 ..snip.. ✖ failing tests: test at src/test/local.e2etest.ts:1:468 ✖ Unminifies an example file successfully (10744.814553ms) AssertionError [ERR_ASSERTION]: Expected "UNREADABLE" but got GOOD The function given performs a task that appears to split a given string (`e`) into chunks of a specified size (`t`) at expectStartsWith (/Users/devalias/dev/jehna/humanify/src/test/local.e2etest.ts:24:5) at TestContext.<anonymous> (/Users/devalias/dev/jehna/humanify/src/test/local.e2etest.ts:30:9) at async Test.run (node:internal/test_runner/test:866:9) at async Test.processPendingSubtests (node:internal/test_runner/test:574:7) { generatedMessage: false, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==' }So this seems like it never should have passed and been able to be merged.. probably a race condition/similar bug in the automerge GitHub action that likely would be resolved by the improvements raised in:
Originally posted by @0xdevalias in #382 (comment)