Skip to content

test(server): strengthen regression coverage and harden query-window search - #734

Draft
Graffioh wants to merge 2 commits into
Luce-Org:mainfrom
Graffioh:codex/unit-test-enforcement
Draft

Graffioh wants to merge 2 commits into
Luce-Org:mainfrom
Graffioh:codex/unit-test-enforcement

Conversation

@Graffioh

@Graffioh Graffioh commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Stacked on #709 — this contains only the coverage deltas; the reorganization lives in the refactor PR. Until #709 merges, this PR's diff includes the refactor. Enforcement-only view: Graffioh/lucebox-hub@codex/unit-test-refactor...codex/unit-test-enforcement

Adds model-free regression coverage and tightens existing assertions:

  • qwen3/test_query_capture.cpp: table-driven query_capture_slice clipping, disjoint/empty-range rejection, and an exhaustive chunked-capture reconstruction that asserts every query token is written exactly once.
  • qwen3/test_qwen3_buffer_plan.cpp / qwen3/test_score_range.cpp: replace single-suite TEST_CASEs with focused cases covering no-KV tail reuse, per-layer legacy scoring state, zero-layer plans, last-forwarded-layer scoring, and the empty-forwarded edge.
  • common/test_gguf_mmap.cpp: failed re-open clears the previous mapping, and move construction/assignment transfers ownership; re-open now verifies payload bytes of a differently-sized replacement.
  • common/test_restore_delta.cpp: negative prefix rejection and zero-prefix/empty-prompt handling.
  • server/test_http_sessions.cpp: every read accessor refreshes session LRU position, reads of missing sessions do not allocate, EMA asserts the literal expected value, and convergence/LRU tests assert exact bounds and turn counts.
  • server/test_http_request.cpp: regression for a repeated suffix in an earlier turn being mistaken for the current pflash query.
  • server/test_prefix_cache.cpp / server/test_text.cpp: hash_prefix avoids nullptr; disabled-reasoning asserts the parsed reasoning fields.
  • server/test_sse_emitter.cpp: restores the stderr content checks in the malformed-tool-call suppression test.

One production change pairs with the new regression test: find_pflash_query_window now bounds the suffix search to max_tokens before search_end, so a repeated phrase in an older turn cannot be mistaken for the current query. Header comment updated to match.

Test plan

  • Combined tree (refactor + enforcement) builds clean; test_server_unit discovers 556 cases; ctest passes 554/556 — the only 2 failures are upstream-verbatim MixedMmqPolicy cases segfaulting inside libamdhip64/ggml HIP kernels on this gfx1201+gfx1151 box (pre-existing environment issue).
  • The refactor-only base (refactor(test): organize component suites and replace simulated coverage #709) independently builds and passes 541/543 with the same 2 environment failures.

Generated with Devin

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 80 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/test/server/test_disk_prefix_cache.cpp">

<violation number="1" location="server/test/server/test_disk_prefix_cache.cpp:443">
P2: This test never exercises cold-prefix boundary selection. It builds the cache with a plain MockBackend, whose snapshot_ref() returns no context, so cold_prefix_boundary() always returns 0 at the `!layout_known_` guard; the assertion `result == 0` only checks that guard. The test name and header comment claim it finds the last boundary <= cold_max_tokens, but that logic is never reached. Use MockBackendWithLayout (already defined above) and call cache.learn_layout(0) before cold_prefix_boundary() so layout_known_ is set and the boundary-finding path is actually covered.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

std::vector<int> boundaries = {1000, 2000, 3000, 4000, 6000, 8000};
// Without layout_known_, returns 0.
int result = cache.cold_prefix_boundary(prompt, boundaries);
TEST_ASSERT(result == 0); // layout not known yet

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This test never exercises cold-prefix boundary selection. It builds the cache with a plain MockBackend, whose snapshot_ref() returns no context, so cold_prefix_boundary() always returns 0 at the !layout_known_ guard; the assertion result == 0 only checks that guard. The test name and header comment claim it finds the last boundary <= cold_max_tokens, but that logic is never reached. Use MockBackendWithLayout (already defined above) and call cache.learn_layout(0) before cold_prefix_boundary() so layout_known_ is set and the boundary-finding path is actually covered.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/test/server/test_disk_prefix_cache.cpp, line 443:

<comment>This test never exercises cold-prefix boundary selection. It builds the cache with a plain MockBackend, whose snapshot_ref() returns no context, so cold_prefix_boundary() always returns 0 at the `!layout_known_` guard; the assertion `result == 0` only checks that guard. The test name and header comment claim it finds the last boundary <= cold_max_tokens, but that logic is never reached. Use MockBackendWithLayout (already defined above) and call cache.learn_layout(0) before cold_prefix_boundary() so layout_known_ is set and the boundary-finding path is actually covered.</comment>

<file context>
@@ -0,0 +1,759 @@
+    std::vector<int> boundaries = {1000, 2000, 3000, 4000, 6000, 8000};
+    // Without layout_known_, returns 0.
+    int result = cache.cold_prefix_boundary(prompt, boundaries);
+    TEST_ASSERT(result == 0);  // layout not known yet
+
+    rm_rf(dir);
</file context>

@Graffioh
Graffioh marked this pull request as draft September 14, 2026 16:59
@Graffioh
Graffioh force-pushed the codex/unit-test-enforcement branch from 0f5c9f4 to 9545c87 Compare September 17, 2026 07:48
Split the 7k-line server/test/test_server_unit.cpp monolith into
component-oriented files under server/test/{common,deepseek4,draft,kernels,
kvflash,moe,placement,qwen3,qwen35,server} matching the production layout,
with shared fixtures/mocks under server/test/support/.

Removes low-value coverage (formula-copying, assignment-only, simulated
allocation, and suite-in-suite tests) and converts the feature-gate suite to
per-case TEST_CASEs. Relocates all remaining upstream coverage verbatim,
including the tests added on main since the original branch point.
…search

New model-free coverage: chunked query_capture_slice reconstruction,
buffer-plan/score-range tables, mmap failure/move semantics,
restore-prompt-delta boundaries, session LRU read-refresh and
no-allocation reads, and a pflash regression where a repeated suffix in
an earlier turn must not supply a query window.

Tightens existing assertions (exact EMA/LRU/convergence bounds, mmap
replace verifies payload bytes, hash_prefix avoids nullptr) and restores
stderr content checks in the malformed-tool-call suppression test.

Pairs with a small production fix: find_pflash_query_window now bounds
the suffix search to max_tokens before search_end so a repeated phrase
in an older turn cannot be mistaken for the current query.
@Graffioh
Graffioh force-pushed the codex/unit-test-enforcement branch from 9545c87 to caf1788 Compare September 19, 2026 08:24
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.

1 participant