From c8c2521fa26b02ab98d662abde60c54ac02bcf7b Mon Sep 17 00:00:00 2001 From: kami Date: Wed, 1 Jul 2026 03:08:24 +0400 Subject: [PATCH] fix(kernel,inference): reliable artifact emission for local models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes that together let the freestyle analyst/architect stages actually produce valid artifacts on a local model (verified end-to-end: analyst passed first try, architect produced a validated plan): 1. Tools-less final emission (SessionOrchestrator): producing the artifact while tools are in the request is unreliable — llama.cpp's Gemma template switches to a channel format and leaks <|channel> markers into the text, and models keep tool-calling until the round cap without ever emitting. After the tool loop, fire ONE tools-less inference to get clean JSON (also re-enables the JSON grammar, since grammar+tools is rejected). Gated on the artifact not already being produced via emit_artifact. 2. GBNF grammar handles arrays/objects (GbnfGrammarConverter): array-typed properties fell through to a 'string' rule, so the grammar forced a string where the schema demanded an array — an unwinnable grammar-vs-validator disagreement. Add generic value/object/array rules and map types correctly. Note: also needs schema 'items' on array props (runtime schemas updated; mirror to repo schemas/*.json). --- .../orchestration/ReplayOrchestrator.kt | 5 +- .../orchestration/SessionOrchestrator.kt | 85 ++++++++++++++----- .../llama/cpp/GbnfGrammarConverter.kt | 23 ++++- .../llama/cpp/GbnfGrammarConverterTest.kt | 23 ++++- 4 files changed, 110 insertions(+), 26 deletions(-) diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ReplayOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ReplayOrchestrator.kt index ec37f21d..477011d9 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ReplayOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ReplayOrchestrator.kt @@ -182,6 +182,7 @@ class ReplayOrchestrator( timeoutMs: Long, responseFormat: ResponseFormat, effectives: RunEffectives, + withTools: Boolean, ): InferenceResult = when (strategy) { is ReplayStrategy.SkipInference -> { // bypass router entirely — use recorded artifact @@ -210,7 +211,9 @@ class ReplayOrchestrator( }, ) } - else -> super.runInference(sessionId, stageId, contextPack, stageConfig, timeoutMs, responseFormat, effectives) + else -> super.runInference( + sessionId, stageId, contextPack, stageConfig, timeoutMs, responseFormat, effectives, withTools, + ) } override suspend fun mapValidationOutcome( diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt index 2099854b..6f4691d3 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt @@ -606,6 +606,40 @@ abstract class SessionOrchestrator( ) toolRounds++ } + // Final clean emission. Producing the artifact while tools are in the request is unreliable on + // some local chat templates (Gemma leaks `<|channel>` markers into the text; other models keep + // tool-calling until the round cap and never emit). For an LLM-emitted stage that hasn't already + // produced its artifact via the emit_artifact tool, fire ONE tools-less inference — verified to + // yield clean JSON deterministically. Skipped when the model already emitted clean final text. + if (inferenceResult is InferenceResult.Success && llmEmittedSlots.isNotEmpty() && llmArtifactOverride == null) { + val last = inferenceResult.response + val needsCleanEmission = last.finishReason is FinishReason.ToolCall || + last.text.isBlank() || last.text.contains("<|channel") + if (needsCleanEmission && !isCancelled(sessionId)) { + val nudge = "Stop calling tools. Output the required '${llmEmittedSlots.first().name.value}' " + + "artifact now as a single JSON object matching the schema — no tool calls, no commentary." + accumulatedEntries = accumulatedEntries + ContextEntry( + id = ContextEntryId(UUID.randomUUID().toString()), + layer = ContextLayer.L2, + sourceType = "toolResult", + sourceId = UUID.randomUUID().toString(), + content = nudge, + tokenEstimate = estimateTokens(nudge), + role = EntryRole.TOOL, + ) + currentContext = contextPackBuilder.build( + id = ContextPackId(UUID.randomUUID().toString()), + sessionId = sessionId, + stageId = stageId, + entries = accumulatedEntries, + budget = TokenBudget(limit = stageConfig.tokenBudget), + ) + inferenceResult = runInference( + sessionId, stageId, currentContext, stageConfig, config.stageTimeoutMs, + responseFormat, effectives, withTools = false, + ) + } + } return when (inferenceResult) { is InferenceResult.Cancelled -> StageExecutionResult.Failure("CANCELLED", retryable = false) is InferenceResult.Failed -> @@ -1853,12 +1887,17 @@ abstract class SessionOrchestrator( timeoutMs: Long, responseFormat: ResponseFormat = ResponseFormat.Text, effectives: RunEffectives = RunEffectives(toolRegistry, toolExecutor, workspacePolicy), + // When false, send NO tools. Some local chat templates (e.g. llama.cpp's Gemma) switch to a + // tool-calling/reasoning-channel format when tools are present and leak `<|channel>` markers + // into the artifact text; a tools-less call yields clean JSON deterministically (and lets the + // JSON grammar constraint re-apply, since grammar+tools is rejected by llama.cpp). + withTools: Boolean = true, ): InferenceResult { // A stage that grants tools needs a tool-calling model, so request ToolCalling on top of any // declared capabilities — the capability-aware strategy then ranks eligible providers by their // ToolCalling score and routes the stage to the best tool-caller. val requiredCapabilities = stageConfig.requiredCapabilities + - if (stageConfig.allowedTools.isNotEmpty()) setOf(ModelCapability.ToolCalling) else emptySet() + if (withTools && stageConfig.allowedTools.isNotEmpty()) setOf(ModelCapability.ToolCalling) else emptySet() val provider = inferenceRouter.route(stageId, requiredCapabilities, stageConfig.modelId) log.debug( "[Orchestrator] inference session={} stage={} provider={} timeoutMs={}", @@ -1872,28 +1911,32 @@ abstract class SessionOrchestrator( contextPack = contextPack, generationConfig = stageConfig.generationConfig, responseFormat = responseFormat, - tools = stageConfig.allowedTools - .mapNotNull { effectives.registry?.resolve(it) } - .filter { tool -> - // ponytail: filter write tools while read-before-write block is active; restored once a read completes - !isReadOnlyMode(sessionId) || ToolCapability.FILE_WRITE !in tool.requiredCapabilities - } - .map { tool -> - ToolDefinition( + tools = if (!withTools) { + emptyList() + } else { + stageConfig.allowedTools + .mapNotNull { effectives.registry?.resolve(it) } + .filter { tool -> + // ponytail: filter write tools while read-before-write block is active; restored once a read completes + !isReadOnlyMode(sessionId) || ToolCapability.FILE_WRITE !in tool.requiredCapabilities + } + .map { tool -> + ToolDefinition( + function = ToolFunction( + name = tool.name, + description = tool.description, + parameters = tool.parametersSchema, + ), + ) + } + ToolDefinition( function = ToolFunction( - name = tool.name, - description = tool.description, - parameters = tool.parametersSchema, + name = STAGE_COMPLETE_TOOL, + description = "Call this tool when the stage's goal is fully met and no further tool calls are needed. " + + "The orchestrator will proceed to the next stage.", + parameters = JsonObject(emptyMap()), ), - ) - } + ToolDefinition( - function = ToolFunction( - name = STAGE_COMPLETE_TOOL, - description = "Call this tool when the stage's goal is fully met and no further tool calls are needed. " + - "The orchestrator will proceed to the next stage.", - parameters = JsonObject(emptyMap()), - ), - ) + emitArtifactTool(stageConfig), + ) + emitArtifactTool(stageConfig) + }, ) val renderedMessages = PromptRenderer.render(contextPack) diff --git a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverter.kt b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverter.kt index ad0595f3..c0561981 100644 --- a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverter.kt +++ b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverter.kt @@ -19,7 +19,17 @@ internal object GbnfGrammarConverter { val escapeSeq = """"\\" (["\\/bfnrt] | $unicodeEscape)""" val stringRule = """string ::= "\"" ($charClass | $escapeSeq)* $closingQuote""" sb.appendLine(stringRule) - sb.appendLine("""number ::= "-"? [0-9]+""") + sb.appendLine("""number ::= "-"? [0-9]+ ("." [0-9]+)?""") + sb.appendLine("""boolean ::= "true" | "false"""") + sb.appendLine("""null ::= "null"""") + // Generic JSON value/object/array rules so non-scalar properties are constrained to the right + // JSON shape. Without these, an `array` property fell through to `string`, so the grammar forced + // the model to emit a string where the schema (and post-hoc validation) demand an array — an + // unwinnable disagreement. Element/field *types* are left to schema validation; the grammar only + // enforces the JSON shape (brackets/braces). + sb.appendLine("""value ::= string | number | boolean | null | object | array""") + sb.appendLine("""object ::= "{" ws (string ws ":" ws value (ws "," ws string ws ":" ws value)*)? ws "}"""") + sb.appendLine("""array ::= "[" ws (value (ws "," ws value)*)? ws "]"""") // Build members rule: // - Required keys are always present, joined by commas. @@ -58,7 +68,14 @@ internal object GbnfGrammarConverter { } private fun valueRuleFor(schema: JsonSchema, key: String): String { - val prop = schema.properties[key] ?: return "string" - return if (prop.type == "integer") "number" else "string" + val prop = schema.properties[key] ?: return "value" + return when (prop.type) { + "integer", "number" -> "number" + "boolean" -> "boolean" + "array" -> "array" + "object" -> "object" + "string" -> "string" + else -> "value" + } } } diff --git a/infrastructure/inference/llama_cpp/src/test/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverterTest.kt b/infrastructure/inference/llama_cpp/src/test/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverterTest.kt index 5327c7c8..c65a4401 100644 --- a/infrastructure/inference/llama_cpp/src/test/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverterTest.kt +++ b/infrastructure/inference/llama_cpp/src/test/kotlin/com/correx/infrastructure/inference/llama/cpp/GbnfGrammarConverterTest.kt @@ -40,10 +40,31 @@ class GbnfGrammarConverterTest { required = emptyList(), ) + @Test + fun `array property maps to the array rule, not string`() { + // Regression: an array-typed property used to fall through to `string`, so the grammar forced + // a string where the schema demands an array (analysis.requirements). It must map to `array`. + val schema = JsonSchema( + type = "object", + properties = mapOf( + "summary" to JsonSchemaProperty(type = "string"), + "requirements" to JsonSchemaProperty(type = "array", items = JsonSchemaProperty(type = "string")), + ), + required = listOf("summary", "requirements"), + ) + val grammar = GbnfGrammarConverter.convert(schema) + assertTrue(grammar.contains("array ::="), "grammar must declare an array rule") + val reqLine = grammar.lineSequence().first { it.contains("\\\"requirements\\\"") } + assertTrue(reqLine.trimEnd().endsWith("array"), "requirements must use the array rule, was: $reqLine") + } + @Test fun `string rule does not contain quoted character class`() { val grammar = GbnfGrammarConverter.convert(allRequiredSchema) - assertFalse(grammar.contains("\"["), "string rule must not contain quoted character class (\"[)") + // Guard the original bug: the `string` rule's char class must stay unquoted. The `array` rule + // legitimately contains a quoted "[" literal, so scope the assertion to the string rule line. + val stringRuleLine = grammar.lineSequence().first { it.trimStart().startsWith("string ::=") } + assertFalse(stringRuleLine.contains("\"["), "string rule must not contain quoted character class (\"[)") } @Test