Skip to content

Sweep 1565 high-risk: extraction marker read failures and .tmp cleanup, Firefunction v2 call id, DeepSeek R1 buildGrammar pin, x86_64 iOS simulator label - #1584

Merged
leehack merged 1 commit into
mainfrom
chore/1565-high-risk-sweep
Oct 29, 2026
Merged

leehack merged 1 commit into
mainfrom
chore/1565-high-risk-sweep

Conversation

@leehack

@leehack leehack commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

Clears the open high-risk items from the 0.17.0 follow-ups tracker #1565:

  • Extraction marker (from the #1575 audit): a test now pins that an archive or extraction the hook cannot read makes the marker check throw FileSystemException. It no longer matches, which kills W1 (the catch widened to the whole check). When a marker write or rename fails, the .tmp is now deleted best-effort before the error is rethrown.
  • Firefunction v2 call id (from the #1578 lane): the required-tool grammar no longer requires an unbounded free-text id string. This mirrors the LFM2 fix in Match llama.cpp tool-call grammars for DeepSeek R1, LFM2 and Functionary v3.2 #1578 (b0c8e14b4). Upstream leaves id undeclared in both places: the legacy common_chat_params_init_firefunction_v2 (566059a26^) lists id under required but not under properties, and at 620d2225c the autoparser builds the grammar from the template, which renders calls as {"name": ..., "arguments": ...}. The parser still reads an id if the model writes one.
  • DeepseekR1Handler.buildGrammar() (mutant M7 from the Match llama.cpp tool-call grammars for DeepSeek R1, LFM2 and Functionary v3.2 #1578 audit): the default is kept (parallel true) and now pinned by a test. The method is public through the exported ChatTemplateEngine.handlerFor(...). The other context-free buildGrammar overrides (Firefunction v2, Functionary v3.1/v3.2, EXAONE MoE) also return the parallel-call grammar, so tightening only DeepSeek R1 would silently change a public method's output and make it inconsistent with its siblings. Nothing in production calls it: render passes the request's parallel flag.
  • iOS simulator labels (from the #1580 audit): the image runtime now labels the x86_64 iOS simulator ios-x64-sim, the hook's stable_diffusion bundle key (stableDiffusionBundleForNativeBundle), instead of ios-x64. New cases cover the default wiring (Abi.iosArm64, no isIosSimulator, non-simulator host gives ios-arm64) and Abi.iosX64 with isIosSimulator: () => true. A cross-check asserts that each iOS label equals the hook's bundle key. These kill M5, M2 and M6.

Production-readiness scope

Completeness checklist

  • Declared scope is fully implemented, or explicitly reduced above.
  • Unsupported platform/option combinations fail loudly with actionable diagnostics or are clearly documented as unavailable.
  • Public API docs, README/website docs, examples, support matrices, and changelog entries are updated where relevant. Both changelogs are updated for items 2 and 4. There is no doc, support-matrix or skill text for these labels or the Firefunction grammar.
  • Regression coverage covers the original issue plus key negative/version-skew paths where relevant.
  • Public capability changes update the validation coverage registry with a pack or a reasoned, issue-linked exemption; inventory checks pass (or N/A with scope). N/A: no capability changes.
  • Security/privacy review completed: no secrets, bearer tokens, signed URLs, or raw secret-bearing paths leak through logs, cache keys, metadata, errors, or snapshots.
  • Follow-up work that is useful but not required for this PR is tracked in GitHub Issues, or explicitly marked "None" above.

PR type guidance

Test Plan

  • dart run tool/prepare_workspace.dart (clean tree after)
  • dart format --output=none --set-exit-if-changed .
  • dart analyze
  • dart run tool/testing/check_platform_boundaries.dart
  • dart test -p vm -j 1 --exclude-tags local-only: +9814 ~8, all passed (with REQUIRE_LLAMA_CPP_TEMPLATES=1, isolated TMPDIR)
  • dart test -p chrome --exclude-tags local-only: +4117 ~1, all passed
  • Other targeted/local-only validation:
    • tool/testing/run_template_parity_suites.sh with test-gbnf-validator built at llama.cpp 620d2225c (the shipped runtime): detection +74, diagnostics +93, unit template +1624, upstream parser +10, compiled grammar +61
    • --run-skipped specialized_tool_grammar_validation_e2e_test.dart + required_tool_emissions_test.dart: +67
    • test/unit/hook, test/unit/backends/stable_diffusion, test/unit/core/template, test/integration/core/template: +2616
    • test/unit/tooling: +1240 ~5

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
structured-output-adversarial Firefunction v2 compiled grammar acceptance and rejection, DeepSeek R1 buildGrammar macOS arm64, test-gbnf-validator @ 620d2225c PASS Firefunction v2 weights unavailable (no cached model). Uses the upstream emission functools[{"name": ..., "arguments": ...}] from llama.cpp tests/test-chat.cpp, which matches what the vendored template renders, and the durable fixture test/fixtures/llama_cpp_templates/fireworks-ai-llama-3-firefunction-v2.jinja. Base grammar rejects the upstream form and accepts "id"; head accepts the upstream form and rejects "id" (llamadart-evidence/sweep-1565-hr2/validator-probes.txt)
template-parity vendored llama.cpp detection, render, parse and compiled grammar macOS arm64 PASS run_template_parity_suites.sh as above
high-risk-exact-head-independent-qa independent blocking review of exact head fresh auditor session, exact head 6b6711f PASS CLEAR: CLEAR independent audit at 6b6711f; evidence /opt/UnitySrc/personal/llama/llamadart-evidence/pr-1584/evidence-1584-6b6711f.json
ios-simulator-smoke iOS x86_64 simulator label N/A N/A The change is message text only (unavailableReason). It is covered by the injected-ABI unit tests and by the cross-check against stableDiffusionBundleForNativeBundle. No bundle or staging change

Mutants, each applied alone and run against its suites (llamadart-evidence/sweep-1565-hr2/mut_*.log):

Mutant Result
W1 marker catch widened to the whole function (returns true) killed
W1b same, returning false killed
T1 leftover .tmp not deleted killed
T2 write/rename failure swallowed killed
F1 Firefunction idKey: 'id' restored (unit + e2e; e2e alone also kills it) killed
M7 DeepseekR1Handler.buildGrammar parallel false killed
M5 default isIosSimulator always true killed
M2 arm64 -sim label applied to Abi.iosX64 killed
M6 Abi.iosX64 labelled by ABI (ios-x64, the pre-fix behaviour) killed
M8 arm64 simulator check dropped killed

Review Notes

  • Independent review status: CLEAR at exact head 6b6711f by a fresh auditor session (0 P1, 0 P2).
  • CI status / head SHA: 6b6711f, CI pending

High-risk regression review

  • Classification: high-risk (surfaces: artifactConsumer, backendRuntime, structuredOutput)
  • Implementation task: sweep lane for 0.17.0 small follow-ups tracker #1565 (this PR's author)
  • Independent blocking QA task: fresh auditor session, CLEAR, 0 known PR-caused P1, 0 unresolved threads
  • Exact head / current base: 6b6711f / 61bebeb (merge-tree clean)
  • Production-branch deletion, bypass, or miswire proof: mutant table above. Each production change is killed when reverted or miswired.
  • Affected-family real-model/artifact evidence: none. Firefunction v2 weights are unavailable.
  • Explicit unavailable-family or other N/A evidence: Firefunction v2 is unavailable. Evidence is primary upstream emissions (llama.cpp tests/test-chat.cpp at 620d2225c), the legacy upstream grammar source (566059a26^:common/chat.cpp) and the vendored template fixture, compiled with test-gbnf-validator at 620d2225c.
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

…p, Firefunction v2 call id, DeepSeek R1 buildGrammar pin, x86_64 iOS simulator label

- An archive or extraction the hook cannot read fails the marker check
  instead of matching (kills W1), and a marker write or rename that fails
  deletes its .tmp.
- Firefunction v2's tool-call grammar no longer requires a free-text `id`;
  llama.cpp's grammar declares only `name` and `arguments`.
- Pin that DeepseekR1Handler.buildGrammar() returns the parallel-call
  grammar, like the other context-free handler grammars (kills M7).
- Image runtime reasons label the x86_64 iOS simulator `ios-x64-sim`, the
  hook's stable_diffusion bundle key, and default-wiring and iosX64 cases
  kill M5, M2 and M6.

Refs #1565
@leehack
leehack marked this pull request as ready for review October 29, 2026 05:25
@leehack
leehack merged commit 44de155 into main Oct 29, 2026
40 checks passed
@leehack
leehack deleted the chore/1565-high-risk-sweep branch October 29, 2026 05:25
@github-actions

Copy link
Copy Markdown
Contributor

Chat app preview removed for leehack/llamadart-chat-pr-1584.

leehack added a commit that referenced this pull request Oct 30, 2026
….5 structured parse (#1605)

Marker: a failed write removes its .tmp and rethrows the original error, even
when the cleanup itself fails (#1584 audit A3, A5, A8). The not-published list
names the hook's stable_diffusion bundles (A9). Genkit: Qwen3.5 with thinking
off keeps a thought in the content, as the migration guide says.
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