fix: draw the API key once, and let status answer the dimensions question
Release / release (push) Successful in 6s
Release / release (push) Successful in 6s
Two things a user setting this up hit immediately. The card drew `embeddingApiKey` twice. The render loop walked every entry in FIELDS, and the secret — which is drawn by hand, as a write-only password control — was one of them, so a second control appeared beside it: an ordinary text input, echoing the key in clear as you typed. One key, two boxes, one of them visible. The loop skips secrets now, and the tests count controls rather than checking that each expected label is present, because a duplicate beside the real control passed the old assertions. `status` could not answer the one question a new user cannot look up: how wide is this model? The probe that reports it was gated on every required setting being present, so in exactly the state that needs the answer — dimensions unset — it reported nothing and said "Missing: embedding_dimensions". It now runs whenever the endpoint can be named (a model and a base URL), and the status message names the width it observed: Missing: embedding_dimensions. The endpoint is reachable and returned 768 dimensions, so set embeddingDimensions to 768. The probe also tells "not set yet" apart from "set wrong", because the two have different remedies. Also documents both ways to read the width by hand, since a wrong value is frozen into the index at first use and costs a full re-embed to correct.
This commit is contained in:
1 parent
85c0d4dccd
commit
9ebd484e60
7 files changed
+142
-7
No files matched your search
@@ -632,6 +632,28 @@ test('the API key is a write-only password control that reports only whether one
|
||||
assert.match(findAll(tree, (node) => node.props['data-state'] === true).map((node) => node.children.join('')).join(''), /A key is stored/)
|
||||
})
|
||||
|
||||
test('the API key has exactly one control, and it is never a plain text field', () => {
|
||||
// The regression: the render loop walked every entry in FIELDS, so the secret
|
||||
// drew a second control — an ordinary text input, echoing the key in clear —
|
||||
// beside the write-only password control. One key, two boxes, one of them
|
||||
// visible.
|
||||
const { render } = registered()
|
||||
const tree = render()
|
||||
const labelled = findAll(tree, (node) => node.type === 'input' && node.props['aria-label'] === 'Embedding API key')
|
||||
assert.equal(labelled.length, 1, 'the key must have exactly one control')
|
||||
assert.equal(labelled[0].props.type, 'password')
|
||||
})
|
||||
|
||||
test('the card draws one control per schema field, and no more', () => {
|
||||
// A count, not a list of names: the label assertions above pass just as
|
||||
// happily with a duplicate row sitting beside the real one.
|
||||
const { render } = registered()
|
||||
const controls = findAll(render(), (node) => node.type === 'input')
|
||||
assert.equal(controls.length, 15, 'the schema declares 15 fields, so each gets one control')
|
||||
const labels = controls.map((node) => node.props['aria-label'])
|
||||
assert.equal(new Set(labels).size, labels.length, `duplicate controls: ${labels.join(', ')}`)
|
||||
})
|
||||
|
||||
test('a deployment with no key says so', () => {
|
||||
const { render } = registered({ secrets: [{ path: ['embeddingApiKey'], set: false }] })
|
||||
const tree = render()
|
||||
|
||||
+31
-2
@@ -641,8 +641,37 @@ test('status returns lossless JSON when the endpoint probe SUCCEEDS', async () =
|
||||
}
|
||||
})
|
||||
|
||||
test('status returns lossless JSON when the endpoint probe FAILS', async () => {
|
||||
const { state } = harness({
|
||||
test('status asks the endpoint how wide the model is when embeddingDimensions is unset', async () => {
|
||||
// The question a new user cannot answer from anywhere else. The probe used to
|
||||
// be gated on every required setting being present, which meant it was
|
||||
// unavailable in exactly the state that needs it — so `status` said "Missing:
|
||||
// embedding_dimensions" and left you to guess.
|
||||
const { createServer } = await import('node:http')
|
||||
const server = createServer((request, response) => {
|
||||
response.writeHead(200, { 'content-type': 'application/json' })
|
||||
response.end(JSON.stringify({ data: [{ embedding: new Array(1024).fill(0.01) }] }))
|
||||
})
|
||||
await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve))
|
||||
const { port } = server.address()
|
||||
try {
|
||||
const { state } = harness({
|
||||
embeddingModel: 'ollama/example:latest',
|
||||
embeddingDimensions: 0,
|
||||
embeddingApiBase: `http://127.0.0.1:${port}`,
|
||||
})
|
||||
const value = await state.tools.get('slopo').execute({ action: 'status' }, execContext('/tmp'))
|
||||
assertLossless('status (dimensions unset)', value)
|
||||
assert.deepEqual(value.details.missingSettings, ['embedding_dimensions'])
|
||||
assert.equal(value.details.embeddingEndpoint.checked, true, 'the probe must still run')
|
||||
assert.equal(value.details.embeddingEndpoint.dimensions, 1024, 'the observed width is the answer')
|
||||
assert.match(value.message, /returned 1024 dimensions/)
|
||||
assert.match(value.message, /set embeddingDimensions to 1024/)
|
||||
} finally {
|
||||
await new Promise((resolve) => server.close(resolve))
|
||||
}
|
||||
})
|
||||
|
||||
test('status returns lossless JSON when the endpoint probe FAILS', async () => { const { state } = harness({
|
||||
embeddingModel: 'ollama/example:latest',
|
||||
embeddingDimensions: 768,
|
||||
// Nothing is listening on this port, so the fetch rejects.
|
||||
|
||||
@@ -73,6 +73,22 @@ test('probeEmbeddingEndpoint catches a dimension mismatch, which is what slopo f
|
||||
assert.match(result.error, /returns 768 dimensions but embeddingDimensions is 1024/)
|
||||
})
|
||||
|
||||
test('probeEmbeddingEndpoint reports the width when dimensions is not set yet', async () => {
|
||||
// This is the case a new user is in, and the reason the probe runs before
|
||||
// every required setting exists: nobody can look up how wide an arbitrary
|
||||
// model is, so `dimensions` in the result IS the answer.
|
||||
const result = await probeEmbeddingEndpoint({
|
||||
apiBase: 'http://host:11434',
|
||||
model: 'ollama/m:latest',
|
||||
dimensions: 0,
|
||||
fetchImpl: fakeFetch({ body: { data: [{ embedding: new Array(768).fill(0) }] } }),
|
||||
})
|
||||
assert.equal(result.ok, false, 'unset is not a match')
|
||||
assert.equal(result.dimensions, 768, 'the observed width is reported regardless')
|
||||
assert.equal(result.expected, 0)
|
||||
assert.match(result.error, /returns 768 dimensions and embeddingDimensions is not set/)
|
||||
})
|
||||
|
||||
test('probeEmbeddingEndpoint reports an HTTP error instead of throwing', async () => {
|
||||
const result = await probeEmbeddingEndpoint({
|
||||
apiBase: 'http://host:11434',
|
||||
|
||||
Reference in new issue
Block a user