From b83831df73133422f8146fd0263d9eeaa3d36919 Mon Sep 17 00:00:00 2001 From: Andrey Mikhaylov Date: Wed, 26 Aug 2026 11:58:23 +0100 Subject: [PATCH 1/3] Speed up prefill on pre-Apple10 Macs --- .../Infrastructure/Metal/MetalContext.swift | 62 ++++++++- .../Kernels/Attention/PrefillAttention.swift | 30 +++- .../Kernels/Vision/VisionResize.swift | 2 +- .../Runtime/Generation/RawCompletion.swift | 2 +- .../Runtime/Inference/RealForwardRunner.swift | 18 ++- .../Runtime/Vision/VisionRuntime.swift | 2 +- Sources/TurboFieldfareCLI/Run.swift | 4 +- ...nteractivityWatchdogEnvironmentTests.swift | 42 ++++++ .../MetalContextDeviceCreationTests.swift | 40 ++++++ .../Attention/PrefillAttentionTests.swift | 39 +++++- .../CommandBufferCompletionTests.swift | 128 ++++++++++++++++-- docs/RUNTIME_CONTROLS.md | 15 ++ 12 files changed, 350 insertions(+), 34 deletions(-) create mode 100644 Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift create mode 100644 Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift diff --git a/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift b/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift index 1eea8155..7b5fa670 100644 --- a/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift +++ b/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift @@ -7,6 +7,7 @@ enum MetalError: Error, CustomStringConvertible { case missingShaderResource(String) case missingFunction(String) case libraryCompileFailed(String) + case commandBufferFailed(String) public var description: String { switch self { @@ -15,14 +16,54 @@ enum MetalError: Error, CustomStringConvertible { case .missingShaderResource(let n): return "Shader resource missing: \(n)" case .missingFunction(let n): return "Metal function missing in library: \(n)" case .libraryCompileFailed(let s):return "Metal library compile failed: \(s)" + case .commandBufferFailed(let s): return "Metal command buffer failed: \(s)" } } } -func checkCommandBufferError(_ error: (any Error)?) throws { +func metalCommandBufferStatusName(_ status: MTLCommandBufferStatus) -> String { + switch status { + case .notEnqueued: return "notEnqueued" + case .enqueued: return "enqueued" + case .committed: return "committed" + case .scheduled: return "scheduled" + case .completed: return "completed" + case .error: return "error" + @unknown default: return "unknown(\(status.rawValue))" + } +} + +/// Builds the diagnostic for a command buffer that did not complete, or nil +/// when it did. Status is checked separately because a failed buffer is not +/// guaranteed to carry an error object. +func metalCommandBufferFailureDetail(label: String?, + status: MTLCommandBufferStatus, + error: (any Error)?) -> String? { + if status == .completed && error == nil { return nil } + + var parts = ["label=\(label.map { $0.isEmpty ? "" : $0 } ?? "")"] + parts.append("status=\(metalCommandBufferStatusName(status))") if let error { - throw error + let nsError = error as NSError + parts.append("domain=\(nsError.domain)") + parts.append("code=\(nsError.code)") + parts.append("description=\(nsError.localizedDescription)") + if !nsError.userInfo.isEmpty { + parts.append("userInfoKeys=\(nsError.userInfo.keys.sorted().joined(separator: ","))") + } + } else { + parts.append("error=") + } + return parts.joined(separator: " ") +} + +func checkCommandBufferError(_ commandBuffer: MTLCommandBuffer) throws { + guard let detail = metalCommandBufferFailureDetail(label: commandBuffer.label, + status: commandBuffer.status, + error: commandBuffer.error) else { + return } + throw MetalError.commandBufferFailed(detail) } public struct MetalFunctionConstant: Hashable, Sendable { @@ -62,8 +103,23 @@ public final class MetalContext: @unchecked Sendable { private var pipelineCache: [PipelineCacheKey: MTLComputePipelineState] = [:] private let pipelineCacheLock = NSLock() + private static func relaxInteractivityWatchdog() { + // The AGX driver reads this once at first device creation. Long prefill + // dispatches can otherwise be killed as compositor-impacting on macOS + // 26. Overwrite 0 preserves an operator's explicit stock-behaviour + // override. This relaxes the deadline; it does not guarantee survival. + setenv("AGX_RELAX_CDM_CTXSTORE_TIMEOUT", "1", 0) + } + + /// Routes every production device creation through the watchdog mitigation + /// before the AGX driver's process-wide one-time environment read. + public static func makeSystemDefaultDevice() -> MTLDevice? { + relaxInteractivityWatchdog() + return MTLCreateSystemDefaultDevice() + } + public init() throws { - guard let dev = MTLCreateSystemDefaultDevice() else { throw MetalError.noDevice } + guard let dev = Self.makeSystemDefaultDevice() else { throw MetalError.noDevice } guard let q = dev.makeCommandQueue() else { throw MetalError.noQueue } self.device = dev self.queue = q diff --git a/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift b/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift index 0f0f4d7d..843ac6af 100644 --- a/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift +++ b/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift @@ -57,13 +57,33 @@ final class PrefillAttention { private let psoParamsSmoke: MTLComputePipelineState private let psoFullTensorOps2DValidityV2: MTLComputePipelineState? - init(context: MetalContext) throws { + var tensorOps2DValidityV2Available: Bool { + psoFullTensorOps2DValidityV2 != nil + } + + convenience init(context: MetalContext) throws { + try self.init(context: context, simulatingMissingTensorOps: false) + } + + /// Tests can force the production fallback even on a host where the MSL 4 + /// TensorOps pipeline builds. + init(context: MetalContext, simulatingMissingTensorOps: Bool) throws { self.context = context self.psoCausalTiled = try context.pipeline("attention_prefill_causal_tiled") self.psoParamsSmoke = try context.pipeline("prefill_attention_params_smoke") - self.psoFullTensorOps2DValidityV2 = context.device.supportsFamily(.apple10) - ? try? context.pipeline("attention_prefill_full_tensorops_2d_validity_v2") - : nil + if simulatingMissingTensorOps { + self.psoFullTensorOps2DValidityV2 = nil + } else { + do { + self.psoFullTensorOps2DValidityV2 = try context.pipeline( + "attention_prefill_full_tensorops_2d_validity_v2") + } catch { + self.psoFullTensorOps2DValidityV2 = nil + FileHandle.standardError.write(Data( + ("PrefillAttention: TensorOps 2D pipeline unavailable; " + + "using causal-tiled fallback: \(error)\n").utf8)) + } + } } func encodeCausal(commandBuffer: MTLCommandBuffer, @@ -103,7 +123,7 @@ final class PrefillAttention { pipeline = tensorOpsPipeline } else if tensorOpsShape && path == .fullTensorOps2DValidityV2 { preconditionFailure( - "TensorOps 2D prefill attention requires Apple10 MPP tensor support") + "TensorOps 2D prefill attention requires MSL 4 TensorOps pipeline support") } else { // Explicit mode also falls back for incompatible shapes. Benchmark // fixtures must use 512/16/2 to prove that TensorOps ran. diff --git a/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift b/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift index 8601a390..0f4c2c61 100644 --- a/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift +++ b/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift @@ -171,7 +171,7 @@ public final class VisionResize { commandBuffer.commit() commandBuffer.waitUntilCompleted() - try checkCommandBufferError(commandBuffer.error) + try checkCommandBufferError(commandBuffer) let finished = sourceHeight == destinationHeight ? intermediate : output let rowBytes = sourceHeight == destinationHeight diff --git a/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift b/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift index af4923f1..11428c44 100644 --- a/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift +++ b/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift @@ -307,6 +307,6 @@ private func sampleOnce(scratch: RawCompletionScratch, context: MetalContext, history: history, config: config, position: position, outToken: scratch.outToken) cb.commit(); cb.waitUntilCompleted() - try checkCommandBufferError(cb.error) + try checkCommandBufferError(cb) return Int32(bitPattern: scratch.outToken.contents().load(as: UInt32.self)) } diff --git a/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift b/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift index d0b12160..a5b13201 100644 --- a/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift +++ b/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift @@ -861,6 +861,7 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun guard var cb = ctx.queue.makeCommandBuffer() else { throw ModelError.residentBufferWrapFailed } + cb.label = "prefill start=\(startPosition) count=\(t) phase=embed" if let embeddingOverride { // The override spans the whole chunk — the planner emits image spans // as standalone chunks — so the INT4 gather it would cover is @@ -1101,6 +1102,8 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun guard let sharedCB = ctx.queue.makeCommandBuffer() else { throw ModelError.residentBufferWrapFailed } + sharedCB.label = + "prefill start=\(startPosition) count=\(t) layer=\(L) phase=shared_expert" let sharedProj = sharedExpertProjections[L] try prefillSharedExpert.encodeBlock(commandBuffer: sharedCB, x: scratch.denseX, @@ -1241,6 +1244,8 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun guard let tileCB = ctx.queue.makeCommandBuffer() else { throw ModelError.residentBufferWrapFailed } + tileCB.label = + "prefill start=\(startPosition) count=\(t) layer=\(L) phase=routed_tile" _ = prefillGroupedMoE.encodeStreamedBatched( commandBuffer: tileCB, hidden: scratch.routedX, @@ -1300,6 +1305,8 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun guard let nextCB = ctx.queue.makeCommandBuffer() else { throw ModelError.residentBufferWrapFailed } + nextCB.label = + "prefill start=\(startPosition) count=\(t) layer=\(L + 1) phase=qkv_attention" cb = nextCB } continue @@ -1311,6 +1318,7 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun guard let finalCB = ctx.queue.makeCommandBuffer() else { throw ModelError.residentBufferWrapFailed } + finalCB.label = "prefill start=\(startPosition) count=\(t) phase=final_head" if outputMode == .greedyIfAvailable, useFusedGreedyHead { fusionHead.encodeGreedyDecode( commandBuffer: finalCB, @@ -1395,12 +1403,12 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun waitUntilCompleted(pending.cb) } if let sharedCB = pending.sharedCB { - try checkCommandBufferError(sharedCB.error) + try checkCommandBufferError(sharedCB) } if let phase1HitCB = pending.phase1HitCB { - try checkCommandBufferError(phase1HitCB.error) + try checkCommandBufferError(phase1HitCB) } - try checkCommandBufferError(pending.cb.error) + try checkCommandBufferError(pending.cb) totalCb2Nanos &+= pending.encodeAndCommitNanos } @@ -1595,7 +1603,7 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun try finishPendingRoutedCommand(pending, waitIfNeeded: false) pendingRoutedCommand = nil } - try checkCommandBufferError(cb.error) + try checkCommandBufferError(cb) totalCb1Nanos &+= clock_gettime_nsec_np(CLOCK_UPTIME_RAW) - tCb1Start - waitNanos // CPU readback to fetch routed-expert blobs from disk. @@ -1873,7 +1881,7 @@ public final class RealForwardRunner: ChunkedPrefillRunner, MultimodalPrefillRun private nonisolated func waitForCompletion(_ cb: MTLCommandBuffer) throws { waitUntilCompleted(cb) - try checkCommandBufferError(cb.error) + try checkCommandBufferError(cb) } private nonisolated func waitUntilCompleted(_ cb: MTLCommandBuffer) { diff --git a/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift b/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift index f286fc4c..4cd9d932 100644 --- a/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift +++ b/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift @@ -244,7 +244,7 @@ public final class VisionRuntime { } public static var isSupportedOnDefaultDevice: Bool { - guard let device = MTLCreateSystemDefaultDevice() else { return false } + guard let device = MetalContext.makeSystemDefaultDevice() else { return false } return isSupported(on: device) } diff --git a/Sources/TurboFieldfareCLI/Run.swift b/Sources/TurboFieldfareCLI/Run.swift index 7b30406a..4995d257 100644 --- a/Sources/TurboFieldfareCLI/Run.swift +++ b/Sources/TurboFieldfareCLI/Run.swift @@ -55,7 +55,7 @@ public func run(args: Args, let input = try parseInput(args: args) var selectedDevice: MTLDevice? if input.hasImages { - guard let device = MTLCreateSystemDefaultDevice() else { + guard let device = MetalContext.makeSystemDefaultDevice() else { return errored(stderr, "no Metal device", 1) } try VisionRuntime.requireSupportedDevice(device) @@ -103,7 +103,7 @@ public func run(args: Args, // Hoisted above the `auto` estimate, which needs a device to read image // geometry. Planning never touches the GPU, but building the plan does // need the device the run will use. - guard let device = selectedDevice ?? MTLCreateSystemDefaultDevice() else { + guard let device = selectedDevice ?? MetalContext.makeSystemDefaultDevice() else { return errored(stderr, "no Metal device", 1) } diff --git a/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift b/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift new file mode 100644 index 00000000..02978de2 --- /dev/null +++ b/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift @@ -0,0 +1,42 @@ +import Darwin +import Testing + +@testable import TurboFieldfare + +@Suite(.serialized) struct InteractivityWatchdogEnvironmentTests { + private static let key = "AGX_RELAX_CDM_CTXSTORE_TIMEOUT" + + private static func read() -> String? { + guard let raw = getenv(key) else { return nil } + return String(cString: raw) + } + + private static func withCleanEnvironment(_ body: () throws -> Void) rethrows { + let saved = read() + unsetenv(key) + defer { + if let saved { + setenv(key, saved, 1) + } else { + unsetenv(key) + } + } + try body() + } + + @Test func constructingAMetalContextSetsTheWatchdogRelaxation() throws { + try Self.withCleanEnvironment { + #expect(Self.read() == nil) + _ = try MetalContext() + #expect(Self.read() == "1") + } + } + + @Test func anExplicitOperatorOverrideIsNotOverwritten() throws { + try Self.withCleanEnvironment { + setenv(Self.key, "0", 1) + _ = try MetalContext() + #expect(Self.read() == "0") + } + } +} diff --git a/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift b/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift new file mode 100644 index 00000000..8da8415d --- /dev/null +++ b/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift @@ -0,0 +1,40 @@ +import Foundation +import Testing + +@Suite struct MetalContextDeviceCreationTests { + private static var sourcesDirectory: URL { + URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("Sources") + } + + @Test func onlyMetalContextCreatesTheSystemDevice() throws { + let sources = Self.sourcesDirectory + try #require(FileManager.default.fileExists(atPath: sources.path), + "cannot locate Sources/ from \(#filePath)") + + var offenders: [String] = [] + let walker = FileManager.default.enumerator( + at: sources, + includingPropertiesForKeys: nil) + + while let url = walker?.nextObject() as? URL { + guard url.pathExtension == "swift", + url.lastPathComponent != "MetalContext.swift", + let text = try? String(contentsOf: url, encoding: .utf8), + text.contains("MTLCreateSystemDefaultDevice(") else { continue } + + for (index, line) in text.split(separator: "\n", omittingEmptySubsequences: false) + .enumerated() where line.contains("MTLCreateSystemDefaultDevice(") { + offenders.append("\(url.lastPathComponent):\(index + 1)") + } + } + + #expect(offenders.isEmpty, + "Production device creation bypasses MetalContext: \(offenders.joined(separator: ", "))") + } +} diff --git a/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift b/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift index 6e4ed304..79ae1325 100644 --- a/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift +++ b/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift @@ -140,9 +140,10 @@ import TurboFieldfareValidationSupport ]) func tensorOps2DFullAttentionMatchesReferenceAtTileBoundaries(_ visibleKeys: Int) throws { let context = try MetalContext() - // Hosted CI has no Apple10 GPU, so it returns without dispatching this - // kernel. Run this suite on Apple10 before changing the TensorOps path. - guard context.device.supportsFamily(.apple10) else { return } + let attention = try PrefillAttention(context: context) + // GPU family is not the capability test: this pipeline compiles and + // dispatches on the project's Apple8 M2 minimum. + guard attention.tensorOps2DValidityV2Available else { return } let fixture = Self.makeFixture(start: visibleKeys - 1, chunk: 1, window: 0, @@ -186,12 +187,35 @@ import TurboFieldfareValidationSupport "preferred TensorOps maxAbs=\(maxAbs) rel=\(rel)") #expect(rel <= 2e-2, "preferred TensorOps rel=\(rel) maxAbs=\(maxAbs)") - if !context.device.supportsFamily(.apple10) { + let attention = try PrefillAttention(context: context) + if attention.tensorOps2DValidityV2Available { + let explicitTensorOps = try Self.runKernel( + fixture, + path: .fullTensorOps2DValidityV2) + #expect(preferred == explicitTensorOps) + } else { let baseline = try Self.runKernel(fixture, path: .causalTiled) #expect(preferred == baseline) } } + @Test func preferredPathUsesTiledWhenTensorOpsIsUnavailable() throws { + let fixture = Self.makeFixture(start: 128, + chunk: 8, + window: 0, + seed: 0xA873, + headDim: 512, + qHeads: 16, + kvHeads: 2) + let preferred = try Self.runKernel( + fixture, + path: .fullTensorOps2DPreferred, + simulatingMissingTensorOps: true) + let tiled = try Self.runKernel(fixture, path: .causalTiled) + + #expect(preferred == tiled) + } + private static func makeFixture(start: Int, chunk: Int, window: Int, @@ -315,10 +339,12 @@ import TurboFieldfareValidationSupport private static func runKernel( _ fixture: Fixture, kvRingCapacity: UInt32 = 0, - path: RuntimePrefillAttentionPath = .causalTiled + path: RuntimePrefillAttentionPath = .causalTiled, + simulatingMissingTensorOps: Bool = false ) throws -> [Float] { let ctx = try MetalContext() - let prefill = try PrefillAttention(context: ctx) + let prefill = try PrefillAttention( + context: ctx, simulatingMissingTensorOps: simulatingMissingTensorOps) let qPrefix = 17 let kPrefix = 19 let vPrefix = 23 @@ -367,6 +393,7 @@ import TurboFieldfareValidationSupport path: path) cb.commit() cb.waitUntilCompleted() + try checkCommandBufferError(cb) let out = Fp16Buffer.read(outBuf, count: outCount) var compact = [Float](repeating: 0, count: fixture.chunk * fixture.qHeads * fixture.headDim) diff --git a/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift b/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift index d010592c..667bf338 100644 --- a/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift +++ b/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift @@ -1,22 +1,130 @@ +import Foundation +import Metal import Testing @testable import TurboFieldfare @Suite struct CommandBufferCompletionTests { - private enum SyntheticCommandBufferError: Error, Equatable { - case failed + private static func nsError(domain: String = "MTLCommandBufferErrorDomain", + code: Int = 1, + description: String, + userInfo extra: [String: Any] = [:]) -> NSError { + var info: [String: Any] = [NSLocalizedDescriptionKey: description] + info.merge(extra) { current, _ in current } + return NSError(domain: domain, code: code, userInfo: info) } - @Test func commandBufferErrorsArePropagated() throws { - try checkCommandBufferError(nil) + @Test func completedBufferWithNoErrorIsNotAFailure() { + #expect(metalCommandBufferFailureDetail(label: "prefill layer=3", + status: .completed, + error: nil) == nil) + } + + @Test func errorStatusWithNoErrorObjectIsStillAFailure() throws { + let detail = try #require( + metalCommandBufferFailureDetail(label: "prefill layer=3", + status: .error, + error: nil)) + #expect(detail.contains("status=error")) + #expect(detail.contains("label=prefill layer=3")) + #expect(detail.contains("error=")) + } + + @Test(arguments: [ + MTLCommandBufferStatus.notEnqueued, + .enqueued, + .committed, + .scheduled, + ]) + func nonCompletedStatusIsAFailure(_ status: MTLCommandBufferStatus) throws { + let detail = try #require( + metalCommandBufferFailureDetail(label: nil, status: status, error: nil)) + #expect(detail.contains("status=\(metalCommandBufferStatusName(status))")) + } + + @Test func interactivityKillDetailPreservesTheIOGPUToken() throws { + let description = + "Impacting Interactivity " + + "(0000000e:kIOGPUCommandBufferCallbackErrorImpactingInteractivity)" + let detail = try #require( + metalCommandBufferFailureDetail( + label: "prefill start=8064 count=128 layer=12 phase=qkv_attention", + status: .error, + error: Self.nsError(description: description))) + + #expect(detail.contains("kIOGPUCommandBufferCallbackErrorImpactingInteractivity")) + #expect(detail.contains("0000000e")) + #expect(detail.contains("domain=MTLCommandBufferErrorDomain")) + #expect(detail.contains("code=1")) + #expect(detail.contains("layer=12")) + } + + @Test func missingLabelIsReportedRatherThanOmitted() throws { + let detail = try #require( + metalCommandBufferFailureDetail(label: nil, + status: .error, + error: Self.nsError(description: "boom"))) + #expect(detail.contains("label=")) + } + + @Test func emptyLabelIsDistinguishedFromAbsentLabel() throws { + let detail = try #require( + metalCommandBufferFailureDetail(label: "", + status: .error, + error: Self.nsError(description: "boom"))) + #expect(detail.contains("label=")) + } + + @Test func userInfoKeysAreNamedButValuesAreNotLeaked() throws { + let detail = try #require( + metalCommandBufferFailureDetail( + label: "x", + status: .error, + error: Self.nsError(description: "boom", + userInfo: ["TFSecretPayload": "do-not-leak-me"]))) + + #expect(detail.contains("userInfoKeys=")) + #expect(detail.contains("TFSecretPayload")) + #expect(!detail.contains("do-not-leak-me")) + } + + @Test func completedStatusCarryingAnErrorIsAFailure() throws { + let detail = try #require( + metalCommandBufferFailureDetail(label: "x", + status: .completed, + error: Self.nsError(description: "boom"))) + #expect(detail.contains("status=completed")) + #expect(detail.contains("description=boom")) + } + + @Test func statusNamesCoverEveryCase() { + #expect(metalCommandBufferStatusName(.notEnqueued) == "notEnqueued") + #expect(metalCommandBufferStatusName(.enqueued) == "enqueued") + #expect(metalCommandBufferStatusName(.committed) == "committed") + #expect(metalCommandBufferStatusName(.scheduled) == "scheduled") + #expect(metalCommandBufferStatusName(.completed) == "completed") + #expect(metalCommandBufferStatusName(.error) == "error") + } + + @Test func realCompletedCommandBufferDoesNotThrow() throws { + let context = try MetalContext() + let commandBuffer = try #require(context.queue.makeCommandBuffer()) + commandBuffer.label = "test empty" + commandBuffer.commit() + commandBuffer.waitUntilCompleted() + try checkCommandBufferError(commandBuffer) + } + + @Test func realNonCompletedCommandBufferThrows() throws { + let context = try MetalContext() + let commandBuffer = try #require(context.queue.makeCommandBuffer()) + commandBuffer.label = "test uncommitted" do { - try checkCommandBufferError(SyntheticCommandBufferError.failed) - Issue.record("expected command-buffer error") - } catch let error as SyntheticCommandBufferError { - #expect(error == .failed) - } catch { - Issue.record("unexpected error: \(error)") + try checkCommandBufferError(commandBuffer) + Issue.record("expected a failure for a buffer that never completed") + } catch let error as MetalError { + #expect("\(error)".contains("test uncommitted")) } } } diff --git a/docs/RUNTIME_CONTROLS.md b/docs/RUNTIME_CONTROLS.md index 3e45441f..525402ea 100644 --- a/docs/RUNTIME_CONTROLS.md +++ b/docs/RUNTIME_CONTROLS.md @@ -45,6 +45,21 @@ each request and do not require a reload. Each CLI invocation loads a new model process, so its selected runtime settings apply immediately. The server fixes its runtime settings at startup, so changing one means restarting the process. +### macOS interactivity mitigation + +Before the first Metal device is created, TurboFieldfare defaults +`AGX_RELAX_CDM_CTXSTORE_TIMEOUT` to `1`. This relaxes an AGX context-store +deadline that can terminate a long prefill dispatch as +`kIOGPUCommandBufferCallbackErrorImpactingInteractivity` on macOS 26. It is a +mitigation, not a guarantee: failures have also been reported with the setting +enabled. + +Export `AGX_RELAX_CDM_CTXSTORE_TIMEOUT=0` before launching TurboFieldfare to +restore stock driver behavior. An explicit environment value is never +overwritten. If a command buffer still fails, the error includes its prefill +phase label, Metal status, domain, code, and IOGPU diagnostic token without +including prompt or generated content. + ## Image controls Image input needs the companion pack installed beside the text model. Without From 04b36ce4c6f6fe87b29b26a58fe246d83bdd6681 Mon Sep 17 00:00:00 2001 From: Andrey Mikhaylov Date: Wed, 26 Aug 2026 13:43:39 +0100 Subject: [PATCH 2/3] Apply the prefill diagnostics review fixes Brings this branch up to the reviewed state of the private tree. Four findings, all of which this branch previously carried unfixed. Reject a clipping window on the TensorOps path, not only on the fallback selection. The full-attention kernels start their key loop at zero and ignore `slidingWindow`, so a `.full`-labelled request carrying a window that actually clips produced wrong output. Production passes `slidingWindow == kvValidCount` for full layers and never clips, so selection there is unchanged; the guard now also requires `layerKind == .full`, which this tree was missing. Restrict the AGX interactivity mitigation to macOS at compile time. The `AGX_RELAX_CDM_CTXSTORE_TIMEOUT` variable is read by the AGX userspace driver and does not exist elsewhere. The operator override is unchanged: the overwrite argument stays 0, so exporting the variable as 0 still restores stock behaviour. Make the system-device source audit fail closed. It swallowed enumerator and file-read errors with `try?` and exempted by bare filename, so it could report green while auditing nothing. It now requires the enumerator, records walk and read failures, asserts them empty, and exempts by exact relative path. Route all three VisionRuntime command-buffer waits through the status-aware diagnostic helper. They stringified `buffer.error`, so a buffer that failed with `status == .error` and no error object was consumed as success. The fourth wait is inside a `defer` on the throw path and stays deliberately unchecked, which is now stated at the site. Each fix lands with a regression: a clipping-window fixture at the production 512/16/2 shape that reaches the real TensorOps pipeline, a compile-selected non-macOS negative test, positive assertions on the audit's failure arrays, and a call-site check on the VisionRuntime waits. --- .../Infrastructure/Metal/MetalContext.swift | 2 + .../docs/experiments/summaries/06-prefill.md" | 0 .../Kernels/Attention/PrefillAttention.swift | 11 +- .../Runtime/Vision/VisionRuntime.swift | 18 +- ...nteractivityWatchdogEnvironmentTests.swift | 12 +- .../MetalContextDeviceCreationTests.swift | 42 +- .../Attention/PrefillAttentionTests.swift | 32 +- .../CommandBufferCompletionTests.swift | 24 + docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md | 201 ++++ docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md | 399 ++++++++ docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md | 404 ++++++++ ...SUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md | 456 +++++++++ docs/OPTIMIZATION_JOURNEY.md | 16 +- docs/TERMINAL_3D_ENGINE_PLAN.md | 886 ++++++++++++++++++ docs/experiments/EXPERIMENT_INVENTORY.md | 2 +- docs/experiments/summaries/06-prefill.md | 15 +- docs/issue84_simulate_payloads.py | 305 ++++++ 17 files changed, 2793 insertions(+), 32 deletions(-) create mode 100644 "Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift\npublic/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift\npublic/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift\npublic/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift\npublic/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift\npublic/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift\npublic/Sources/TurboFieldfareCLI/Run.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift\npublic/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift\npublic/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift\npublic/docs/OPTIMIZATION_JOURNEY.md\npublic/docs/RUNTIME_CONTROLS.md\npublic/docs/experiments/EXPERIMENT_INVENTORY.md\npublic/docs/experiments/summaries/06-prefill.md" create mode 100644 docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md create mode 100644 docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md create mode 100644 docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md create mode 100644 docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md create mode 100644 docs/TERMINAL_3D_ENGINE_PLAN.md create mode 100644 docs/issue84_simulate_payloads.py diff --git a/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift b/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift index 7b5fa670..db862311 100644 --- a/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift +++ b/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift @@ -104,11 +104,13 @@ public final class MetalContext: @unchecked Sendable { private let pipelineCacheLock = NSLock() private static func relaxInteractivityWatchdog() { + #if os(macOS) // The AGX driver reads this once at first device creation. Long prefill // dispatches can otherwise be killed as compositor-impacting on macOS // 26. Overwrite 0 preserves an operator's explicit stock-behaviour // override. This relaxes the deadline; it does not guarantee survival. setenv("AGX_RELAX_CDM_CTXSTORE_TIMEOUT", "1", 0) + #endif } /// Routes every production device creation through the watchdog mitigation diff --git "a/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift\npublic/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift\npublic/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift\npublic/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift\npublic/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift\npublic/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift\npublic/Sources/TurboFieldfareCLI/Run.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift\npublic/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift\npublic/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift\npublic/docs/OPTIMIZATION_JOURNEY.md\npublic/docs/RUNTIME_CONTROLS.md\npublic/docs/experiments/EXPERIMENT_INVENTORY.md\npublic/docs/experiments/summaries/06-prefill.md" "b/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift\npublic/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift\npublic/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift\npublic/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift\npublic/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift\npublic/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift\npublic/Sources/TurboFieldfareCLI/Run.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift\npublic/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift\npublic/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift\npublic/docs/OPTIMIZATION_JOURNEY.md\npublic/docs/RUNTIME_CONTROLS.md\npublic/docs/experiments/EXPERIMENT_INVENTORY.md\npublic/docs/experiments/summaries/06-prefill.md" new file mode 100644 index 00000000..e69de29b diff --git a/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift b/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift index 843ac6af..7b7df5e1 100644 --- a/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift +++ b/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift @@ -107,11 +107,14 @@ final class PrefillAttention { let requestsTensorOps = path == .fullTensorOps2DPreferred || path == .fullTensorOps2DValidityV2 - // The pinned model uses 512/16/2 only for full attention; its - // sliding-window layers use 256/16/8. A future model that reuses this - // shape for sliding attention must add a full-visibility check here. + // TensorOps starts its key loop at zero and ignores slidingWindow, so + // these are visibility guards rather than shape optimizations. + let windowNeverClips = effectiveParams.slidingWindow == 0 + || effectiveParams.slidingWindow >= effectiveParams.kvValidCount let tensorOpsShape = requestsTensorOps + && layerKind == .full && kvRingCapacity == 0 + && windowNeverClips && effectiveParams.headDim == 512 && effectiveParams.numQHeads == 16 && effectiveParams.numKVHeads == 2 @@ -123,7 +126,7 @@ final class PrefillAttention { pipeline = tensorOpsPipeline } else if tensorOpsShape && path == .fullTensorOps2DValidityV2 { preconditionFailure( - "TensorOps 2D prefill attention requires MSL 4 TensorOps pipeline support") + "TensorOps 2D prefill attention pipeline is unavailable on this Metal stack") } else { // Explicit mode also falls back for incompatible shapes. Benchmark // fixtures must use 512/16/2 to prove that TensorOps ran. diff --git a/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift b/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift index 4cd9d932..a8082e67 100644 --- a/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift +++ b/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift @@ -519,8 +519,10 @@ public final class VisionRuntime { let started = clock_gettime_nsec_np(CLOCK_UPTIME_RAW) entry.buffer.waitUntilCompleted() gpuWaitNanoseconds += clock_gettime_nsec_np(CLOCK_UPTIME_RAW) - started - if let error = entry.buffer.error { - throw VisionRuntimeError.commandFailed(String(describing: error)) + if let detail = metalCommandBufferFailureDetail(label: entry.buffer.label, + status: entry.buffer.status, + error: entry.buffer.error) { + throw VisionRuntimeError.commandFailed(detail) } gpuNanoseconds += UInt64( max(0, entry.buffer.gpuEndTime - entry.buffer.gpuStartTime) @@ -844,8 +846,10 @@ public final class VisionRuntime { let commandBuffer = try makeCommandBuffer() commandBuffer.commit() commandBuffer.waitUntilCompleted() - if let error = commandBuffer.error { - throw VisionRuntimeError.commandFailed(String(describing: error)) + if let detail = metalCommandBufferFailureDetail(label: commandBuffer.label, + status: commandBuffer.status, + error: commandBuffer.error) { + throw VisionRuntimeError.commandFailed(detail) } return clock_gettime_nsec_np(CLOCK_UPTIME_RAW) - started } @@ -853,8 +857,10 @@ public final class VisionRuntime { private func commitAndMeasure(_ commandBuffer: MTLCommandBuffer) throws -> UInt64 { commandBuffer.commit() commandBuffer.waitUntilCompleted() - if let error = commandBuffer.error { - throw VisionRuntimeError.commandFailed(String(describing: error)) + if let detail = metalCommandBufferFailureDetail(label: commandBuffer.label, + status: commandBuffer.status, + error: commandBuffer.error) { + throw VisionRuntimeError.commandFailed(detail) } return UInt64(max(0, commandBuffer.gpuEndTime - commandBuffer.gpuStartTime) * 1_000_000_000) diff --git a/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift b/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift index 02978de2..e094a420 100644 --- a/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift +++ b/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift @@ -24,7 +24,8 @@ import Testing try body() } - @Test func constructingAMetalContextSetsTheWatchdogRelaxation() throws { + #if os(macOS) + @Test func constructingAMetalContextSetsTheWatchdogRelaxationOnMacOS() throws { try Self.withCleanEnvironment { #expect(Self.read() == nil) _ = try MetalContext() @@ -39,4 +40,13 @@ import Testing #expect(Self.read() == "0") } } + #else + @Test func constructingAMetalContextDoesNotSetTheMacOSWatchdogRelaxation() throws { + try Self.withCleanEnvironment { + #expect(Self.read() == nil) + _ = try MetalContext() + #expect(Self.read() == nil) + } + } + #endif } diff --git a/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift b/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift index 8da8415d..6478aace 100644 --- a/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift +++ b/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift @@ -12,28 +12,54 @@ import Testing .appendingPathComponent("Sources") } + private static let sanctionedRelativePath = + "TurboFieldfare/Infrastructure/Metal/MetalContext.swift" + @Test func onlyMetalContextCreatesTheSystemDevice() throws { let sources = Self.sourcesDirectory try #require(FileManager.default.fileExists(atPath: sources.path), "cannot locate Sources/ from \(#filePath)") + var enumerationFailures: [String] = [] + var readFailures: [String] = [] var offenders: [String] = [] - let walker = FileManager.default.enumerator( + let walker = try #require(FileManager.default.enumerator( at: sources, - includingPropertiesForKeys: nil) + includingPropertiesForKeys: nil, + errorHandler: { url, error in + enumerationFailures.append("\(url.path): \(error)") + return false + })) + let sourcePrefix = sources.standardizedFileURL.path + "/" - while let url = walker?.nextObject() as? URL { - guard url.pathExtension == "swift", - url.lastPathComponent != "MetalContext.swift", - let text = try? String(contentsOf: url, encoding: .utf8), - text.contains("MTLCreateSystemDefaultDevice(") else { continue } + while let url = walker.nextObject() as? URL { + guard url.pathExtension == "swift" else { continue } + let path = url.standardizedFileURL.path + guard path.hasPrefix(sourcePrefix) else { + readFailures.append("cannot make \(path) relative to \(sources.path)") + continue + } + let relativePath = String(path.dropFirst(sourcePrefix.count)) + guard relativePath != Self.sanctionedRelativePath else { continue } + let text: String + do { + text = try String(contentsOf: url, encoding: .utf8) + } catch { + readFailures.append("\(relativePath): \(error)") + continue + } + guard text.contains("MTLCreateSystemDefaultDevice(") else { continue } for (index, line) in text.split(separator: "\n", omittingEmptySubsequences: false) .enumerated() where line.contains("MTLCreateSystemDefaultDevice(") { - offenders.append("\(url.lastPathComponent):\(index + 1)") + offenders.append("\(relativePath):\(index + 1)") } } + #expect(enumerationFailures.isEmpty, + "source enumeration failed: \(enumerationFailures.joined(separator: "; "))") + #expect(readFailures.isEmpty, + "source reads failed: \(readFailures.joined(separator: "; "))") #expect(offenders.isEmpty, "Production device creation bypasses MetalContext: \(offenders.joined(separator: ", "))") } diff --git a/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift b/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift index 79ae1325..c4fb6f39 100644 --- a/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift +++ b/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift @@ -216,6 +216,32 @@ import TurboFieldfareValidationSupport #expect(preferred == tiled) } + /// TensorOps starts its key loop at zero, so a production-shaped request + /// with a clipping window must use the tiled path even when the pipeline is + /// available. + @Test func preferredTensorOpsPathRejectsAWindowThatActuallyClips() throws { + let context = try MetalContext() + let attention = try PrefillAttention(context: context) + guard attention.tensorOps2DValidityV2Available else { return } + + let fixture = Self.makeFixture(start: 40, + chunk: 8, + window: 16, + seed: 0xA877, + headDim: 512, + qHeads: 16, + kvHeads: 2) + let preferred = try Self.runKernel( + fixture, + path: .fullTensorOps2DPreferred, + layerKindOverride: .full) + let tiled = try Self.runKernel(fixture, + path: .causalTiled, + layerKindOverride: .full) + + #expect(RelError.maxAbsDiff(preferred, tiled) == 0) + } + private static func makeFixture(start: Int, chunk: Int, window: Int, @@ -340,7 +366,8 @@ import TurboFieldfareValidationSupport _ fixture: Fixture, kvRingCapacity: UInt32 = 0, path: RuntimePrefillAttentionPath = .causalTiled, - simulatingMissingTensorOps: Bool = false + simulatingMissingTensorOps: Bool = false, + layerKindOverride: PrefillAttentionLayerKind? = nil ) throws -> [Float] { let ctx = try MetalContext() let prefill = try PrefillAttention( @@ -389,7 +416,8 @@ import TurboFieldfareValidationSupport outOffset: oPrefix * MemoryLayout.size, params: params, kvRingCapacity: kvRingCapacity, - layerKind: fixture.window == 0 ? .full : .slidingWindow, + layerKind: layerKindOverride + ?? (fixture.window == 0 ? .full : .slidingWindow), path: path) cb.commit() cb.waitUntilCompleted() diff --git a/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift b/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift index 667bf338..f08a947f 100644 --- a/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift +++ b/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift @@ -5,6 +5,17 @@ import Testing @testable import TurboFieldfare @Suite struct CommandBufferCompletionTests { + private static var visionRuntimeSource: URL { + URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift") + } + private static func nsError(domain: String = "MTLCommandBufferErrorDomain", code: Int = 1, description: String, @@ -127,4 +138,17 @@ import Testing #expect("\(error)".contains("test uncommitted")) } } + + /// The shared formatter covers status-only failures only if every vision + /// wait routes through it. A direct `buffer.error` check accepts `.error` + /// with no error object and can consume incomplete GPU output. + @Test func visionRuntimeUsesStatusAwareCommandBufferChecks() throws { + let source = try String(contentsOf: Self.visionRuntimeSource, encoding: .utf8) + #expect(!source.contains("if let error = entry.buffer.error")) + #expect(!source.contains("if let error = commandBuffer.error")) + let checkedWaits = source.components( + separatedBy: "metalCommandBufferFailureDetail(").count - 1 + #expect(checkedWaits == 3, + "expected all three VisionRuntime waits to preserve status diagnostics") + } } diff --git a/docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md b/docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md new file mode 100644 index 00000000..56c0fc63 --- /dev/null +++ b/docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md @@ -0,0 +1,201 @@ +# Independent Review Request: TurboFieldfare Issue #84 + +Perform a read-only, evidence-based investigation. Do not implement or edit +anything. Separate the immediate failure mechanism from the underlying cause, +and independently verify every conclusion below. + +## Primary Sources + +- Issue: +- New reproduction: + +- Diagnostic PR already merged: + +- Proposed grammar-constrained fix: + + +Repository: + +```text +/Users/andreymikhaylov/development/turbo-fieldfare +``` + +Refresh the current GitHub and `origin/main` state before drawing conclusions. + +## Original Report + +An OpenCode client successfully completed two tool-calling turns, then failed +on the third: + +```text +prepared prompt=23803 -> completed in 46.575s cached=16742 completion=57 finish=tool_calls +prepared prompt=25658 -> completed in 14.396s cached=23875 completion=57 finish=tool_calls +prepared prompt=27513 -> failed after 231s phase=generating status=500 +error=TurboFieldfare.GemmaToolCallParserError.malformed +``` + +Setup: + +- Commit `fcd8f78` +- `--max-context 65536` +- Apple Silicon, 48 GB +- Tool definitions sent on each turn + +The original theory was drift or truncation after a very long generation, but +raw generated output was unavailable. + +## New Independent Reproduction + +The linked comment reports: + +- Commit `acefaf1` +- Hermes Agent using `/v1/chat/completions` +- 26 tools +- Two-message conversation +- 17,355 rendered prompt tokens +- Failure is reportedly 100% reproducible at this prompt size +- Same result at temperature `0.2` and greedy temperature `0` + +Diagnostics: + +```text +completion_tokens=16 max_completion_tokens=48181 raw_stop=stop_string +tool_start_count=1 tool_end_count=1 decoded_calls=0 +last_tool_start_offset=0 last_tool_end_offset=15 +effective_count_matches_result=true effective_prefix_matches_kv=true +kv_position_matches_history=true completion_count_matches_history=true +prefill_accounting_matches=true +``` + +The commenter also reports different generated-token hashes across two +ostensibly identical temperature-0 attempts. + +## Current Interpretation to Verify + +The immediate failure mechanism appears clear: + +1. Generated token 0 is `<|tool_call>`. +2. Tokens 1-14 form the tool-call payload. +3. Token 15 is ``. +4. When the closing marker arrives, `StructuredAssistantDecoder` decodes the + collected payload. +5. `GemmaToolCallParser` expects: + + ```text + call:{...} + ``` + +6. The payload fails that parser, producing + `decoder_consume cause=malformed`. +7. The server stops generation and returns HTTP 500. + +Relevant files: + +- `Sources/TurboFieldfareServer/Core/ServerInference.swift` +- `Sources/TurboFieldfare/Tokenization/StructuredAssistantDecoder.swift` +- `Sources/TurboFieldfare/Tokenization/GemmaToolCallParser.swift` +- `Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift` +- `Sources/TurboFieldfareServer/Core/GemmaToolSchema.swift` + +Important nuance: `raw_stop=stop_string` probably does not mean a +client-supplied stop string matched. `ServerInference` sets `shouldStop = true` +after the decoder throws, while `RawCompletion` combines +`stopMatcher.isStopped || shouldStop` and records either condition as +`.stopString`. Verify this carefully. + +## What the Diagnostics Seem to Establish + +- This reproduction is not an unfinished tool block or maximum-token + truncation. +- The model emitted both opening and closing tool markers. +- Parsing failed while consuming the closing marker. +- The failure is not merely an orphan `<|tool_response>` marker. +- It reproduces under greedy generation, so ordinary sampling luck is + unlikely. +- The server's token-history, prefix-cache, position, and prefill-count + bookkeeping is internally consistent. + +Do not overinterpret the KV checks. They compare token IDs, counts, and +positions. They do not validate the numerical K/V tensors or the correctness +of long-context attention or prefill computation. + +Also challenge the commenter's claim that 14 payload tokens are "nowhere near +enough" for a valid call. A short tool name with an empty or small argument +object may fit. The token count proves that the closed block was malformed, +not why it was malformed. + +## Main Unresolved Question + +What were the actual payload bytes or token IDs between the two tool markers? + +Without that evidence, distinguish among: + +1. The model emitted canonical JSON instead of Gemma's native + `call:name{...}` dialect. +2. The model emitted another malformed native-dialect call. +3. The parser rejects a representation that should reasonably be accepted. +4. Detokenization changed the payload before parsing. +5. Long-context prefill or attention produced incorrect logits despite + consistent bookkeeping. +6. A nondeterministic runtime issue explains the differing temperature-0 + hashes. +7. The two "identical" greedy attempts were not actually identical in prompt + IDs, cache state, runtime configuration, or process state. + +## PR #107 + +PR #107 claims its private reproduction showed the model drifting to canonical +JSON inside `<|tool_call>`. It adds grammar-constrained decoding to prevent +invalid tool payloads. + +Treat that claim as a hypothesis requiring independent verification because: + +- The public issue comment does not include the raw payload. +- The end-to-end fixture used by the PR author is private and not checked in. +- The PR is broad, adding general forced-JSON support as well as the + issue-specific tool grammar. +- At the previous inspection it was open, conflicting, and had no reported + GitHub checks; refresh this state. + +Assess whether PR #107: + +- fixes the demonstrated cause or merely masks malformed generation; +- is necessary compared with accepting both native and canonical + representations; +- could conceal a long-context numerical correctness problem; +- preserves prompt-cache and fail-closed semantics; +- handles incomplete calls, UTF-8, byte limits, unknown tools, and completion + budgets correctly; and +- is appropriately scoped for issue #84. + +## Template Observation + +The commenter noticed several unguarded `value['type'] | upper` expressions in +`chat_template.jinja`. + +The current server's `GemmaToolSchema` adapter appears to reject unsupported +types and normalize nullable type arrays to one concrete string type before +invoking the template. Verify whether every unguarded Jinja path is therefore +unreachable through validated OpenAI requests. + +Treat this as separate from issue #84 unless a concrete request reaches the +Jinja error. + +## Requested Output + +Return: + +1. A direct verdict: is the immediate failure understood? +2. A separate verdict: is the underlying root cause established? +3. Confirmed facts, probable explanations, and unsupported claims. +4. An explanation of the temperature-0 hash discrepancy. +5. An assessment of PR #107's root-cause claim and scope. +6. The smallest decisive next experiment. +7. Any mistakes in the interpretation above. + +The likely decisive experiment is to obtain the reporter's full request or a +minimized reproducer, run current `main` at temperature 0 with prompt caching +disabled, and capture the exact bounded payload between `<|tool_call>` and +``. Do not expose private prompt text in public logs. + +No source files should be changed during this review. diff --git a/docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md b/docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md new file mode 100644 index 00000000..818e4a02 --- /dev/null +++ b/docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md @@ -0,0 +1,399 @@ +# Independent Review Findings: Issue #84 + +Read-only investigation performed 2026-08-14 against `origin/main` (`3e87d92`) +per `docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md`. No source files were changed. + +GitHub state at review time: + +- Issue #84: OPEN. Comments: two maintainer replies, then the independent + reproduction from `idallasj` (2026-08-13, comment 5284001871). +- PR #90 (diagnostics): merged; the reproduction used its output. +- PR #107 (grammar-constrained decoding): OPEN, CONFLICTING, no CI checks. + +## 1. Verdict: immediate failure mechanism — understood, with one correction + +The brief's step-by-step mechanism is confirmed against `main`: + +- `StructuredAssistantDecoder.consume` collects payload token IDs between + `<|tool_call>` and ``, re-decodes them with + `decode(skipSpecialTokens: false)`, and hands the text to + `GemmaToolCallParser.parse` + (`Sources/TurboFieldfare/Tokenization/StructuredAssistantDecoder.swift:71-87`). +- On parse failure, `ServerInference` records the error and sets + `shouldStop = true` + (`Sources/TurboFieldfareServer/Core/ServerInference.swift:590-593`). + `RawCompletion.swift:221-226` folds `stopMatcher.isStopped || shouldStop()` + into one label and records `reason = .stopString`. **`raw_stop=stop_string` + in the diagnostics is the server's own post-error abort, not a client stop + string.** The brief's nuance is verified correct. +- `completion_tokens=16` with `last_tool_end_offset=15` matches the loop + arithmetic exactly: token 0 opens the call, tokens 1–14 are payload, token + 15 closes it, the decoder throws, the loop aborts on the next check. + +Correction: **the two reproductions are not the same failure shape.** + +- Original OpenCode report (`fcd8f78`, 231s, ~3k tokens, thrown at old + `ServerInference.swift:365`): the *orphan `<|tool_response>`* guard. The + model sampled the stop sentinel with zero complete calls — it either never + opened a call or wrote something else for thousands of tokens. Post-#90 + this would be `kind=orphan_tool_response cause=none`. +- New Hermes repro (`acefaf1`): `kind=decoder_consume cause=malformed` — a + **closed 14-payload-token block that failed parsing** after only 16 + generated tokens. + +Both are "model output the parser refuses," but evidence from one does not +automatically transfer to the other. + +## 2. Verdict: underlying root cause — NOT established + +The payload bytes between the markers are unknown and nothing checked in can +recover them (the #90 diagnostics deliberately log no content). Assessment of +the brief's seven candidates: + +1. **Canonical JSON instead of `call:name{...}`** — most probable, unproven. + 14 tokens fits `{"name": "x", "arguments": {}}` comfortably. Contributing + prior: `encodeToolChat` hardcodes `enable_thinking: false` + (`Tokenizer.swift:368-372`), so the template force-closes an empty thought + channel and the model must emit the call format with zero deliberation + tokens at a 17k-token prompt. +2. **Another malformed native call** — equally possible; the parser also + rejects `call :`, leading-zero/`+`-prefixed numbers, trailing text. +3. **Parser rejects what it should accept** — partially confirmed as latent + bugs, but they cannot be this specific failure: + - `OpenAIToolName.isValid` (`OpenAIModels.swift:258-269`) accepts hyphens; + `identifier()` (`GemmaToolCallParser.swift:88`) stops at them. A + correctly emitted `call:web-search{...}` is undecodable. But that path + throws `unknownTool`, and the diagnostic says `cause=malformed`, so the + Hermes failure is not the hyphen bug. + - `jsonString()` (`GemmaToolCallParser.swift:170-183`) loops through + `take()`, which calls `skipWhitespace()` first — spaces/tabs/newlines + inside JSON string values are silently eaten (`"/tmp/a b"` → `"/tmp/ab"`). + Silent corruption, not a throw. +4. **Detokenization changed the payload** — weak on current `main`: #118 + (`acefaf1`) made decode lossless by construction (`GemmaDecoding`, no + `clean_up_tokenization_spaces`). PR #107's "cleanup swallows bytes" + rationale describes the pre-#118 decoder. +5. **Long-context numeric error** — cannot be ruled out; the KV diagnostics + compare token IDs/counts/positions only, never tensor values. +6. **Nondeterministic runtime** — ruled out for the GPU code (see §4). +7. **The two "identical" attempts were not identical** — favored explanation + for the hash discrepancy (see §4), and decidable from existing diagnostics. + +Signal worth noting: 100% reproducibility *with differing token hashes* means +the failure is robust across trajectory variation — this fits systematic +format drift better than a knife's-edge numeric fault. + +The commenter's "14 tokens is nowhere near enough for a real call" is wrong: a +valid minimal call (`call:` + short name + `{}`) fits in fewer tokens. The +count proves only that the closed block was malformed, not why. + +### Payload simulation (2026-08-14) + +Since the payload bytes are unknown, candidate payloads were simulated with +the real tokenizer (`scratch/gemma4.gturbo/tokenizer/tokenizer.json`) and a +bug-faithful Python port of `GemmaToolCallParser` (both known bugs +reproduced). Script: `docs/issue84_simulate_payloads.py` (needs +`pip install tokenizers`; the port was validated against all predicted +parser behaviors — space-eating corruption, hyphen/dot → `unknown_tool`, +backslash-space → `malformed`, gemma strings parse). + +Observed signature to match: exactly 14 payload tokens, `cause=malformed`. +Token-count bands across 9 representative tool names: + +| payload shape | verdict | tokens (minimal args) | +| --- | --- | --- | +| canonical JSON `{"name": "N", "arguments": "{}"}` (string-typed args, i.e. the OpenAI wire serialization) | malformed | **exactly 14** for every 2-token name tried | +| canonical JSON `{"name": "N", "arguments": {}}` | malformed | 13 | +| canonical JSON, one short arg | malformed | 15–17 | +| `{"tool": "N", "args": {}}` / fenced ```` ```json ```` block | malformed | 13 | +| native near-misses (`Call:`, missing colon, paren args, single quotes, unquoted value, JSON-quoted keys, trailing text) | malformed | 3–11 | +| valid native `call:N{}` / gemma-string args | ok | 4–12 | + +Read: the observed 14-token `malformed` block sits squarely in the +**canonical-JSON band** and is an exact-width match for the OpenAI wire +format with string-encoded empty `arguments` — the precise shape a model +imitating the request-side JSON it was shown would produce. Native-dialect +near-misses are systematically too short unless they carry real arguments. + +Caveats: the model's own token split need not equal canonical BPE encoding +(±1–2 tokens), and the real Hermes tool names are unknown; this narrows the +hypothesis space, it does not replace capturing the payload. But it upgrades +hypothesis 1 (canonical-JSON drift) from "most probable" to "strongly +favored", and it demonstrates that essentially *any* JSON-shaped drift +classifies as `malformed` while dialect-prefix drift (`call:` + bad name) +classifies as `unknown_tool` — so the observed `cause=malformed` itself is +evidence the model abandoned the `call:` prefix entirely. + +## 3. Confirmed facts, probable explanations, unsupported claims + +**Confirmed by code reading:** + +- The full immediate mechanism in §1, including the `stop_string` mislabeling. +- Both parser bugs claimed by PR #107 exist on `main` (hyphen identifier, + whitespace-eating `jsonString`), and its diff for them is correct. +- The server accepts tool names (`-` allowed) that the parser can never + decode — a validator/parser alphabet mismatch. +- Payload decode on `main` is lossless (`Tokenizer.swift:233-249`, + `Detokenizer.swift`). +- A failed request invalidates the prompt cache and resets the runner via the + `defer` in `generate` (`ServerInference.swift:486-491`). + +**Probable:** the model drifted out of the native dialect (canonical JSON or +similar) under a 17k prompt with no thinking budget. + +**Unsupported:** PR #107's claim that its private repro's drift is this +issue's cause; any inference from KV bookkeeping consistency to numeric +correctness; treating the OpenCode and Hermes failures as one mechanism. + +## 4. Temperature-0 hash discrepancy + +A full kernel audit found **no run-to-run nondeterminism in the GPU code**: + +- Greedy argmax is deterministic: fixed lane→element mappings, `simd_max` + + lowest-index tie-break at every level (`Metal/Sampling/logit.metal:687-787`, + same construction in the `sample` kernel's greedy branch). +- MoE expert streaming cannot reorder accumulation: each pread writes a + pre-assigned slot; the combine (`moe_phase2_down_reduce_k8`, + `Metal/MoE/moe.metal:446-482`) sums in router-rank order on a single + thread. Expert-cache hit/miss subset kernels are bit-identical to the full + kernel. +- No float atomics, no concurrent dispatch, fixed-order split-KV attention + combine. + +But two identical-looking runs can still diverge legitimately: + +- **Prefill and decode use different kernel families with different + precision** (tensor-core MPP path dequantizes weights to `half`, + `Metal/TensorCore/tensorops.metal:75`; decode GEMV uses FP32 fma). A chunk + under 32 tokens switches kernels again + (`RealForwardRunner.swift:646`, dispatch policy `:118-131`). Chunk + boundaries shift with `cachedPromptTokens` + (`PrefillRuntimeConfig.swift:95-116`). +- So a **cache-resume legitimately produces different logits than a fresh + prefill** of the same tokens. The MoE router's discrete top-8 selection + (`moe.metal:135-185`) amplifies 1-ULP differences into O(1) activation + changes; at 17k context a near-tie top-2 flips the argmax with no bug. +- At temperature 0.2 the sampler seeds from `CLOCK_MONOTONIC` per token + unless the client sends `seed` (`Sampler.swift:198-208`) — nondeterministic + by design. + +Likely explanations for the differing temp-0 hashes, in order: different +cache/KV state between the attempts (attempt ordering matters because +failures invalidate the cache), or prompts that were not byte-identical +(agent frameworks often embed timestamps). **Decidable today with zero new +code:** the two failure lines already carry `rendered_prompt_i32le_sha256` +and `cached_prompt_tokens` — ask the commenter to diff those fields across +the two attempts. Identical rendered hashes and cached counts with different +generated hashes would be a genuinely new finding deserving its own issue. + +## 5. PR #107 assessment + +- **Root-cause claim: unverified.** The drift-to-JSON evidence is a private + fixture; the public repro has no payload. Plausible, but the PR should not + merge under the banner "root-cause fix" on current evidence. +- **Fix vs mask:** grammar-constrained decoding is standard, legitimate + engineering for tool calls, but it is a behavioral guarantee, not a + root-cause fix — and it would conceal hypothesis 5 (bad logits would + silently produce well-formed but wrong calls) and mask the drift signal. + Acceptable as a knowing product decision. +- **Scope: too broad for #84.** `--force-json` CLI mode, `TokenByteTable`, + and the sampler rework are unrelated to the issue. The two parser fixes are + correct and necessary regardless — worth extracting into a small standalone + PR together with a decision on accepting canonical-JSON payloads as a + fallback. +- **Staleness:** it predates #118; its decoder rationale (library `cleanUp` + mangling bytes) no longer describes `main`, and its + `StructuredAssistantDecoder` diff conflicts with the rewritten one. + +## 6. Smallest decisive next experiment + +The reporter explicitly offered the full request payload — take it. Then on +current `main`: + +1. Run the captured request at `temperature 0` with `--prompt-cache-mode off`, + fresh process per attempt. +2. Capture the bounded region between `<|tool_call>` and ``. This + requires the one thing #90 withheld: an opt-in, failure-only, flag-gated + dump of just the tool-region token IDs (~10 lines). +3. Cheaper first step, zero code: have the reporter rerun twice and diff the + two diagnostic lines' `rendered_prompt_i32le_sha256` and + `cached_prompt_tokens` fields. + +The payload bytes immediately separate hypotheses 1/2/3; the cache-off +determinism run separates 5/6/7. + +## 7. Test and fix plan (deferred — all details for later execution) + +State of coverage on `main`: `GemmaToolCallParser` and +`StructuredAssistantDecoder` have **no dedicated unit tests**. Only indirect +references exist, in `Tests/TurboFieldfareServer/OpenAIValidationTests.swift` +and `Tests/TurboFieldfareServer/StructuredOutputDiagnosticsTests.swift`. +There is no `Tests/.../GemmaToolCallParserTests.swift` — PR #107 adds one, +but on its own conflicted branch. + +### Phase A — confirmed bugs, writable today, no model, no decisions needed + +Create `Tests/TurboFieldfare/Core/Tokenization/GemmaToolCallParserTests.swift`. + +**A1. Hyphenated tool names are undecodable.** + +- Bug: `OpenAIToolName.isValid` + (`Sources/TurboFieldfareServer/Core/OpenAIModels.swift:258-269`) accepts + byte 45 (`-`); `identifier()` + (`Sources/TurboFieldfare/Tokenization/GemmaToolCallParser.swift:88`) + accepts only letters/digits/`_` and stops at `-`. +- Failing test: `parse("call:web-search{}", allowedTools: ["web-search"], + id: "t")` — currently throws `unknownTool("web")`; must return a call + named `web-search` with empty arguments. +- Fix: extend the `identifier()` alphabet with `-` (PR #107's one-line diff + for this is correct and can be lifted verbatim). +- Hardening test: property test over the `OpenAIToolName` alphabet + (`[A-Za-z0-9_-]{1,64}`): every valid name must round-trip through + `parse("call:{}", allowedTools: [name])`. Pins the validator and + parser alphabets together permanently. + +**A2. `jsonString()` silently eats whitespace inside string values.** + +- Bug: the loop at `GemmaToolCallParser.swift:170-183` matches quote and + backslash via `take()`, which calls `skipWhitespace()` first + (`:278-283`) — every space/tab/newline inside a `"…"` value is dropped. +- Failing tests: + - `parse("call:read{path:\"/tmp/a b\tc\"}", ...)` → argument must be + `/tmp/a b\tc`; currently decodes as `/tmp/abc` (silent corruption, no + throw). + - Backslash-space inside a string (`"a\ b"` in payload bytes): currently + throws `malformed` via `escapedFragment()` because the space after `\` + is treated as an escape char; after the fix `\` + invalid escape should + still throw, but a literal space must never be consumed by + `skipWhitespace`. +- Fix: read characters positionally inside the string loop instead of via + `take()` (PR #107's diff for `jsonString()` is correct and liftable). +- Note: `gemmaString()` (`:151-168`) reads positionally and does NOT have + this bug — add a test locking that in (`<|"|>a b<|"|>` keeps its space). + +**A3. Jinja `| upper` crash is reachable through validated requests.** + +- Mechanism: `GemmaToolSchema.adapt` + (`Sources/TurboFieldfareServer/Core/GemmaToolSchema.swift`) validates + `type` only where it recurses (`properties`, `items`); annotation values + (`default`, `examples`, `title`, `$comment`, …) pass through untouched. + `validateSchemaKeys` (`OpenAIModels.swift:382-410`) whitelists nothing — + it only charset-checks property names. The template's + `filter_keys=true` branch (in `format_parameters`, + `scratch/gemma4.gturbo/tokenizer/chat_template.jinja`) is taken for an + object-typed schema with **no** `properties` key and iterates all + non-standard keys (standard = description/type/properties/required/ + nullable) as if they were property schemas, evaluating + `value['type'] | upper` on arbitrary annotation values. +- Reproducer schema (passes validation, expected to crash at render): + + ```json + {"type": "object", "properties": {"cfg": {"type": "object", "default": {}}}} + ``` + +- Test location: `Tests/TurboFieldfare/Core/Tokenization/ChatTemplateTests.swift` + (or the server validation suite if rendering there is easier). First + assert current behavior (Jinja runtime error, e.g. "upper filter requires + string") to confirm reachability, then flip the assertion after fixing. +- Fix options: (a) guard the 6 unguarded `value['type'] | upper` template + sites the commenter identified (they offered their diff); (b) make + `GemmaToolSchema` strip or whitelist annotation keys so the + `filter_keys=true` path can never see non-schema values. (b) is safer: + it also fixes `default: 5` (subscripting an int) and keeps the template + in sync with upstream. +- This is a separate crash class from #84 (render-time, before generation). + +### Phase B — writable today but each encodes a product decision + +**B1. Accept canonical JSON tool-call payloads.** + +- Test: `parse("{\"name\": \"x\", \"arguments\": {\"a\": 1}}", + allowedTools: ["x"])` decodes as a call to `x`. +- This is the direct fix for hypothesis 1 (most probable cause of the + Hermes repro) and the lightweight alternative to PR #107's grammar. Risk: + if the captured payload turns out to be something else, this fixes a + different bug than #84. Decide after the payload capture (Phase C), or + accept both dialects proactively — accepting both is strictly more + permissive and cannot break existing native-dialect parses. + +**B2. Fail-open server behavior (original reporter's actual ask).** + +- Behavior change: when `StructuredAssistantDecoder` fails, return the raw + generated text as assistant `content` with `finish_reason: "stop"` + instead of HTTP 500, so agent clients can retry/recover. +- Test at `ServerInference.generate` level with a scripted + `LogitProducer`/backend emitting a malformed tool block; assert 200-path + completion, cache invalidated (the current `defer` semantics must stay), + and no partial `tool_calls` array. +- Tension: current design is deliberately fail-closed; #90's plan doc lists + "returning raw generation as assistant content" as an explicit non-goal. + This needs a maintainer decision, not just a patch. + +**B3. Diagnostics honesty for the abort label.** + +- `raw_stop=stop_string` currently conflates a client stop match with the + server's own decoder-abort (`RawCompletion.swift:221-226` + + `ServerInference.swift:590-593`). Either add a distinct `StopReason` + label for the abort, or add a test asserting `stop_string_matched=false` + always accompanies decoder-abort lines so log readers can distinguish. + (#90's plan forbade changing `StopReason` — a doc/test-only clarification + is the minimal version.) + +### Phase C — blocked on evidence (the actual #84 regression test) + +The regression test for the Hermes `cause=malformed` failure cannot be +written yet: the payload bytes between `<|tool_call>` and `` are +unknown, so there is no string to assert on. Required sequence: + +1. Get the reporter's full request (offered in the issue comment). +2. Zero-code first step: have the reporter run the failing request twice and + diff `rendered_prompt_i32le_sha256` and `cached_prompt_tokens` across the + two diagnostic lines (resolves the temp-0 hash puzzle: prompt + non-identity vs cache-state difference). +3. Add an opt-in, failure-only, flag-gated dump of the tool-region token IDs + (~10 lines in `ServerInference`; #90 deliberately withheld this, so it + must be explicit opt-in). +4. Run on current `main`, `temperature 0`, `--prompt-cache-mode off`, fresh + process per attempt; capture the payload. +5. Turn the captured payload into a fixed unit-test string in + `GemmaToolCallParserTests` — the true #84 regression test — and only + then decide between B1 (accept canonical JSON), PR #107's grammar + constraint, or a numeric investigation (if the payload is garbage bytes + rather than a recognizable format). + +### Recommended PR ordering + +1. Phase A1+A2 (pure parser bug fixes + new test file; zero risk, needed + regardless of #84's root cause). +2. Phase A3 (template/schema guard, separate crash class, cite the + commenter's report). +3. Phase C instrumentation (opt-in dump flag) + ask the reporter to rerun. +4. Phase B decisions once the payload is in hand; evaluate PR #107 against + the evidence then (its parser fixes will already be merged via step 1; + its remaining value is the grammar constraint, which should be judged as + a product feature, not a bug fix). + +Validation commands used by this repo: + +```bash +swift build -c release +Scripts/test.sh +ruby Scripts/check_markdown_links.rb +``` + +## 8. Mistakes in the brief's interpretation + +- It treats both reproductions as one failure class; they differ in kind + (orphan sentinel vs failed closed block) and scale (~3k vs 16 tokens). +- The template observation is understated: the unguarded Jinja `| upper` + paths are **not** all unreachable. `GemmaToolSchema.adapt` never validates + annotation values (`default`, `examples`, …), and the template's + `filter_keys=true` branch — taken for an object-typed parameter with + annotations but no `properties` key — iterates those annotation values as + property schemas and hits unguarded `value['type'] | upper`. A validated + request like `{"type":"object","default":{}}` as a parameter plausibly + reaches the crash the commenter couldn't isolate. One unit test would + confirm. +- "Deterministic at temp 0" must not be assumed even absent bugs: + cache-resume vs fresh prefill is a numeric fork by design, and temp 0.2 is + clock-seeded by design. diff --git a/docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md b/docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md new file mode 100644 index 00000000..603245a6 --- /dev/null +++ b/docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md @@ -0,0 +1,404 @@ +# Issue 84: Root-Cause Test Plan + +## Purpose + +Determine why issue 84 produces a closed `<|tool_call>...` block +that `GemmaToolCallParser` rejects. The immediate failure path is already +understood; this plan isolates the source of the malformed payload. + +Treat the two known reports as separate cases until evidence shows they share a +cause: + +- **Case O — original OpenCode flow:** two successful tool turns followed by a + long third generation and HTTP 500. +- **Case H — Hermes reproduction:** a two-message, 26-tool request that closes a + malformed tool block after 16 generated tokens. + +This plan complements: + +- [Independent review brief](ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md) +- [Structured tool-generation diagnostics](ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md) +- [Local server guide](OPENAI_SERVER.md) +- [Runtime controls](RUNTIME_CONTROLS.md) + +## Root-cause standard + +Do not call a hypothesis the root cause merely because one run succeeds after +changing a setting. A root cause is established only when all of the following +are true: + +1. The exact request, rendered prompt token IDs, generation settings, runtime + settings, model revision, and process state are known. +2. The exact generated token IDs and bytes inside the rejected tool block are + captured privately. +3. One controlled change predicts the first output or logit divergence. +4. An A-B-A check removes the failure and then restores it, or an independent + oracle reproduces the same first divergence. +5. The explanation accounts for the entire failing path without contradicting + successful tool turns. + +Case O and Case H may finish with different root causes. + +## Safety and privacy rules + +- Follow the repository model-process, OS, Swift, disk, memory-pressure, and + completed-model checks before every model session. +- Run only one TurboFieldfare, MLX-LM, app, CLI, server, or model-backed test at + a time. +- Keep the server on `127.0.0.1`; do not proxy, tunnel, or expose it. +- Do not download another checkpoint, duplicate the `.gturbo` model, purge + caches, or create a worktree for this investigation. +- Keep full requests, raw generations, tool schemas, tool results, and token + sequences outside the repository. They may contain private or reversible + information. +- Public logs and issue updates may contain bounded counts and hashes, but not + prompt text, generated text, tool names, arguments, results, or token arrays. +- Do not alter the installed `.gturbo` sidecars. Render template variants + out-of-process and record the template hash. +- A temporary local probe or oracle harness requires separate authorization. + Keep it uncommitted until its output proves that a durable test is needed. + +## Required inputs + +Obtain these separately for Case O and Case H before running a comparison: + +- complete request JSON for every turn, including tool order and full schemas; +- original server command and commit; +- whether the request was streaming; +- client name and version; +- generation options, especially temperature, maximum completion tokens, stop + strings, seed, Top-K, and Top-P; +- server runtime settings and prompt-cache mode; +- complete request-correlated diagnostic line; +- for Case O, all messages returned by the server and tool results appended by + the client during the two successful turns. + +Store the private request unchanged. Generate experimental variants from it so +the original remains a byte-for-byte reference. + +## Evidence record + +Create one row per run in a private results table: + +| Field | Required value | +| --- | --- | +| Run ID | Stable case and variant label | +| Code | `git rev-parse HEAD` and dirty-state summary | +| Host | Hardware, RAM, macOS, Swift version | +| Model | `.gturbo` manifest/revision and tokenizer/template hashes | +| Request | Private request SHA-256 and turn number | +| Prompt | Rendered/effective token counts and token hashes | +| Runtime | Full server command and process freshness | +| Generation | Temperature, limits, stop options, Top-K, Top-P, seed | +| Result | Exit/HTTP status, timing footer, finish reason | +| Structure | Marker counts/offsets, decoder phase, parser cause | +| Output | Private generated token IDs and exact tool-payload bytes | + +If rendered or effective prompt hashes differ between supposedly identical +runs, stop. Fix the harness or explain the changed input before comparing +generated output. + +## Phase 1: Capture the decisive artifact + +### 1.1 Reproduce on current `main` + +Build release once. Start with: + +- temperature `0`; +- prompt cache off; +- prefill on with 128-token chunks; +- 16 expert-cache slots, LFU; +- RDADVISE off; +- the reporter's context and completion limits; +- a fresh server process. + +Submit the exact request three times. Do not run unrelated warmups. For Case O, +replay the complete three-turn flow; do not submit only its final request unless +that final request is independently shown to reproduce with cache off. + +The baseline is usable when either: + +- all three runs fail with the same generated token hash and parser cause; or +- the inputs match but output hashes differ, proving a deterministic-runtime + investigation is required. + +### 1.2 Capture raw token evidence privately + +Capture, before structured parsing: + +- all generated token IDs; +- exact token IDs between the final tool-start and tool-end markers; +- the exact bytes produced from those IDs with special-token skipping disabled; +- incremental decoder deltas around both markers; +- final stop reason and boundary token IDs; +- first parser error and allowed-tool set hash. + +Prefer a debugger breakpoint at the `StructuredAssistantDecoder` call to +`GemmaToolCallParser.parse`. If release optimization prevents inspection, use +the smallest failure-only local probe that writes to a permission-restricted +private file. Do not add raw output to normal server logging. + +### 1.3 Classify the payload + +| Captured payload | Next branch | +| --- | --- | +| Valid native `call:name{...}` rejected | Parser or allowed-tool validation | +| Canonical JSON object | Template/model dialect branch | +| Native prefix with malformed/truncated body | Template, model, or numerical branch | +| Token IDs reconstruct differently by decoding path | Tokenizer/detokenizer branch | +| Payload differs with identical prompt IDs and greedy settings | Runtime nondeterminism branch | + +Do not proceed to a broad runtime matrix until this classification exists. + +## Phase 2: Minimize each reproducible case + +Minimize one dimension at a time while requiring the same parser cause and +payload shape: + +1. Binary-search prior conversation turns. +2. Binary-search the ordered tool list. +3. Remove unused schema properties and descriptions. +4. Reduce tool results and ordinary message content. +5. Check whether tool ordering, one tool name, one schema construct, or one + historical assistant/tool turn is necessary. + +After every reduction, run A-B-A: failing original, candidate reduction, +failing original. Stop minimizing when another reduction changes the failure +class or removes the first divergent token. + +Useful deliverables are: + +- the smallest private reproducer preserving the original failure; +- a sanitized public reproducer if its tokenization and behavior are identical; +- an explicit list of request features that are necessary and unnecessary. + +## Phase 3: Template and protocol comparison + +This phase is especially important for Case O because it fails after successful +tool turns. + +### 3.1 Freeze both templates + +Compare: + +- the pinned template embedded in the installed model; and +- a hash-pinned copy of Google's current canonical Gemma 4 template. + +Record both SHA-256 hashes and a focused diff covering assistant tool calls, +tool responses, turn closures, reasoning/history reinjection, and null schema +handling. Relevant upstream references: + +- +- +- + +Discussion reports are hypotheses, not proof for issue 84. + +### 3.2 Run the template A-B-A + +Render the same messages and tools out-of-process with each template. Preserve +the resulting prompt token IDs as private artifacts. Run: + +1. pinned template; +2. current canonical template; +3. pinned template again. + +Keep the model, runtime, and generation settings fixed. Do not modify the +installed `.gturbo` directory. If TurboFieldfare cannot accept explicit prompt +IDs through an existing validation surface, build the smallest non-production +harness only after authorization. + +Interpretation: + +- Only the canonical template succeeds: prompt framing is causal. +- Both templates generate the same malformed dialect: template revision is not + sufficient; compare against the model oracle. +- Only a historical tool turn is required: inspect its exact rendered closure + and the next model-turn opening. +- The template changes the output but neither result is valid: continue with + first-divergence oracle comparison rather than choosing the nicer output. + +### 3.3 Protocol parser comparison + +Feed the captured payload bytes, without model execution, to: + +- `GemmaToolCallParser`; +- a minimal parser implementing the pinned template's native syntax; and +- MLX-LM's Gemma tool parser where applicable. + +MLX-LM documents native Gemma tool-call support here: +. + +Agreement between parsers establishes payload validity, not model correctness. + +## Phase 4: Small runtime differential matrix + +Run this phase only if the payload is malformed under the pinned template and +the cause remains numerical, cache-related, or nondeterministic. Keep exact +request bytes and generation settings fixed. + +Start from baseline `B0`: + +```text +max-context=65536 +prompt-cache-mode=off +prefill=on +prefill-chunk-tokens=128 +expert-cache-slots=16 +expert-cache-policy=lfu +rdadvise=off +temperature=0 +``` + +Run only the first-level variants initially: + +| ID | Single change from B0 | Question answered | +| --- | --- | --- | +| B1 | `--prefill off` | Does chunked prefill change the first output token? | +| B2 | `--max-context 32768` | Is the failure dependent on the 64K allocation/path? | +| B3 | `--expert-cache-slots 32` | Does expert residency alter greedy output? | +| O1 | `--prompt-cache-mode single-prefix` | Does verified KV reuse change Case O? | + +Use B2 only when the full prompt plus fixed completion limit fits 32K. O1 must +replay the real multi-turn sequence and is not useful as a substitute for raw +payload capture in Case H. + +Escalate only the axis that changes the first divergent token: + +- If B1 differs, test chunk sizes 32, 64, and 128. +- If B2 differs, compare the first logits at identical prompt positions and + inspect position/RoPE and attention-length handling. +- If B3 differs, compare LFU and LRU, then expert loads and routed expert IDs. +- If O1 differs, compare each turn's rendered/effective prompt hashes, cached + token count, KV position, and first post-prefix logits. +- If identical inputs produce different greedy hashes, run three fresh-process + repetitions and three same-process repetitions to distinguish initialization + from mutable process state. + +The bounded diagnostics prove token lineage and accounting only. They do not +prove numerical K/V tensor equality. + +## Phase 5: Independent MLX-LM oracle + +Use the exact pinned checkpoint named in +[Implementation references](IMPLEMENTATION_REFERENCES.md). The checkpoint is +already present on the current investigation host; do not download or copy it. +At planning time, the active shell did not expose an `mlx_lm` runtime, so locate +an existing compatible environment or obtain authorization before installing +anything. + +Run TurboFieldfare and MLX-LM sequentially with: + +- identical explicit prompt token IDs; +- the same pinned weights, tokenizer, and template sidecars; +- greedy decoding; +- the same maximum generation count; +- cache reuse disabled; +- generated token IDs retained privately. + +Compare at every generated position: + +- chosen token ID; +- top-k token IDs and logits; +- top-1/top-2 margin; +- first position at which ranking or token choice differs. + +Interpretation: + +- Same malformed payload from both engines: model/template behavior, not a + TurboFieldfare-only numerical defect. +- MLX emits a valid call and TurboFieldfare diverges with a large logit margin: + investigate TurboFieldfare model math at the first divergent position. +- Different token choice with a near-zero margin: repeat and compare logits; + token equality alone is too strict for floating-point implementations. +- Logits agree through the payload but parsing differs: parser or decoding + defect. + +Do not compare free-form text alone. The oracle is useful only with exact prompt +IDs and token/logit evidence. + +## Phase 6: Focused hard tests + +Add durable tests only after the captured payload or first divergence identifies +the relevant boundary. + +### Parser and tokenizer branch + +- Round-trip native calls through template rendering, tokenization, + token-by-token structured decoding, and parsing. +- Preserve a sanitized minimal failing payload as a regression fixture. +- Test exact token-byte reconstruction separately from ordinary decoded text. +- Cover only observed edge classes: marker boundaries, whitespace, Unicode, + quoted or native object keys, nested values, and allowed tool identifiers. + +### Runtime branch + +- Add the smallest scalar/CPU or bounded-logit comparison that fails at the + first divergent layer or position. +- Avoid a full model-backed package test when an existing local reference test + can express the failing invariant. +- Require the test to fail before the fix and pass after it. + +Do not begin broad parser fuzzing, long soak runs, or performance profiling +without evidence that the corresponding subsystem is involved. + +## Phase 7: Evaluate fixes and PR #107 + +Evaluate a proposed change only after the root-cause branch is known: + +- If the parser rejects a valid supported representation, fix and test the + parser at that representation. +- If the pinned template frames multi-turn tool history incorrectly, update the + pinned template with exact before/after render fixtures. +- If TurboFieldfare logits diverge from the oracle, fix the earliest numerical + defect before adding grammar constraints. +- If both engines naturally emit malformed syntax, grammar-constrained decoding + is a mitigation. Report it as such unless the experiment also proves why the + unconstrained model changed dialect. + +PR #107 must still be checked for incomplete calls, UTF-8, size limits, unknown +tools, completion budgets, prompt-cache semantics, and fail-closed behavior. A +successful grammar-constrained run does not by itself establish root cause. + +## Execution order and stop conditions + +Run the investigation in this order: + +1. Acquire exact Case O and Case H requests. +2. Reproduce and capture private payload token IDs and bytes. +3. Classify and minimize each case. +4. Run the template A-B-A. +5. Run only the relevant first-level runtime variants. +6. Use MLX-LM at the first unresolved model-output boundary. +7. Add one focused regression test and evaluate the smallest fix. + +Stop early when an A-B-A result plus raw evidence establishes the root cause. +Do not complete the remaining matrix merely for coverage. + +Stop and report a blocker when: + +- the full request cannot be obtained and the public reproduction cannot be + recreated; +- preflight checks fail; +- supposedly identical runs have different prompt hashes; +- the private payload cannot be captured safely; +- the oracle would require another checkpoint download; +- a test requires simultaneous model processes. + +## Final report + +Create `docs/ISSUE_84_ROOT_CAUSE_RESULTS.md` only after experiments begin. It +should contain: + +1. commit, host, model revision, and exact commands; +2. protocol deviations and unavailable artifacts; +3. one compact run table; +4. the captured payload classification without private content; +5. first-divergence evidence; +6. separate conclusions for Case O and Case H; +7. confirmed root cause, probable contributors, and rejected hypotheses; +8. fix recommendation and assessment of PR #107; +9. remaining uncertainty. + +Do not publish private prompts, payloads, tool schemas, results, or reversible +token sequences in that report. diff --git a/docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md b/docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md new file mode 100644 index 00000000..7b2fc5e3 --- /dev/null +++ b/docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md @@ -0,0 +1,456 @@ +# Issue 84: Structured Tool-Generation Diagnostics + +## Goal + +Make failures like issue 84 diagnosable from one server log line, without +logging prompts, generated text, tool arguments, or reversible token +sequences. + +This change diagnoses the failure. It does not attempt to fix model behavior, +cache behavior, retry policy, or response fallback. + +## Confirmed behavior + +The implementation must be based on these established facts: + +1. `runRawCompletion` sets `.toolCalls` only when the model samples + `tokenizer.toolResponseID`. +2. That stop token is intercepted before + `StructuredAssistantDecoder.consume` receives it. +3. The guard in `ServerInference.swift` runs only after no decoder error was + captured, `decoder.finish()` succeeded, and zero complete calls were + decoded. +4. Therefore that guard means specifically that the model emitted + `<|tool_response>` without a preceding complete valid tool call. +5. `RawDecodeResult` already retains everything required for diagnostics: + the effective prompt and committed generated tokens in + `kvBackedTokenIDs`, the final sampled stop or failing token in + `uncommittedBoundaryTokenIDs`, and prompt, cache, completion, stop-reason, + and KV-position fields. + +No decode-loop collector is needed. + +## Scope + +Modify only: + +- `Sources/TurboFieldfareServer/Core/ServerInference.swift` +- `Tests/TurboFieldfareServer/OpenAIValidationTests.swift`, or one small + dedicated diagnostics test file if that is materially clearer + +Do not modify: + +- `RawCompletion.swift` +- `StructuredAssistantDecoder.swift` +- `GemmaToolCallParser.swift` +- `HTTPServer.swift` +- `ServerLog.swift` +- server arguments or environment handling +- prompt-cache logic +- documentation beyond this implementation plan + +The existing `ServerLog.failed` path already provides request-ID correlation +and logs `String(reflecting: error)`. Carry the diagnostic payload in the +thrown error and reuse that path. + +## 1. Add a dedicated structured-generation error + +Add internal types near the server inference types in `ServerInference.swift`. + +Suggested shape: + +```swift +enum StructuredOutputFailureKind: String, Sendable { + case decoderConsume = "decoder_consume" + case decoderFinish = "decoder_finish" + case orphanToolResponse = "orphan_tool_response" +} + +struct StructuredOutputFailureDiagnostics: Equatable, Sendable { + // Scalar fields and hashes. +} + +struct StructuredOutputFailure: Error, CustomDebugStringConvertible, Sendable { + let kind: StructuredOutputFailureKind + let cause: String + let diagnostics: StructuredOutputFailureDiagnostics + + var debugDescription: String { + // Stable, bounded, single-line key=value representation. + } +} +``` + +Keep these types internal. They are server implementation details, not API +types. + +`CustomDebugStringConvertible` is important because `ServerLog.failed` uses +`String(reflecting:)`. Add a test proving that reflection produces the +intended bounded log text. + +### Cause classification + +Do not store or reflect an arbitrary underlying error. Classify known parser +errors: + +| Existing error | Diagnostic cause | +| --- | --- | +| `.malformed` | `malformed` | +| `.unknownTool` | `unknown_tool` | +| `.oversized` | `oversized` | +| Anything unexpected | `unexpected` | +| Orphan-sentinel guard | `none` | + +Do not include the generated unknown tool name. Although request tool names +are validated, a model-generated identifier could otherwise make the log +unnecessarily large or expose internal names. + +## 2. Wrap all post-generation structured-output failures + +There are three distinct failure points after `runRawCompletion` returns. + +### A. Decoder failure during token consumption + +Replace: + +```swift +if let decodingError { throw decodingError } +``` + +with a `StructuredOutputFailure` using: + +- `kind = .decoderConsume` +- the classified parser cause +- diagnostics built from the completed `RawDecodeResult` + +This covers malformed arguments, invalid framing, duplicate starts, unmatched +ends, unknown tools, and oversized calls detected during streaming. + +Do not throw directly from the progress callback. The existing callback +behavior captures the error, requests generation stop, and throws after +`runRawCompletion` returns. That is what makes the final token evidence +available. + +### B. Decoder failure at `finish()` + +Wrap: + +```swift +try decoder?.finish() +``` + +in `do/catch` and throw: + +- `kind = .decoderFinish` +- the classified parser cause +- the same diagnostics + +This primarily identifies an unfinished `<|tool_call>` block at EOS, +end-of-turn, stop-string, or max-token termination. + +### C. Orphan tool-response sentinel + +Replace the current `GemmaToolCallParserError.malformed` throw with: + +- `kind = .orphanToolResponse` +- `cause = "none"` +- the same diagnostics + +This corrects the misleading classification without weakening fail-closed +behavior. + +### Preserve failure semantics + +The wrapper must remain an ordinary non-`ServerRequestError`, so HTTP behavior +remains unchanged: + +- non-streaming requests return HTTP 500 with the generic internal-error + envelope; +- streaming requests emit the existing SSE error and `[DONE]`; +- the prompt cache is invalidated; +- the runner is reset; +- no partial content is converted into a successful response. + +## 3. Reconstruct the exact generated token sequence + +Add one private or internal helper in `ServerInference.swift`. + +The generated sequence is: + +```swift +let committedGenerated = result.kvBackedTokenIDs.dropFirst(result.prefillTokens) +let generatedIDs = + Array(committedGenerated) + result.uncommittedBoundaryTokenIDs +``` + +Why this is correct: + +- `kvBackedTokenIDs` begins with the full effective prompt. +- Nonterminal generated tokens are appended after being accepted into KV. +- The terminal or failing token is retained in + `uncommittedBoundaryTokenIDs`. +- Therefore the concatenation includes the orphan `<|tool_response>` token + and parser-failing boundary tokens. + +Make reconstruction defensive: diagnostics must never crash while reporting +another failure. If `prefillTokens > kvBackedTokenIDs.count`, use a safe +bounded drop and report failed lineage invariants. + +Do not add a generated-token property to `RawDecodeResult`; reconstruction is +only needed on this server failure path. + +## 4. Diagnostic payload + +Emit a stable, single-line, failure-only summary. + +### Request and cache fields + +- `rendered_prompt_tokens`: `promptIDs.count` +- `effective_prompt_tokens`: `effectivePromptIDs.count` +- `result_prompt_tokens`: `result.prefillTokens` +- `cached_prompt_tokens`: `result.cachedPromptTokens` +- `computed_prefill_tokens`: `result.computedPrefillTokens` +- `completion_tokens`: `result.newTokens` +- `max_completion_tokens`: final `config.maxNewTokens` +- `raw_stop`: explicit mapping to `eos`, `end_of_turn`, `max_tokens`, + `stop_string`, or `tool_calls` +- `kv_position`: `result.kvPosition` +- `kv_backed_tokens`: `result.kvBackedTokenIDs.count` +- `boundary_tokens`: `result.uncommittedBoundaryTokenIDs.count` + +### Structured decoder fields + +- `decoded_calls`: `calls.count` +- `visible_bytes`: `content.utf8.count` +- `stop_string_matched`: `stopMatcher.isStopped` + +Use UTF-8 bytes rather than Swift character count. + +### Tool-marker fields + +Scan only the reconstructed generated sequence: + +- `tool_start_count` +- `tool_end_count` +- `tool_response_count` +- `tool_response_end_count` +- `last_tool_start_offset` +- `last_tool_end_offset` +- `last_tool_response_offset` +- `last_tool_response_end_offset` + +Offsets are zero-based within the generated sequence. Use `-1` when absent. + +Do not log arbitrary special-token IDs or a complete special-token trace. +Counts and final offsets are sufficient to distinguish: + +- no call framing followed by a tool response; +- an opened but unfinished call; +- an unmatched close; +- one valid call followed by malformed additional framing; +- repeated response markers. + +### Lineage invariants + +Report separate booleans so a false result is actionable: + +- `effective_count_matches_result`: + `effectivePromptIDs.count == result.prefillTokens` +- `effective_prefix_matches_kv`: enough KV-backed tokens exist and their + prompt prefix exactly equals `effectivePromptIDs` +- `kv_position_matches_history`: + `result.kvPosition == result.kvBackedTokenIDs.count` +- `completion_count_matches_history`: reconstructed generated count equals + `result.newTokens` +- `prefill_accounting_matches`: cached plus computed prefill equals total + prefill + +These checks are failure-only and bounded by the configured context size. + +## 5. Token hashes + +Compute three deterministic SHA-256 fingerprints: + +- `rendered_prompt_i32le_sha256` +- `effective_prompt_i32le_sha256` +- `generated_i32le_sha256` + +Serialization must be defined precisely: + +1. Treat each token as its `UInt32(bitPattern:)`. +2. Serialize four bytes in little-endian order. +3. Concatenate without delimiters. +4. SHA-256 the resulting bytes. +5. Emit lowercase hexadecimal. + +`CryptoKit` is already imported by `ServerInference.swift`; do not add a +dependency or a general hashing utility. + +These hashes allow later cache-on/cache-off comparison without sharing +content. They are fingerprints, not anonymization; do not describe them as +anonymous or secret-safe against dictionary attacks. + +Hashing happens only after a structured generation fails, so it does not +affect successful generation performance. + +## 6. Log format + +Keep one line and follow existing server logging conventions. + +Expected shape: + +```text +error=structured_output_failure kind=orphan_tool_response cause=none rendered_prompt_tokens=27513 effective_prompt_tokens=... cached_prompt_tokens=... computed_prefill_tokens=... completion_tokens=... max_completion_tokens=... raw_stop=tool_calls kv_position=... kv_backed_tokens=... boundary_tokens=1 decoded_calls=0 visible_bytes=0 stop_string_matched=false tool_start_count=0 tool_end_count=0 tool_response_count=1 tool_response_end_count=0 last_tool_start_offset=-1 last_tool_end_offset=-1 last_tool_response_offset=... last_tool_response_end_offset=-1 effective_count_matches_result=true effective_prefix_matches_kv=true kv_position_matches_history=true completion_count_matches_history=true prefill_accounting_matches=true rendered_prompt_i32le_sha256=... effective_prompt_i32le_sha256=... generated_i32le_sha256=... +``` + +Requirements: + +- no newlines; +- stable field order; +- no arrays; +- no request text; +- no decoded generation; +- no tool schemas, names, arguments, or results; +- no arbitrary error descriptions; +- bounded length independent of prompt or completion size. + +Do not change `ServerLog.failed`; its existing prefix supplies the timestamp, +request ID, HTTP status, and request phase. + +## 7. Focused test + +Add one synthetic diagnostics test using `RawDecodeResult`; no model is +required. + +Suggested scenario: + +1. Build a small effective prompt. +2. Put two ordinary generated tokens in `kvBackedTokenIDs`. +3. Put `tokenizer.toolResponseID` in + `uncommittedBoundaryTokenIDs`. +4. Set `reason = .toolCalls`, `newTokens = 3`, nonzero cached prompt tokens, + and consistent KV position and accounting. +5. Construct `.orphanToolResponse` diagnostics. +6. Assert that: + - reconstructed completion includes the boundary token; + - completion count is correct; + - tool-response count is one; + - tool start and end counts are zero; + - response offset is the final generated offset; + - all lineage invariants are true; + - hash output is lowercase 64-character hex; + - `String(reflecting: error)` contains the required fields; + - the reflected error contains no decoded prompt or content text and no + complete token arrays. + +Include a fixed hash vector for a tiny known `[Int32]` sequence so the `i32le` +serialization convention cannot silently change. + +Do not create model-backed server tests or mock `ServerModelSession`; that +would require unnecessary dependency injection for a pure diagnostic +transformation. + +## 8. Validation + +Mandatory checks: + +```bash +Scripts/test.sh --filter 'TurboFieldfareServerTests\.(StructuredOutputDiagnosticsTests|GemmaToolCallTests|ServerPromptCacheTests)' +swift build -c release --product TurboFieldfareServer +git diff --check +``` + +If the new test is added to an existing suite, adjust the filter accordingly. + +Also review the final diff for these properties: + +- only failure paths changed; +- successful completion logging is unaffected; +- no backend protocol signatures changed; +- no new CLI or environment controls exist; +- no prompt-cache behavior changed; +- HTTP and SSE envelopes remain unchanged; +- the existing `defer` still invalidates the cache and resets the runner on + every wrapped failure. + +No model run is required to validate this implementation. + +## 9. Reporter follow-up + +After the diagnostic patch is available, ask the issue 84 reporter to rerun +the same three-turn OpenCode flow and provide: + +- the complete single failure log line; +- server commit; +- whether the request was streaming; +- the unchanged launch command. + +Do not initially ask for the raw prompt or generated text. + +Interpret the result as follows: + +| Evidence | Likely meaning | +| --- | --- | +| `tool_start_count=0`, `tool_end_count=0`, `tool_response_count=1` | Confirmed orphan response sentinel | +| Start count greater than end count and `kind=decoder_finish` | Unfinished tool call | +| `kind=decoder_consume` with matching start and end counts | Malformed call body or invalid tool | +| Any false lineage invariant | Runtime or result-accounting defect; investigate before blaming model or cache | +| Cached tokens greater than zero | Cache path participated; this does not prove it caused divergence | +| Cache-off and cache-on produce identical generated hashes | Behavior is not caused by cache reuse | +| Only cache-on reproduces under deterministic generation | Investigate long-context continuation and KV parity | + +If cache causation remains plausible, perform a separate controlled +reproduction: + +1. Capture the exact sanitized request sequence. +2. Force deterministic generation with temperature `0`. +3. Run sequentially with `--prompt-cache-mode off`. +4. Run sequentially with `--prompt-cache-mode single-prefix`. +5. Follow all repository model-process and memory checks. +6. Compare counts, prompt fingerprints, generated fingerprints, and marker + positions. + +Do not run two model processes simultaneously. + +## Explicit non-goals + +Do not expand this work into: + +- returning raw generation as assistant content; +- retrying automatically; +- exposing hidden thought output; +- adding `--debug`, `--verbose`, or raw-dump flags; +- logging complete token IDs; +- changing `StopReason`; +- changing the decoder grammar; +- changing prompt-cache matching; +- adding a general logging framework; +- claiming the root cause is model drift or KV divergence before comparative + evidence exists. + +A raw-generation dump can be considered later only if the scalar diagnostics +reproduce but cannot distinguish the cause. It must be a separate, explicitly +sensitive opt-in change. + +## Acceptance criteria + +The work is complete when: + +1. The former orphan-sentinel guard throws `orphan_tool_response`, not + `GemmaToolCallParserError.malformed`. +2. All three structured-output failure phases carry the same bounded + diagnostic schema. +3. The existing HTTP logger produces one request-correlated line with actual + completion and cache counts. +4. The terminal boundary token is included in marker counts and the generated + hash. +5. No raw user, model, or tool content is logged. +6. Existing failure, cache invalidation, runner reset, HTTP 500, and SSE + behavior remain unchanged. +7. Focused tests and the release server build pass. + +Suggested single commit: + +```text +Diagnose malformed server tool generations +``` diff --git a/docs/OPTIMIZATION_JOURNEY.md b/docs/OPTIMIZATION_JOURNEY.md index 29626a4c..21d95793 100644 --- a/docs/OPTIMIZATION_JOURNEY.md +++ b/docs/OPTIMIZATION_JOURNEY.md @@ -201,16 +201,22 @@ K/V head across eight query heads, but the tiled kernel handled each query head separately. It launched eight threadgroups and read the same K/V data eight times. -TensorOps processes all eight heads at once, making attention 11x faster at -64K. +On Apple10, TensorOps processes all eight heads at once, making attention 11x +faster at 64K. -The full runtime kept much of that gain. On the same 32K input, prefill fell -from 491.09 to 204.29 seconds, a 2.404x speedup without increasing memory use. +The full runtime kept much of that gain. On the same Apple10 32K input, prefill +fell from 491.09 to 204.29 seconds, a 2.404x speedup without increasing memory +use. We nearly threw this result away because the new reduction order changed the final logits slightly. Better checks showed the differences were harmless. -TensorOps is now the Apple10 path. Earlier GPUs keep tiled attention. +Production now selects TensorOps by pipeline capability rather than GPU family. +The pipeline also compiles and dispatches on Apple8 M2, where it measured +9.027-9.294x faster than tiled attention in isolation and reduced a matched +6,784-token prefill from 419.469 to 243.100 seconds without increasing memory +use. Incompatible shapes and stacks that cannot build the pipeline use tiled +attention. ## Sampling removed repeated vocabulary scans diff --git a/docs/TERMINAL_3D_ENGINE_PLAN.md b/docs/TERMINAL_3D_ENGINE_PLAN.md new file mode 100644 index 00000000..766203d3 --- /dev/null +++ b/docs/TERMINAL_3D_ENGINE_PLAN.md @@ -0,0 +1,886 @@ +# Procedural Terminal 3D Engine Plan + +## Goal + +Build a procedural 3D endless runner rendered entirely with terminal symbols. +The engine and game are written in C17 with no runtime assets and no external +runtime dependencies. Geometry, animation, materials, physics, levels, and +rendering are all described in code. + +The first proof of concept is a spinning torus. It must be rendered as a normal +indexed triangle mesh through the same generic pipeline that later renders the +runner, track, and obstacles. The project must not contain a torus-specific +rendering path. + +## Product constraints + +- C17 engine and game. +- Command-line executable for macOS and Linux terminals. +- POSIX platform layer using `termios`, `poll`, `ioctl`, `clock_gettime`, and + `write`. +- No ncurses, Notcurses, SDL, OpenGL, Metal, Lua, ECS framework, or asset + loader in the initial game. +- No OBJ, glTF, textures, configuration files, or resource directory. +- All geometry and animation are procedural. +- Deterministic level generation from a numeric seed. +- No heap allocation during the steady-state frame loop. +- One buffered terminal write per full frame where practical. +- ASCII fallback when the terminal or font cannot display the selected Unicode + symbols safely. + +The intended technical description is: + +> A procedural 3D endless runner rendered entirely with terminal symbols. It is +> written in pure C with no dependencies and no assets; everything from +> geometry and animation to physics and levels is generated in code. + +## Development sources and single-file distribution + +Development must use normal, modular source files. The one-file version is a +generated release artifact, not the canonical source and not a file edited by +hand. This is an amalgamation build, similar to single-file distributions used +by established C projects. + +Canonical development layout: + +```text +src/ + main.c + math.c + math.h + memory.c + memory.h + mesh.c + mesh.h + render.c + render.h + glyph.c + glyph.h + terminal.c + terminal.h + world.c + world.h + game.c + game.h + +tests/ + test_math.c + test_mesh.c + test_render.c + test_glyph.c + test_world.c + +tools/ + amalgamate.py + +dist/ + term3d.c # generated; never edited manually +``` + +The initial torus milestone should use fewer modules if some of these files do +not yet have real content. Empty `world`, `game`, physics, or animation modules +must not be created in advance merely to reserve the names. + +Development build: + +```bash +cc -std=c17 -O3 -c src/math.c +cc -std=c17 -O3 -c src/mesh.c +cc -std=c17 -O3 -c src/render.c +cc -std=c17 -O3 -c src/glyph.c +cc -std=c17 -O3 -c src/terminal.c +cc -std=c17 -O3 -c src/main.c +cc math.o mesh.o render.o glyph.o terminal.o main.o -lm -o term3d +``` + +Release generation and build: + +```bash +python3 tools/amalgamate.py src/main.c --output dist/term3d.c +cc -std=c17 -O3 dist/term3d.c -lm -o term3d +``` + +The amalgamator recursively expands project-local quoted includes, includes +each project header or source once, preserves system includes, and emits `#line` +directives so diagnostics still name the canonical source file: + +```c +#line 1 "src/render.c" +``` + +Amalgamation-specific rules: + +- Prefix externally visible and internal file-scope names with `t3d_` and a + module name where useful. +- Do not define identically named `static` functions in separate `.c` files; + they collide when combined into one translation unit. +- Prefix project macros and `#undef` temporary implementation macros. +- Do not rely on include order or on macros leaking between modules. +- Never edit `dist/term3d.c` directly. +- Build and test both modular and amalgamated forms in CI. +- Compare a deterministic headless frame checksum from both builds. + +Suggested targets: + +```text +make modular development build +make test modular tests +make amalgamate regenerate dist/term3d.c +make dist-test compile and test the generated source +make release produce dist/term3d.c and the release executable +``` + +## Architecture + +```text +fixed-step game update + |-- input actions + |-- procedural animation + |-- simple physics and collision + `-- deterministic track generation + | + v + transforms and meshes + | + v + generic indexed-mesh renderer + transform -> clip -> project + -> cull -> triangle rasterize + | + v + terminal subcell sample target + inverse depth + shade + colour + | + v + glyph resolver + ASCII / shade / half / quadrant / Braille + | + v + packed terminal cells + | + v + ANSI presenter -> buffered write() +``` + +The renderer consumes immutable meshes, transforms, cameras, and materials. It +must not know whether a transform came from animation, physics, procedural +generation, or input. The terminal presenter consumes resolved cells and must +not know anything about triangles or 3D math. + +## Core data + +```c +typedef struct { float x, y; } T3D_Vec2; +typedef struct { float x, y, z; } T3D_Vec3; +typedef struct { float x, y, z, w; } T3D_Vec4; +typedef struct { float m[16]; } T3D_Mat4; + +typedef struct { + T3D_Vec3 position; + T3D_Vec3 normal; +} T3D_Vertex; + +typedef struct { + T3D_Vertex *vertices; + uint32_t *indices; + uint32_t vertex_count; + uint32_t index_count; +} T3D_Mesh; + +typedef struct { + T3D_Vec3 position; + T3D_Vec3 rotation; + T3D_Vec3 scale; +} T3D_Transform; + +typedef struct { + const T3D_Mesh *mesh; + T3D_Mat4 model; + uint32_t material_id; +} T3D_RenderInstance; +``` + +Euler rotation is sufficient for the first spinning torus and procedural +runner limbs. Add quaternions only when composed rotations or interpolation +make them necessary. + +Transformed vertices preserve the fields needed for clipping and later +perspective-correct interpolation: + +```c +typedef struct { + T3D_Vec4 clip_position; + float shade; +} T3D_ClipVertex; + +typedef struct { + float x; + float y; + float inv_w; + float shade_over_w; +} T3D_ScreenVertex; +``` + +## Procedural torus + +Generate the indexed torus once during initialization. Trigonometry must not be +performed once per torus vertex per frame. + +```c +static T3D_Vertex t3d_make_torus_vertex( + float major_radius, + float minor_radius, + float u, + float v) +{ + const float cu = cosf(u); + const float su = sinf(u); + const float cv = cosf(v); + const float sv = sinf(v); + const float ring = major_radius + minor_radius * cv; + + return (T3D_Vertex) { + .position = { + ring * cu, + minor_radius * sv, + ring * su + }, + .normal = { + cv * cu, + sv, + cv * su + } + }; +} +``` + +Connect adjacent rings with wrapped indexed triangles: + +```c +for (uint32_t i = 0; i < major_segments; ++i) { + for (uint32_t j = 0; j < minor_segments; ++j) { + const uint32_t i1 = (i + 1) % major_segments; + const uint32_t j1 = (j + 1) % minor_segments; + + const uint32_t a = i * minor_segments + j; + const uint32_t b = i1 * minor_segments + j; + const uint32_t c = i1 * minor_segments + j1; + const uint32_t d = i * minor_segments + j1; + + *index++ = a; *index++ = b; *index++ = c; + *index++ = a; *index++ = c; *index++ = d; + } +} +``` + +Start with `64 x 24` segments: 1,536 vertices and 3,072 triangles. The same +renderer must also draw a procedurally generated cube before the torus +milestone is accepted; this proves that the implementation is a generic +renderer rather than a disguised donut algorithm. + +## Symbol-native sample target + +Terminal glyphs such as quadrants and Braille encode multiple spatial samples +inside one cell. The renderer therefore writes depth, coverage, shade, and +colour into a small subcell target. These samples exist only to resolve one +terminal glyph; they are not a separate image or texture pipeline. + +```c +typedef struct { + uint16_t cell_cols; + uint16_t cell_rows; + uint8_t samples_x; + uint8_t samples_y; + uint16_t width; + uint16_t height; + + float *inv_depth; + uint8_t *shade; + uint32_t *colour; +} T3D_SampleTarget; +``` + +Allocate capacity for the largest built-in mode, Braille at `2 x 4`, once. +Less detailed modes use smaller active dimensions without reallocating. + +At `120 x 40` terminal cells, Braille mode contains only 38,400 samples, so a +single-threaded CPU rasterizer is sufficient until profiling proves otherwise. + +## Rendering pipeline + +For every frame: + +1. Clear the active inverse-depth and shading arrays. +2. Build model, view, and projection matrices. +3. Transform each unique mesh vertex once. +4. Transform its normal and calculate directional lighting. +5. Assemble indexed triangles. +6. Clip triangles against the near plane. +7. Perform perspective division. +8. Correct projection for terminal-cell aspect ratio. +9. Cull back-facing or degenerate triangles. +10. Rasterize with incremental edge functions. +11. Interpolate inverse depth and shade. +12. Depth-test every active subcell sample. +13. Resolve samples into packed terminal cells. +14. Encode either a full frame or changed runs into the output arena. +15. Present the completed byte stream with a partial-write-safe loop. + +Initial lighting: + +```c +float diffuse = fmaxf(0.0f, t3d_dot3(normal, light_direction)); +float shade = 0.15f + 0.85f * diffuse; +``` + +Use inverse depth so zero represents an empty sample and larger values are +closer: + +```c +if (inv_depth > target->inv_depth[index]) { + target->inv_depth[index] = inv_depth; + target->shade[index] = t3d_quantize_shade(shade); + target->colour[index] = colour; +} +``` + +Rasterize with edge functions and a consistent top-left fill rule: + +```c +static inline int64_t t3d_edge( + int32_t ax, int32_t ay, + int32_t bx, int32_t by, + int32_t px, int32_t py) +{ + return (int64_t)(px - ax) * (by - ay) + - (int64_t)(py - ay) * (bx - ax); +} +``` + +Transform and clip in floating point, then use a fixed-point screen coordinate +representation for the inner raster loop. Once the initial edge values are +known, advance them across rows with additions rather than recalculating the +full expression for every sample. + +The first torus may be placed wholly in front of the near plane, but near-plane +clipping is required before movable cameras or arbitrary procedural scenes. +Do not clamp vertices to the near plane. Full six-plane homogeneous clipping +can follow when off-screen meshes make it necessary. + +## Glyph modes + +Built-in modes: + +| Mode | Samples per cell | Symbols | +|---|---:|---| +| ASCII | `1 x 1` | ` .:-=+*#%@` | +| Dense ASCII | `1 x 1` | `.,-~:;=!*#$@` | +| Shade | `1 x 1` | ` `, `░`, `▒`, `▓`, `█` | +| Half block | `1 x 2` | ` `, `▀`, `▄`, `█` | +| Quadrant | `2 x 2` | sixteen block masks | +| Braille | `2 x 4` | U+2800 through U+28FF | + +Braille bit order is not row-major: + +```c +static const uint8_t t3d_braille_bit[4][2] = { + { 1u << 0, 1u << 3 }, + { 1u << 1, 1u << 4 }, + { 1u << 2, 1u << 5 }, + { 1u << 6, 1u << 7 } +}; +``` + +Precompute all built-in glyphs as UTF-8 during initialization. Braille becomes +a direct table lookup: + +```c +glyph = braille_utf8[coverage_mask]; +``` + +Call `setlocale(LC_CTYPE, "")`, validate custom ramp code points with +`wcwidth() == 1`, and fall back to ASCII for unsupported locales or glyphs. +Reject combining marks, variation selectors, zero-width joiners, and emoji in +custom ramps. + +## Terminal cells and presentation + +Resolve the sample target into packed logical cells before producing terminal +bytes. A 64-bit cell should contain a glyph-table index, foreground colour, +background colour, and flags. Equality then requires one integer comparison. + +Maintain `next` and `shown` cell grids and support: + +```text +--present full +--present diff +--present auto +``` + +- `full` homes the cursor and encodes the complete frame. +- `diff` groups adjacent changed cells into horizontal runs and emits one cursor + movement per run. +- `auto` estimates both byte costs and emits the smaller representation. + +Do not assume differential output is faster. A rotating object or moving camera +may change enough cells that a complete frame is smaller than many cursor +commands. + +Never use `printf`, `putchar`, `fflush`, `snprintf`, or dynamic string growth in +the cell loop. Build a contiguous byte stream and handle partial writes: + +```c +static bool t3d_write_all(int fd, const void *data, size_t size) +{ + const uint8_t *p = data; + + while (size != 0) { + const ssize_t n = write(fd, p, size); + + if (n > 0) { + p += (size_t)n; + size -= (size_t)n; + } else if (n < 0 && errno == EINTR) { + continue; + } else { + return false; + } + } + + return true; +} +``` + +The terminal backend must: + +- Verify that input and output are TTYs for interactive mode. +- Save and restore the exact original terminal attributes. +- Use the alternate screen and hide the cursor. +- Read dimensions with `ioctl(TIOCGWINSZ)`. +- Use `poll` for input and frame waiting. +- Let signal handlers set only `volatile sig_atomic_t` flags. +- Handle `SIGWINCH` in the main loop. +- Restore terminal state after normal exit, Ctrl-C, termination, or errors. +- Avoid writing a newline after the final row. +- Avoid accidental scrolling from the bottom-right cell. + +## Memory policy + +Use one live backing allocation where practical. Divide it into aligned regions +for long-lived and frame buffers: + +```c +size_t bytes = + vertex_bytes + + index_bytes + + transformed_bytes + + depth_bytes + + shade_bytes + + colour_bytes + + cell_bytes * 2 + + output_bytes; + +uint8_t *memory = malloc(bytes); +uint8_t *cursor = memory; + +vertices = t3d_take(&cursor, vertex_bytes, _Alignof(T3D_Vertex)); +indices = t3d_take(&cursor, index_bytes, _Alignof(uint32_t)); +depth = t3d_take(&cursor, depth_bytes, _Alignof(float)); +cells_a = t3d_take(&cursor, cell_bytes, _Alignof(uint64_t)); +cells_b = t3d_take(&cursor, cell_bytes, _Alignof(uint64_t)); +output = t3d_take(&cursor, output_bytes, 1); +``` + +Allowed allocations: + +- Initial backing block. +- A complete replacement block after terminal resize. + +Forbidden during steady-state frames: + +- `malloc` +- `calloc` +- `realloc` +- `free` +- growing containers or strings + +On resize, allocate the entire replacement first, swap only after success, and +then free the old block. Keep the old renderer alive if replacement allocation +fails. A debug allocation counter must verify zero frame-loop allocations. + +## Fixed update and procedural animation + +Use a fixed simulation timestep independent of terminal presentation: + +```c +const double fixed_dt = 1.0 / 60.0; +double accumulator = 0.0; +double previous = t3d_monotonic_seconds(); + +while (!quit_requested) { + const double now = t3d_monotonic_seconds(); + double elapsed = now - previous; + previous = now; + + if (elapsed > 0.25) + elapsed = 0.25; + + accumulator += elapsed; + t3d_poll_input(&input); + + while (accumulator >= fixed_dt) { + t3d_game_update(&game, (float)fixed_dt); + accumulator -= fixed_dt; + } + + t3d_render_scene(&renderer, &game.scene); + t3d_resolve_glyphs(&renderer, glyph_mode); + t3d_present(&terminal, renderer.cells); + t3d_wait_until_next_frame(); +} +``` + +Advance an absolute monotonic deadline rather than sleeping for a complete frame +duration after rendering. + +The torus animation is initially just code: + +```c +demo.rotation.x += 0.7f * dt; +demo.rotation.y += 1.1f * dt; +``` + +## Procedural geometry and character + +Required code-generated primitives: + +```c +T3D_Mesh t3d_make_cube(...); +T3D_Mesh t3d_make_box(...); +T3D_Mesh t3d_make_torus(...); +T3D_Mesh t3d_make_cylinder(...); +T3D_Mesh t3d_make_sphere(...); +T3D_Mesh t3d_make_capsule(...); +T3D_Mesh t3d_make_ramp(...); +T3D_Mesh t3d_make_arch(...); +T3D_Mesh t3d_make_track(...); +``` + +Assemble the runner from primitive instances rather than creating a single +special mesh: + +```c +typedef struct { + T3D_Transform body; + T3D_Transform head; + T3D_Transform arm_l; + T3D_Transform arm_r; + T3D_Transform leg_l; + T3D_Transform leg_r; +} T3D_RunnerPose; +``` + +Initial running animation: + +```c +const float phase = run_time * run_speed; + +pose.arm_l.rotation.x = sinf(phase) * 0.8f; +pose.arm_r.rotation.x = -sinf(phase) * 0.8f; +pose.leg_l.rotation.x = -sinf(phase) * 0.9f; +pose.leg_r.rotation.x = sinf(phase) * 0.9f; +pose.body.position.y = fabsf(sinf(phase * 2.0f)) * 0.04f; +``` + +Game animation states: + +```text +idle +running +jumping +falling +sliding +crashed +``` + +Each state is a small C function that calculates a pose. Do not add a skeletal +animation framework unless procedural poses become insufficient. + +## Deterministic endless track + +Use a small deterministic PRNG: + +```c +typedef struct { + uint32_t state; +} T3D_Rng; + +static uint32_t t3d_rng_next(T3D_Rng *rng) +{ + uint32_t x = rng->state; + x ^= x << 13; + x ^= x >> 17; + x ^= x << 5; + return rng->state = x; +} +``` + +Generate track segments ahead of the camera and recycle segments that pass +behind it: + +```c +typedef enum { + T3D_SEGMENT_EMPTY, + T3D_SEGMENT_BARRIER, + T3D_SEGMENT_GAP, + T3D_SEGMENT_ARCH, + T3D_SEGMENT_COINS, + T3D_SEGMENT_MOVING_OBSTACLE +} T3D_SegmentType; +``` + +The same seed and input sequence must generate the same game state, level, and +score. This enables deterministic tests, replays, and shareable challenges +without storing level files. + +## Minimal physics + +Implement only the behavior required by a lane-based endless runner: + +```c +typedef struct { + T3D_Vec3 position; + T3D_Vec3 velocity; + int lane; + bool on_ground; +} T3D_RunnerBody; +``` + +Required behavior: + +- gravity +- jump impulse +- smooth lane interpolation +- ground collision +- player capsule versus obstacle box collision +- trigger volumes for collectibles +- gradual forward-speed increase + +For tens of active objects, a direct pair scan is acceptable. Do not implement +torque, joints, stacked rigid bodies, arbitrary convex collision, or a general +physics engine before the game demonstrates a need. + +## Fast inverse square root experiment + +Keep the Quake-style fast inverse square root as an optional implementation and +benchmark, not as the default contract: + +```c +#ifdef T3D_QUAKE_RSQRT +#define t3d_rsqrt(x) t3d_fast_rsqrt(x) +#else +#define t3d_rsqrt(x) (1.0f / sqrtf(x)) +#endif +``` + +Procedural primitives should generate analytic unit normals, rotation preserves +their length, and fixed directions are normalized once. The engine should avoid +normalization rather than force the bit hack into the frame loop. Enable it by +default only if a real target benchmark shows an improvement without unacceptable +error. + +## CLI + +```text +term3d + --glyph ascii|dense|shade|half|quadrant|braille + --ramp STRING + --colour mono|16|256|truecolor + --present full|diff|auto + --fps N + --size COLSxROWS + --aspect RATIO + --segments MAJORxMINOR + --seed N + --frames N + --headless + --benchmark + --stats + --dump-frame PATH + --glyph-test +``` + +Initial controls: + +```text +q or Esc quit +1 through 5 select glyph mode +c cycle colour mode +Space pause +Arrow keys rotate while the torus demo is paused +``` + +Later game controls should use discrete left, right, jump, and slide actions. +Traditional terminal input does not reliably report key releases, so game +movement must not require precise press/release state. + +## Milestones + +### 0. Terminal and glyph probe + +Deliver terminal entry/restoration, resize, raw input, UTF-8 tables, a glyph +test grid, and one full-frame buffered write. + +Gate: + +- Normal exit and Ctrl-C restore the terminal. +- Every selected glyph occupies one column. +- ASCII fallback works. +- Sanitizers find no error across small and large terminal sizes. + +### 1. Generic torus renderer + +Deliver math, generic indexed meshes, procedural torus and cube, camera, +backface culling, fixed-point edge rasterization, inverse depth, Lambert +lighting, ASCII/shade modes, and fixed-step rotation. + +Gate: + +- Correct occlusion and no cracks between adjacent triangles. +- No torus-specific branch in the renderer. +- The same path draws a cube. +- Zero steady-state allocations. +- A fixed headless frame produces a stable logical-cell checksum. + +### 2. Symbol-native rendering + +Deliver half-block, quadrant, and Braille resolution, ordered dithering, +runtime mode switching, Unicode fallback, and aspect correction. + +Gate: + +- All 16 quadrant and all 256 Braille masks map correctly. +- Depth is independent for every active subcell. +- Switching modes or resizing leaves no stale samples. + +### 3. Fast presenter + +Deliver packed front/back cells, full and changed-run encoders, automatic byte +cost selection, colour-state caching, and presenter telemetry. + +Gate: + +- No per-cell stdio call. +- Normally one buffered `write` per full frame. +- Interrupted and partial writes are correct. +- Full and differential presentation are chosen from measurements. + +### 4. Robust procedural scene + +Deliver near-plane clipping, multiple instances, movable camera, perspective- +correct attributes, frustum rejection, and the complete primitive set. + +Gate: + +- Triangles crossing the camera plane do not explode. +- Opaque output does not depend on triangle submission order. +- Random off-screen triangles never write outside the buffers. + +### 5. Procedural runner + +Deliver the primitive-composed runner, run/jump/slide poses, lane movement, +simple collision, score, deterministic track segments, and seeded replay. + +Gate: + +- Recorded input reproduces identical state and score. +- Rendering contains no game or physics behavior. +- No external file is required to start a complete game. +- The torus remains as a regression/demo mode. + +### 6. Amalgamated release + +Deliver the generator, checked-in `dist/term3d.c`, modular and amalgamated CI +builds, and a one-command user build. + +Gate: + +- Modular and amalgamated builds pass the same tests. +- Both builds produce the same deterministic headless checksum. +- `dist/term3d.c` contains no unresolved project-local include. +- `cc -std=c17 -O3 dist/term3d.c -lm -o term3d` succeeds on supported systems. + +## Validation and performance reporting + +Measure independently: + +```text +simulation time +vertex-transform time +clipping time +triangle-raster time +glyph-resolution time +ANSI-encoding time +terminal-write time +bytes per frame +changed cells and runs +missed frame deadlines +allocations per frame +``` + +Required benchmark modes: + +```bash +term3d --headless --benchmark --frames 10000 +term3d --present full --frames 1000 --stats +term3d --present diff --frames 1000 --stats +term3d --present auto --frames 1000 --stats +``` + +Suggested builds: + +```make +CFLAGS_DEBUG = -std=c17 -O0 -g3 -Wall -Wextra -Wshadow -Wconversion \ + -fsanitize=address,undefined +CFLAGS_RELEASE = -std=c17 -O3 -DNDEBUG -flto -Wall -Wextra +LDLIBS = -lm +``` + +Use `-march=native` only for local benchmark builds. Do not enable +`-ffast-math`, add SIMD intrinsics, or add raster threads until profiling shows +that scalar rendering rather than terminal output is the bottleneck. + +## Final acceptance criteria + +```text +canonical development sources: modular C files +release source: one generated C file +external runtime dependencies: zero +runtime assets: zero +procedural geometry: all +procedural animation: all +procedural levels: all +steady-state allocations: zero +normal full-frame writes: one +deterministic seed and replay: supported +headless benchmark: supported +``` + +## Explicit non-goals until proven necessary + +- Runtime asset loading. +- Textures or conventional pixel output. +- General ECS framework. +- Scene graph. +- Lua or another scripting VM. +- Skeletal animation framework. +- General rigid-body physics. +- Multithreaded or SIMD rasterization. +- GPU renderer. +- Windows terminal backend. +- Custom build system. + +The plan intentionally keeps reusable boundaries around rendering, glyph +resolution, terminal presentation, and fixed-step game updates while rejecting +speculative subsystems. Future work should add complexity only when a measured +game requirement crosses the current design's stated ceiling. diff --git a/docs/experiments/EXPERIMENT_INVENTORY.md b/docs/experiments/EXPERIMENT_INVENTORY.md index 634baaf5..de8d741e 100644 --- a/docs/experiments/EXPERIMENT_INVENTORY.md +++ b/docs/experiments/EXPERIMENT_INVENTORY.md @@ -162,7 +162,7 @@ resident set size; and **NLL** is negative log-likelihood. See | [PF-14](summaries/06-prefill.md#pf-14) — QMM TG reuse | Families +3.2-9.7%; current opportunity about 0.41%. | Rejected. | | [PF-15](summaries/06-prefill.md#pf-15) — Batched routed MoE | Isolated +30.91%; balanced end to end about +2%. | Reversed rejection; production. | | [PF-16](summaries/06-prefill.md#pf-16) — Long endpoint gate | Delta-NLL +0.002588; top-1 16/16; RSS 888.3 MiB. | Production; validation result. | -| [PF-17](summaries/06-prefill.md#pf-17) — Apple10 TensorOps full attention | Isolated 11.24x at 16K and 11.63x at 64K; 32K end to end 2.404x. | Production on Apple10; tiled fallback elsewhere. | +| [PF-17](summaries/06-prefill.md#pf-17) — TensorOps full attention | Apple10: isolated 11.24x at 16K and 11.63x at 64K; 32K end to end 2.404x. Apple8 M2: isolated 9.027-9.294x; 6,784-token prefill 1.726x. | Production when the pipeline builds; Apple8 M2 verified; tiled fallback when unavailable or incompatible. | ### Fusions, head, and orchestration diff --git a/docs/experiments/summaries/06-prefill.md b/docs/experiments/summaries/06-prefill.md index 18521883..8f2ecc3c 100644 --- a/docs/experiments/summaries/06-prefill.md +++ b/docs/experiments/summaries/06-prefill.md @@ -13,7 +13,7 @@ failed the M2 long-row gate. | Current result | Disposition | | --- | --- | | Chunk 128, staged affine MPP, and batched routed MoE | Production | -| Apple10 TensorOps full attention | Production on Apple10; tiled fallback elsewhere | +| TensorOps full attention | Production when the pipeline builds; Apple8 M2 verified; tiled fallback when unavailable or incompatible | | Shared/fetch overlap v3 | Rejected and removed | | Shared INT8 QMM, deeper lookahead, and argument-buffer rings | Rejected on M2 | @@ -268,7 +268,7 @@ failed the M2 long-row gate. separate a local tie from a systematic quality change. -### PF-17: Apple10 TensorOps full-prefill attention +### PF-17: TensorOps full-prefill attention - **Hypothesis:** One threadgroup could process all eight Q heads in a full attention GQA group, reuse K/V reads, and map QK/PV onto cooperative matrix @@ -279,13 +279,18 @@ failed the M2 long-row gate. Same-input 32K prefill fell from 491.09 to 204.29 seconds, a 2.404x end-to-end speedup, with identical post-prefill RSS. Direct attention reference checks passed through 64K, and frozen MLX-relative endpoints - matched top-1 at 8K/16K/32K/64K. + matched top-1 at 8K/16K/32K/64K. On Apple8 M2, the same pipeline measured + 9.027-9.294x faster than tiled attention in isolation and reduced a matched + 6,784-token prefill from 419.469 to 243.100 seconds without increasing the + measured footprint. The frozen 8K MLX-relative endpoint selected the + reference top-1. - **What changed the conclusion:** Exact production-logit identity falsely rejected a valid floating-point reduction order. Direct attention error and independent MLX quality gates isolated top-k routing amplification instead of a shader semantic defect. -- **Final disposition:** Production on Apple10; automatic causal-tiled fallback - on earlier GPU families and a named rollback remain. +- **Final disposition:** Production when the MSL 4 TensorOps pipeline builds; + Apple8 M2 is verified. Incompatible shapes and pipeline-build failures use + the automatic causal-tiled fallback. M3 and M4 remain unmeasured. - **Lesson:** Reordered floating-point kernels need a direct numerical oracle plus model-quality gates, not identity with one reduction order. diff --git a/docs/issue84_simulate_payloads.py b/docs/issue84_simulate_payloads.py new file mode 100644 index 00000000..312a41f9 --- /dev/null +++ b/docs/issue84_simulate_payloads.py @@ -0,0 +1,305 @@ +#!/usr/bin/env python3 +"""Issue #84 payload simulation. + +Faithful Python port of GemmaToolCallParser (as of acefaf1/main, bugs +included) + the real gemma4 tokenizer. Sweeps candidate payloads a drifting +model might emit between <|tool_call> and and reports which +match the observed diagnostics: exactly 14 payload tokens and a `malformed` +classification (not unknown_tool / not ok). +""" +import json +import re +from tokenizers import Tokenizer + +TOK = Tokenizer.from_file( + "/Users/andreymikhaylov/development/turbo-fieldfare/scratch/gemma4.gturbo/tokenizer/tokenizer.json") + +NUMBER_RE = re.compile(r'^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?$') + + +class Malformed(Exception): + pass + + +class UnknownTool(Exception): + pass + + +class Parser: + """Port of the private Parser struct in GemmaToolCallParser.swift. + + Deliberately reproduces both known bugs: + - identifier() stops at '-' and '.' (letters/digits/_ only) + - jsonString() calls take(), whose skipWhitespace eats ws inside strings + """ + + def __init__(self, text): + self.c = list(text) + self.i = 0 + + def at_end(self): + return self.i == len(self.c) + + def skip_ws(self): + while self.i < len(self.c) and self.c[self.i].isspace(): + self.i += 1 + + def consume(self, literal): + self.skip_ws() + n = len(literal) + if self.c[self.i:self.i + n] != list(literal): + raise Malformed(f"expected {literal!r} at {self.i}") + self.i += n + + def identifier(self): + self.skip_ws() + start = self.i + while self.i < len(self.c): + ch = self.c[self.i] + if not (ch.isalpha() or ch.isdigit() or ch == '_'): + break + self.i += 1 + if self.i == start: + raise Malformed("empty identifier") + return ''.join(self.c[start:self.i]) + + def starts(self, literal): + return self.c[self.i:self.i + len(literal)] == list(literal) + + def take(self, ch): + self.skip_ws() + if self.i < len(self.c) and self.c[self.i] == ch: + self.i += 1 + return True + return False + + def take_word(self, w): + if self.starts(w): + self.i += len(w) + return True + return False + + def object(self): + self.consume('{') + result = {} + self.skip_ws() + if self.take('}'): + return result + while True: + key = self.object_key() + self.consume(':') + result[key] = self.value() + self.skip_ws() + if self.take('}'): + return result + self.consume(',') + + def object_key(self): + self.skip_ws() + start = self.i + while self.i < len(self.c): + ch = self.c[self.i] + if not (ch.isalpha() or ch.isdigit() or ch in '_-.$'): + break + self.i += 1 + if self.i == start: + raise Malformed("empty object key") + return ''.join(self.c[start:self.i]) + + def value(self): + self.skip_ws() + if self.starts('<|"|>'): + return self.gemma_string() + if self.starts('"'): + return self.json_string() + if self.starts('{'): + return self.object() + if self.starts('['): + return self.array() + if self.take_word('true'): + return True + if self.take_word('false'): + return False + if self.take_word('null'): + return None + return self.number() + + def array(self): + self.consume('[') + result = [] + self.skip_ws() + if self.take(']'): + return result + while True: + result.append(self.value()) + self.skip_ws() + if self.take(']'): + return result + self.consume(',') + + def gemma_string(self): + self.consume('<|"|>') + out = '' + while not self.at_end(): + if self.starts('<|"|>'): + self.consume('<|"|>') + return out + if self.c[self.i] == '\\' and self.i + 1 < len(self.c): + self.i += 1 + out += self.escaped_fragment() + else: + out += self.c[self.i] + self.i += 1 + raise Malformed("unterminated gemma string") + + def json_string(self): + # BUG-FAITHFUL: take() skips whitespace each iteration, so spaces, + # tabs and newlines inside the string are silently dropped. + self.consume('"') + out = '' + while not self.at_end(): + if self.take('"'): + return out + if self.take('\\'): + out += self.escaped_fragment() + else: + out += self.c[self.i] + self.i += 1 + raise Malformed("unterminated json string") + + def escaped_fragment(self): + if self.i >= len(self.c): + raise Malformed("dangling escape") + e = self.c[self.i] + self.i += 1 + simple = {'"': '"', '\\': '\\', '/': '/', 'b': '\b', 'f': '\f', + 'n': '\n', 'r': '\r', 't': '\t'} + if e in simple: + return simple[e] + if e == 'u': + first = self.unicode_unit() + if 0xD800 <= first <= 0xDBFF: + if not (self.c[self.i:self.i + 2] == ['\\', 'u']): + raise Malformed("bad surrogate pair") + self.i += 2 + second = self.unicode_unit() + if not (0xDC00 <= second <= 0xDFFF): + raise Malformed("bad low surrogate") + scalar = 0x10000 + ((first - 0xD800) << 10) + (second - 0xDC00) + else: + if 0xDC00 <= first <= 0xDFFF: + raise Malformed("lone low surrogate") + scalar = first + return chr(scalar) + raise Malformed(f"bad escape {e!r}") + + def unicode_unit(self): + if self.i + 4 > len(self.c): + raise Malformed("short \\u") + s = ''.join(self.c[self.i:self.i + 4]) + try: + v = int(s, 16) + except ValueError: + raise Malformed("bad hex") + self.i += 4 + return v + + def number(self): + start = self.i + while self.i < len(self.c) and self.c[self.i] in '-+0123456789.eE': + self.i += 1 + if self.i == start: + raise Malformed("expected value") + lit = ''.join(self.c[start:self.i]) + if not NUMBER_RE.match(lit): + raise Malformed(f"bad number {lit!r}") + return float(lit) + + +def parse(text, allowed): + if len(text.encode()) > 256 * 1024: + return "oversized", None + p = Parser(text) + try: + p.consume('call:') + name = p.identifier() + if name not in allowed: + raise UnknownTool(name) + args = p.object() + p.skip_ws() + if not p.at_end(): + raise Malformed("trailing content") + return "ok", (name, args) + except UnknownTool as e: + return "unknown_tool", str(e) + except Malformed as e: + return "malformed", str(e) + except IndexError: + return "malformed", "index" + + +def ntok(text): + return len(TOK.encode(text, add_special_tokens=False).ids) + + +# --- Candidate sweep ------------------------------------------------------- +# Generic agent-tool names of varying lengths; the real Hermes names are +# unknown, so treat counts as a band, not an exact match. +NAMES = ["search", "read_file", "web_search", "list_files", "get_weather", + "execute_command", "todo-write", "browser.open", "final_answer"] +ALLOWED = set(NAMES) + +candidates = [] + +for name in NAMES: + # 1. Canonical OpenAI JSON drift + candidates += [ + ('canonical json, empty args', f'{{"name": "{name}", "arguments": {{}}}}'), + ('canonical json, compact', f'{{"name":"{name}","arguments":{{}}}}'), + ('canonical json, str args', f'{{"name": "{name}", "arguments": "{{}}"}}'), + ('canonical json, one arg', f'{{"name": "{name}", "arguments": {{"query": "x"}}}}'), + ('canonical json, tool key', f'{{"tool": "{name}", "args": {{}}}}'), + ('bare args object', '{"query": "weather today"}'), + # 2. Native dialect, near misses + ('native, ok empty', f'call:{name}{{}}'), + ('native, ok gemma str', f'call:{name}{{query:<|"|>x<|"|>}}'), + ('native, json-quoted key', f'call:{name}{{"query":"x"}}'), + ('native, capital C', f'Call:{name}{{}}'), + ('native, missing colon', f'call {name}{{}}'), + ('native, paren args', f'call:{name}("x")'), + ('native, single quotes', f"call:{name}{{query:'x'}}"), + ('native, trailing text', f'call:{name}{{}} done'), + ('native, unquoted value', f'call:{name}{{query:x}}'), + ('native, leading-zero num', f'call:{name}{{n:01}}'), + # 3. Other drift formats + ('python style', f'{name}(query="x")'), + ('fenced json', f'```json\n{{"name": "{name}"}}\n```'), + ('tool_code style', f'print({name}(query="x"))'), + ('name colon args', f'{name}: {{"query": "x"}}'), + ] + +print(f'{"verdict":13} {"tok":>3} fits {"kind":28} payload') +print('-' * 100) +rows = [] +for kind, text in candidates: + verdict, detail = parse(text, ALLOWED) + n = ntok(text) + fits = ' *' if 13 <= n <= 15 and verdict == 'malformed' else ' ' + rows.append((verdict, n, fits, kind, text)) + +# Matches first, then by verdict. +for verdict, n, fits, kind, text in sorted(rows, key=lambda r: (r[2] != ' *', r[0])): + print(f'{verdict:13} {n:>3} {fits} {kind:28} {text[:60]!r}') + +# --- Bug demos on valid native calls -------------------------------------- +print('\n--- silent-corruption / latent-bug checks (valid native dialect) ---') +demos = [ + ('space inside json string', 'call:read_file{path:"/tmp/a b c"}'), + ('gemma string keeps space', 'call:read_file{path:<|"|>/tmp/a b c<|"|>}'), + ('hyphenated tool', 'call:todo-write{}'), + ('dotted tool', 'call:browser.open{}'), + ('backslash-space in string', 'call:read_file{path:"a\\ b"}'), +] +for kind, text in demos: + verdict, detail = parse(text, ALLOWED) + print(f'{verdict:13} {ntok(text):>3} {kind:28} {text!r} -> {detail!r}') From adfe5a1ce8509aef1c8375ec47eb00bce9c478d4 Mon Sep 17 00:00:00 2001 From: Andrey Mikhaylov Date: Wed, 26 Aug 2026 13:55:58 +0100 Subject: [PATCH 3/3] Remove internal working documents committed by mistake The previous commit staged the whole docs/ directory and swept in six untracked working files that were never meant to be published, plus a stray directory left by a failed sync whose name contained a newline. Removed: the four ISSUE_84 review and planning documents, the terminal 3D engine plan, and issue84_simulate_payloads.py. These are internal working notes; they contain local filesystem paths and unreleased planning material. They remain reachable in this branch's history at 04b36ce. Rewriting that history would discard the main merge made on top of it, so it is left for a deliberate decision rather than done here. --- .../docs/experiments/summaries/06-prefill.md" | 0 docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md | 201 ---- docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md | 399 -------- docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md | 404 -------- ...SUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md | 456 --------- docs/TERMINAL_3D_ENGINE_PLAN.md | 886 ------------------ docs/issue84_simulate_payloads.py | 305 ------ 7 files changed, 2651 deletions(-) delete mode 100644 "Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift\npublic/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift\npublic/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift\npublic/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift\npublic/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift\npublic/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift\npublic/Sources/TurboFieldfareCLI/Run.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift\npublic/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift\npublic/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift\npublic/docs/OPTIMIZATION_JOURNEY.md\npublic/docs/RUNTIME_CONTROLS.md\npublic/docs/experiments/EXPERIMENT_INVENTORY.md\npublic/docs/experiments/summaries/06-prefill.md" delete mode 100644 docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md delete mode 100644 docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md delete mode 100644 docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md delete mode 100644 docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md delete mode 100644 docs/TERMINAL_3D_ENGINE_PLAN.md delete mode 100644 docs/issue84_simulate_payloads.py diff --git "a/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift\npublic/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift\npublic/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift\npublic/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift\npublic/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift\npublic/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift\npublic/Sources/TurboFieldfareCLI/Run.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift\npublic/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift\npublic/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift\npublic/docs/OPTIMIZATION_JOURNEY.md\npublic/docs/RUNTIME_CONTROLS.md\npublic/docs/experiments/EXPERIMENT_INVENTORY.md\npublic/docs/experiments/summaries/06-prefill.md" "b/Sources/TurboFieldfare/Infrastructure/Metal/MetalContext.swift\npublic/Sources/TurboFieldfare/Kernels/Attention/PrefillAttention.swift\npublic/Sources/TurboFieldfare/Kernels/Vision/VisionResize.swift\npublic/Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift\npublic/Sources/TurboFieldfare/Runtime/Inference/RealForwardRunner.swift\npublic/Sources/TurboFieldfare/Runtime/Vision/VisionRuntime.swift\npublic/Sources/TurboFieldfareCLI/Run.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/InteractivityWatchdogEnvironmentTests.swift\npublic/Tests/TurboFieldfare/Core/Infrastructure/MetalContextDeviceCreationTests.swift\npublic/Tests/TurboFieldfare/Core/Kernels/Attention/PrefillAttentionTests.swift\npublic/Tests/TurboFieldfare/Core/Runtime/Generation/CommandBufferCompletionTests.swift\npublic/docs/OPTIMIZATION_JOURNEY.md\npublic/docs/RUNTIME_CONTROLS.md\npublic/docs/experiments/EXPERIMENT_INVENTORY.md\npublic/docs/experiments/summaries/06-prefill.md" deleted file mode 100644 index e69de29b..00000000 diff --git a/docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md b/docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md deleted file mode 100644 index 56c0fc63..00000000 --- a/docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md +++ /dev/null @@ -1,201 +0,0 @@ -# Independent Review Request: TurboFieldfare Issue #84 - -Perform a read-only, evidence-based investigation. Do not implement or edit -anything. Separate the immediate failure mechanism from the underlying cause, -and independently verify every conclusion below. - -## Primary Sources - -- Issue: -- New reproduction: - -- Diagnostic PR already merged: - -- Proposed grammar-constrained fix: - - -Repository: - -```text -/Users/andreymikhaylov/development/turbo-fieldfare -``` - -Refresh the current GitHub and `origin/main` state before drawing conclusions. - -## Original Report - -An OpenCode client successfully completed two tool-calling turns, then failed -on the third: - -```text -prepared prompt=23803 -> completed in 46.575s cached=16742 completion=57 finish=tool_calls -prepared prompt=25658 -> completed in 14.396s cached=23875 completion=57 finish=tool_calls -prepared prompt=27513 -> failed after 231s phase=generating status=500 -error=TurboFieldfare.GemmaToolCallParserError.malformed -``` - -Setup: - -- Commit `fcd8f78` -- `--max-context 65536` -- Apple Silicon, 48 GB -- Tool definitions sent on each turn - -The original theory was drift or truncation after a very long generation, but -raw generated output was unavailable. - -## New Independent Reproduction - -The linked comment reports: - -- Commit `acefaf1` -- Hermes Agent using `/v1/chat/completions` -- 26 tools -- Two-message conversation -- 17,355 rendered prompt tokens -- Failure is reportedly 100% reproducible at this prompt size -- Same result at temperature `0.2` and greedy temperature `0` - -Diagnostics: - -```text -completion_tokens=16 max_completion_tokens=48181 raw_stop=stop_string -tool_start_count=1 tool_end_count=1 decoded_calls=0 -last_tool_start_offset=0 last_tool_end_offset=15 -effective_count_matches_result=true effective_prefix_matches_kv=true -kv_position_matches_history=true completion_count_matches_history=true -prefill_accounting_matches=true -``` - -The commenter also reports different generated-token hashes across two -ostensibly identical temperature-0 attempts. - -## Current Interpretation to Verify - -The immediate failure mechanism appears clear: - -1. Generated token 0 is `<|tool_call>`. -2. Tokens 1-14 form the tool-call payload. -3. Token 15 is ``. -4. When the closing marker arrives, `StructuredAssistantDecoder` decodes the - collected payload. -5. `GemmaToolCallParser` expects: - - ```text - call:{...} - ``` - -6. The payload fails that parser, producing - `decoder_consume cause=malformed`. -7. The server stops generation and returns HTTP 500. - -Relevant files: - -- `Sources/TurboFieldfareServer/Core/ServerInference.swift` -- `Sources/TurboFieldfare/Tokenization/StructuredAssistantDecoder.swift` -- `Sources/TurboFieldfare/Tokenization/GemmaToolCallParser.swift` -- `Sources/TurboFieldfare/Runtime/Generation/RawCompletion.swift` -- `Sources/TurboFieldfareServer/Core/GemmaToolSchema.swift` - -Important nuance: `raw_stop=stop_string` probably does not mean a -client-supplied stop string matched. `ServerInference` sets `shouldStop = true` -after the decoder throws, while `RawCompletion` combines -`stopMatcher.isStopped || shouldStop` and records either condition as -`.stopString`. Verify this carefully. - -## What the Diagnostics Seem to Establish - -- This reproduction is not an unfinished tool block or maximum-token - truncation. -- The model emitted both opening and closing tool markers. -- Parsing failed while consuming the closing marker. -- The failure is not merely an orphan `<|tool_response>` marker. -- It reproduces under greedy generation, so ordinary sampling luck is - unlikely. -- The server's token-history, prefix-cache, position, and prefill-count - bookkeeping is internally consistent. - -Do not overinterpret the KV checks. They compare token IDs, counts, and -positions. They do not validate the numerical K/V tensors or the correctness -of long-context attention or prefill computation. - -Also challenge the commenter's claim that 14 payload tokens are "nowhere near -enough" for a valid call. A short tool name with an empty or small argument -object may fit. The token count proves that the closed block was malformed, -not why it was malformed. - -## Main Unresolved Question - -What were the actual payload bytes or token IDs between the two tool markers? - -Without that evidence, distinguish among: - -1. The model emitted canonical JSON instead of Gemma's native - `call:name{...}` dialect. -2. The model emitted another malformed native-dialect call. -3. The parser rejects a representation that should reasonably be accepted. -4. Detokenization changed the payload before parsing. -5. Long-context prefill or attention produced incorrect logits despite - consistent bookkeeping. -6. A nondeterministic runtime issue explains the differing temperature-0 - hashes. -7. The two "identical" greedy attempts were not actually identical in prompt - IDs, cache state, runtime configuration, or process state. - -## PR #107 - -PR #107 claims its private reproduction showed the model drifting to canonical -JSON inside `<|tool_call>`. It adds grammar-constrained decoding to prevent -invalid tool payloads. - -Treat that claim as a hypothesis requiring independent verification because: - -- The public issue comment does not include the raw payload. -- The end-to-end fixture used by the PR author is private and not checked in. -- The PR is broad, adding general forced-JSON support as well as the - issue-specific tool grammar. -- At the previous inspection it was open, conflicting, and had no reported - GitHub checks; refresh this state. - -Assess whether PR #107: - -- fixes the demonstrated cause or merely masks malformed generation; -- is necessary compared with accepting both native and canonical - representations; -- could conceal a long-context numerical correctness problem; -- preserves prompt-cache and fail-closed semantics; -- handles incomplete calls, UTF-8, byte limits, unknown tools, and completion - budgets correctly; and -- is appropriately scoped for issue #84. - -## Template Observation - -The commenter noticed several unguarded `value['type'] | upper` expressions in -`chat_template.jinja`. - -The current server's `GemmaToolSchema` adapter appears to reject unsupported -types and normalize nullable type arrays to one concrete string type before -invoking the template. Verify whether every unguarded Jinja path is therefore -unreachable through validated OpenAI requests. - -Treat this as separate from issue #84 unless a concrete request reaches the -Jinja error. - -## Requested Output - -Return: - -1. A direct verdict: is the immediate failure understood? -2. A separate verdict: is the underlying root cause established? -3. Confirmed facts, probable explanations, and unsupported claims. -4. An explanation of the temperature-0 hash discrepancy. -5. An assessment of PR #107's root-cause claim and scope. -6. The smallest decisive next experiment. -7. Any mistakes in the interpretation above. - -The likely decisive experiment is to obtain the reporter's full request or a -minimized reproducer, run current `main` at temperature 0 with prompt caching -disabled, and capture the exact bounded payload between `<|tool_call>` and -``. Do not expose private prompt text in public logs. - -No source files should be changed during this review. diff --git a/docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md b/docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md deleted file mode 100644 index 818e4a02..00000000 --- a/docs/ISSUE_84_INDEPENDENT_REVIEW_FINDINGS.md +++ /dev/null @@ -1,399 +0,0 @@ -# Independent Review Findings: Issue #84 - -Read-only investigation performed 2026-08-14 against `origin/main` (`3e87d92`) -per `docs/ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md`. No source files were changed. - -GitHub state at review time: - -- Issue #84: OPEN. Comments: two maintainer replies, then the independent - reproduction from `idallasj` (2026-08-13, comment 5284001871). -- PR #90 (diagnostics): merged; the reproduction used its output. -- PR #107 (grammar-constrained decoding): OPEN, CONFLICTING, no CI checks. - -## 1. Verdict: immediate failure mechanism — understood, with one correction - -The brief's step-by-step mechanism is confirmed against `main`: - -- `StructuredAssistantDecoder.consume` collects payload token IDs between - `<|tool_call>` and ``, re-decodes them with - `decode(skipSpecialTokens: false)`, and hands the text to - `GemmaToolCallParser.parse` - (`Sources/TurboFieldfare/Tokenization/StructuredAssistantDecoder.swift:71-87`). -- On parse failure, `ServerInference` records the error and sets - `shouldStop = true` - (`Sources/TurboFieldfareServer/Core/ServerInference.swift:590-593`). - `RawCompletion.swift:221-226` folds `stopMatcher.isStopped || shouldStop()` - into one label and records `reason = .stopString`. **`raw_stop=stop_string` - in the diagnostics is the server's own post-error abort, not a client stop - string.** The brief's nuance is verified correct. -- `completion_tokens=16` with `last_tool_end_offset=15` matches the loop - arithmetic exactly: token 0 opens the call, tokens 1–14 are payload, token - 15 closes it, the decoder throws, the loop aborts on the next check. - -Correction: **the two reproductions are not the same failure shape.** - -- Original OpenCode report (`fcd8f78`, 231s, ~3k tokens, thrown at old - `ServerInference.swift:365`): the *orphan `<|tool_response>`* guard. The - model sampled the stop sentinel with zero complete calls — it either never - opened a call or wrote something else for thousands of tokens. Post-#90 - this would be `kind=orphan_tool_response cause=none`. -- New Hermes repro (`acefaf1`): `kind=decoder_consume cause=malformed` — a - **closed 14-payload-token block that failed parsing** after only 16 - generated tokens. - -Both are "model output the parser refuses," but evidence from one does not -automatically transfer to the other. - -## 2. Verdict: underlying root cause — NOT established - -The payload bytes between the markers are unknown and nothing checked in can -recover them (the #90 diagnostics deliberately log no content). Assessment of -the brief's seven candidates: - -1. **Canonical JSON instead of `call:name{...}`** — most probable, unproven. - 14 tokens fits `{"name": "x", "arguments": {}}` comfortably. Contributing - prior: `encodeToolChat` hardcodes `enable_thinking: false` - (`Tokenizer.swift:368-372`), so the template force-closes an empty thought - channel and the model must emit the call format with zero deliberation - tokens at a 17k-token prompt. -2. **Another malformed native call** — equally possible; the parser also - rejects `call :`, leading-zero/`+`-prefixed numbers, trailing text. -3. **Parser rejects what it should accept** — partially confirmed as latent - bugs, but they cannot be this specific failure: - - `OpenAIToolName.isValid` (`OpenAIModels.swift:258-269`) accepts hyphens; - `identifier()` (`GemmaToolCallParser.swift:88`) stops at them. A - correctly emitted `call:web-search{...}` is undecodable. But that path - throws `unknownTool`, and the diagnostic says `cause=malformed`, so the - Hermes failure is not the hyphen bug. - - `jsonString()` (`GemmaToolCallParser.swift:170-183`) loops through - `take()`, which calls `skipWhitespace()` first — spaces/tabs/newlines - inside JSON string values are silently eaten (`"/tmp/a b"` → `"/tmp/ab"`). - Silent corruption, not a throw. -4. **Detokenization changed the payload** — weak on current `main`: #118 - (`acefaf1`) made decode lossless by construction (`GemmaDecoding`, no - `clean_up_tokenization_spaces`). PR #107's "cleanup swallows bytes" - rationale describes the pre-#118 decoder. -5. **Long-context numeric error** — cannot be ruled out; the KV diagnostics - compare token IDs/counts/positions only, never tensor values. -6. **Nondeterministic runtime** — ruled out for the GPU code (see §4). -7. **The two "identical" attempts were not identical** — favored explanation - for the hash discrepancy (see §4), and decidable from existing diagnostics. - -Signal worth noting: 100% reproducibility *with differing token hashes* means -the failure is robust across trajectory variation — this fits systematic -format drift better than a knife's-edge numeric fault. - -The commenter's "14 tokens is nowhere near enough for a real call" is wrong: a -valid minimal call (`call:` + short name + `{}`) fits in fewer tokens. The -count proves only that the closed block was malformed, not why. - -### Payload simulation (2026-08-14) - -Since the payload bytes are unknown, candidate payloads were simulated with -the real tokenizer (`scratch/gemma4.gturbo/tokenizer/tokenizer.json`) and a -bug-faithful Python port of `GemmaToolCallParser` (both known bugs -reproduced). Script: `docs/issue84_simulate_payloads.py` (needs -`pip install tokenizers`; the port was validated against all predicted -parser behaviors — space-eating corruption, hyphen/dot → `unknown_tool`, -backslash-space → `malformed`, gemma strings parse). - -Observed signature to match: exactly 14 payload tokens, `cause=malformed`. -Token-count bands across 9 representative tool names: - -| payload shape | verdict | tokens (minimal args) | -| --- | --- | --- | -| canonical JSON `{"name": "N", "arguments": "{}"}` (string-typed args, i.e. the OpenAI wire serialization) | malformed | **exactly 14** for every 2-token name tried | -| canonical JSON `{"name": "N", "arguments": {}}` | malformed | 13 | -| canonical JSON, one short arg | malformed | 15–17 | -| `{"tool": "N", "args": {}}` / fenced ```` ```json ```` block | malformed | 13 | -| native near-misses (`Call:`, missing colon, paren args, single quotes, unquoted value, JSON-quoted keys, trailing text) | malformed | 3–11 | -| valid native `call:N{}` / gemma-string args | ok | 4–12 | - -Read: the observed 14-token `malformed` block sits squarely in the -**canonical-JSON band** and is an exact-width match for the OpenAI wire -format with string-encoded empty `arguments` — the precise shape a model -imitating the request-side JSON it was shown would produce. Native-dialect -near-misses are systematically too short unless they carry real arguments. - -Caveats: the model's own token split need not equal canonical BPE encoding -(±1–2 tokens), and the real Hermes tool names are unknown; this narrows the -hypothesis space, it does not replace capturing the payload. But it upgrades -hypothesis 1 (canonical-JSON drift) from "most probable" to "strongly -favored", and it demonstrates that essentially *any* JSON-shaped drift -classifies as `malformed` while dialect-prefix drift (`call:` + bad name) -classifies as `unknown_tool` — so the observed `cause=malformed` itself is -evidence the model abandoned the `call:` prefix entirely. - -## 3. Confirmed facts, probable explanations, unsupported claims - -**Confirmed by code reading:** - -- The full immediate mechanism in §1, including the `stop_string` mislabeling. -- Both parser bugs claimed by PR #107 exist on `main` (hyphen identifier, - whitespace-eating `jsonString`), and its diff for them is correct. -- The server accepts tool names (`-` allowed) that the parser can never - decode — a validator/parser alphabet mismatch. -- Payload decode on `main` is lossless (`Tokenizer.swift:233-249`, - `Detokenizer.swift`). -- A failed request invalidates the prompt cache and resets the runner via the - `defer` in `generate` (`ServerInference.swift:486-491`). - -**Probable:** the model drifted out of the native dialect (canonical JSON or -similar) under a 17k prompt with no thinking budget. - -**Unsupported:** PR #107's claim that its private repro's drift is this -issue's cause; any inference from KV bookkeeping consistency to numeric -correctness; treating the OpenCode and Hermes failures as one mechanism. - -## 4. Temperature-0 hash discrepancy - -A full kernel audit found **no run-to-run nondeterminism in the GPU code**: - -- Greedy argmax is deterministic: fixed lane→element mappings, `simd_max` + - lowest-index tie-break at every level (`Metal/Sampling/logit.metal:687-787`, - same construction in the `sample` kernel's greedy branch). -- MoE expert streaming cannot reorder accumulation: each pread writes a - pre-assigned slot; the combine (`moe_phase2_down_reduce_k8`, - `Metal/MoE/moe.metal:446-482`) sums in router-rank order on a single - thread. Expert-cache hit/miss subset kernels are bit-identical to the full - kernel. -- No float atomics, no concurrent dispatch, fixed-order split-KV attention - combine. - -But two identical-looking runs can still diverge legitimately: - -- **Prefill and decode use different kernel families with different - precision** (tensor-core MPP path dequantizes weights to `half`, - `Metal/TensorCore/tensorops.metal:75`; decode GEMV uses FP32 fma). A chunk - under 32 tokens switches kernels again - (`RealForwardRunner.swift:646`, dispatch policy `:118-131`). Chunk - boundaries shift with `cachedPromptTokens` - (`PrefillRuntimeConfig.swift:95-116`). -- So a **cache-resume legitimately produces different logits than a fresh - prefill** of the same tokens. The MoE router's discrete top-8 selection - (`moe.metal:135-185`) amplifies 1-ULP differences into O(1) activation - changes; at 17k context a near-tie top-2 flips the argmax with no bug. -- At temperature 0.2 the sampler seeds from `CLOCK_MONOTONIC` per token - unless the client sends `seed` (`Sampler.swift:198-208`) — nondeterministic - by design. - -Likely explanations for the differing temp-0 hashes, in order: different -cache/KV state between the attempts (attempt ordering matters because -failures invalidate the cache), or prompts that were not byte-identical -(agent frameworks often embed timestamps). **Decidable today with zero new -code:** the two failure lines already carry `rendered_prompt_i32le_sha256` -and `cached_prompt_tokens` — ask the commenter to diff those fields across -the two attempts. Identical rendered hashes and cached counts with different -generated hashes would be a genuinely new finding deserving its own issue. - -## 5. PR #107 assessment - -- **Root-cause claim: unverified.** The drift-to-JSON evidence is a private - fixture; the public repro has no payload. Plausible, but the PR should not - merge under the banner "root-cause fix" on current evidence. -- **Fix vs mask:** grammar-constrained decoding is standard, legitimate - engineering for tool calls, but it is a behavioral guarantee, not a - root-cause fix — and it would conceal hypothesis 5 (bad logits would - silently produce well-formed but wrong calls) and mask the drift signal. - Acceptable as a knowing product decision. -- **Scope: too broad for #84.** `--force-json` CLI mode, `TokenByteTable`, - and the sampler rework are unrelated to the issue. The two parser fixes are - correct and necessary regardless — worth extracting into a small standalone - PR together with a decision on accepting canonical-JSON payloads as a - fallback. -- **Staleness:** it predates #118; its decoder rationale (library `cleanUp` - mangling bytes) no longer describes `main`, and its - `StructuredAssistantDecoder` diff conflicts with the rewritten one. - -## 6. Smallest decisive next experiment - -The reporter explicitly offered the full request payload — take it. Then on -current `main`: - -1. Run the captured request at `temperature 0` with `--prompt-cache-mode off`, - fresh process per attempt. -2. Capture the bounded region between `<|tool_call>` and ``. This - requires the one thing #90 withheld: an opt-in, failure-only, flag-gated - dump of just the tool-region token IDs (~10 lines). -3. Cheaper first step, zero code: have the reporter rerun twice and diff the - two diagnostic lines' `rendered_prompt_i32le_sha256` and - `cached_prompt_tokens` fields. - -The payload bytes immediately separate hypotheses 1/2/3; the cache-off -determinism run separates 5/6/7. - -## 7. Test and fix plan (deferred — all details for later execution) - -State of coverage on `main`: `GemmaToolCallParser` and -`StructuredAssistantDecoder` have **no dedicated unit tests**. Only indirect -references exist, in `Tests/TurboFieldfareServer/OpenAIValidationTests.swift` -and `Tests/TurboFieldfareServer/StructuredOutputDiagnosticsTests.swift`. -There is no `Tests/.../GemmaToolCallParserTests.swift` — PR #107 adds one, -but on its own conflicted branch. - -### Phase A — confirmed bugs, writable today, no model, no decisions needed - -Create `Tests/TurboFieldfare/Core/Tokenization/GemmaToolCallParserTests.swift`. - -**A1. Hyphenated tool names are undecodable.** - -- Bug: `OpenAIToolName.isValid` - (`Sources/TurboFieldfareServer/Core/OpenAIModels.swift:258-269`) accepts - byte 45 (`-`); `identifier()` - (`Sources/TurboFieldfare/Tokenization/GemmaToolCallParser.swift:88`) - accepts only letters/digits/`_` and stops at `-`. -- Failing test: `parse("call:web-search{}", allowedTools: ["web-search"], - id: "t")` — currently throws `unknownTool("web")`; must return a call - named `web-search` with empty arguments. -- Fix: extend the `identifier()` alphabet with `-` (PR #107's one-line diff - for this is correct and can be lifted verbatim). -- Hardening test: property test over the `OpenAIToolName` alphabet - (`[A-Za-z0-9_-]{1,64}`): every valid name must round-trip through - `parse("call:{}", allowedTools: [name])`. Pins the validator and - parser alphabets together permanently. - -**A2. `jsonString()` silently eats whitespace inside string values.** - -- Bug: the loop at `GemmaToolCallParser.swift:170-183` matches quote and - backslash via `take()`, which calls `skipWhitespace()` first - (`:278-283`) — every space/tab/newline inside a `"…"` value is dropped. -- Failing tests: - - `parse("call:read{path:\"/tmp/a b\tc\"}", ...)` → argument must be - `/tmp/a b\tc`; currently decodes as `/tmp/abc` (silent corruption, no - throw). - - Backslash-space inside a string (`"a\ b"` in payload bytes): currently - throws `malformed` via `escapedFragment()` because the space after `\` - is treated as an escape char; after the fix `\` + invalid escape should - still throw, but a literal space must never be consumed by - `skipWhitespace`. -- Fix: read characters positionally inside the string loop instead of via - `take()` (PR #107's diff for `jsonString()` is correct and liftable). -- Note: `gemmaString()` (`:151-168`) reads positionally and does NOT have - this bug — add a test locking that in (`<|"|>a b<|"|>` keeps its space). - -**A3. Jinja `| upper` crash is reachable through validated requests.** - -- Mechanism: `GemmaToolSchema.adapt` - (`Sources/TurboFieldfareServer/Core/GemmaToolSchema.swift`) validates - `type` only where it recurses (`properties`, `items`); annotation values - (`default`, `examples`, `title`, `$comment`, …) pass through untouched. - `validateSchemaKeys` (`OpenAIModels.swift:382-410`) whitelists nothing — - it only charset-checks property names. The template's - `filter_keys=true` branch (in `format_parameters`, - `scratch/gemma4.gturbo/tokenizer/chat_template.jinja`) is taken for an - object-typed schema with **no** `properties` key and iterates all - non-standard keys (standard = description/type/properties/required/ - nullable) as if they were property schemas, evaluating - `value['type'] | upper` on arbitrary annotation values. -- Reproducer schema (passes validation, expected to crash at render): - - ```json - {"type": "object", "properties": {"cfg": {"type": "object", "default": {}}}} - ``` - -- Test location: `Tests/TurboFieldfare/Core/Tokenization/ChatTemplateTests.swift` - (or the server validation suite if rendering there is easier). First - assert current behavior (Jinja runtime error, e.g. "upper filter requires - string") to confirm reachability, then flip the assertion after fixing. -- Fix options: (a) guard the 6 unguarded `value['type'] | upper` template - sites the commenter identified (they offered their diff); (b) make - `GemmaToolSchema` strip or whitelist annotation keys so the - `filter_keys=true` path can never see non-schema values. (b) is safer: - it also fixes `default: 5` (subscripting an int) and keeps the template - in sync with upstream. -- This is a separate crash class from #84 (render-time, before generation). - -### Phase B — writable today but each encodes a product decision - -**B1. Accept canonical JSON tool-call payloads.** - -- Test: `parse("{\"name\": \"x\", \"arguments\": {\"a\": 1}}", - allowedTools: ["x"])` decodes as a call to `x`. -- This is the direct fix for hypothesis 1 (most probable cause of the - Hermes repro) and the lightweight alternative to PR #107's grammar. Risk: - if the captured payload turns out to be something else, this fixes a - different bug than #84. Decide after the payload capture (Phase C), or - accept both dialects proactively — accepting both is strictly more - permissive and cannot break existing native-dialect parses. - -**B2. Fail-open server behavior (original reporter's actual ask).** - -- Behavior change: when `StructuredAssistantDecoder` fails, return the raw - generated text as assistant `content` with `finish_reason: "stop"` - instead of HTTP 500, so agent clients can retry/recover. -- Test at `ServerInference.generate` level with a scripted - `LogitProducer`/backend emitting a malformed tool block; assert 200-path - completion, cache invalidated (the current `defer` semantics must stay), - and no partial `tool_calls` array. -- Tension: current design is deliberately fail-closed; #90's plan doc lists - "returning raw generation as assistant content" as an explicit non-goal. - This needs a maintainer decision, not just a patch. - -**B3. Diagnostics honesty for the abort label.** - -- `raw_stop=stop_string` currently conflates a client stop match with the - server's own decoder-abort (`RawCompletion.swift:221-226` + - `ServerInference.swift:590-593`). Either add a distinct `StopReason` - label for the abort, or add a test asserting `stop_string_matched=false` - always accompanies decoder-abort lines so log readers can distinguish. - (#90's plan forbade changing `StopReason` — a doc/test-only clarification - is the minimal version.) - -### Phase C — blocked on evidence (the actual #84 regression test) - -The regression test for the Hermes `cause=malformed` failure cannot be -written yet: the payload bytes between `<|tool_call>` and `` are -unknown, so there is no string to assert on. Required sequence: - -1. Get the reporter's full request (offered in the issue comment). -2. Zero-code first step: have the reporter run the failing request twice and - diff `rendered_prompt_i32le_sha256` and `cached_prompt_tokens` across the - two diagnostic lines (resolves the temp-0 hash puzzle: prompt - non-identity vs cache-state difference). -3. Add an opt-in, failure-only, flag-gated dump of the tool-region token IDs - (~10 lines in `ServerInference`; #90 deliberately withheld this, so it - must be explicit opt-in). -4. Run on current `main`, `temperature 0`, `--prompt-cache-mode off`, fresh - process per attempt; capture the payload. -5. Turn the captured payload into a fixed unit-test string in - `GemmaToolCallParserTests` — the true #84 regression test — and only - then decide between B1 (accept canonical JSON), PR #107's grammar - constraint, or a numeric investigation (if the payload is garbage bytes - rather than a recognizable format). - -### Recommended PR ordering - -1. Phase A1+A2 (pure parser bug fixes + new test file; zero risk, needed - regardless of #84's root cause). -2. Phase A3 (template/schema guard, separate crash class, cite the - commenter's report). -3. Phase C instrumentation (opt-in dump flag) + ask the reporter to rerun. -4. Phase B decisions once the payload is in hand; evaluate PR #107 against - the evidence then (its parser fixes will already be merged via step 1; - its remaining value is the grammar constraint, which should be judged as - a product feature, not a bug fix). - -Validation commands used by this repo: - -```bash -swift build -c release -Scripts/test.sh -ruby Scripts/check_markdown_links.rb -``` - -## 8. Mistakes in the brief's interpretation - -- It treats both reproductions as one failure class; they differ in kind - (orphan sentinel vs failed closed block) and scale (~3k vs 16 tokens). -- The template observation is understated: the unguarded Jinja `| upper` - paths are **not** all unreachable. `GemmaToolSchema.adapt` never validates - annotation values (`default`, `examples`, …), and the template's - `filter_keys=true` branch — taken for an object-typed parameter with - annotations but no `properties` key — iterates those annotation values as - property schemas and hits unguarded `value['type'] | upper`. A validated - request like `{"type":"object","default":{}}` as a parameter plausibly - reaches the crash the commenter couldn't isolate. One unit test would - confirm. -- "Deterministic at temp 0" must not be assumed even absent bugs: - cache-resume vs fresh prefill is a numeric fork by design, and temp 0.2 is - clock-seeded by design. diff --git a/docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md b/docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md deleted file mode 100644 index 603245a6..00000000 --- a/docs/ISSUE_84_ROOT_CAUSE_TEST_PLAN.md +++ /dev/null @@ -1,404 +0,0 @@ -# Issue 84: Root-Cause Test Plan - -## Purpose - -Determine why issue 84 produces a closed `<|tool_call>...` block -that `GemmaToolCallParser` rejects. The immediate failure path is already -understood; this plan isolates the source of the malformed payload. - -Treat the two known reports as separate cases until evidence shows they share a -cause: - -- **Case O — original OpenCode flow:** two successful tool turns followed by a - long third generation and HTTP 500. -- **Case H — Hermes reproduction:** a two-message, 26-tool request that closes a - malformed tool block after 16 generated tokens. - -This plan complements: - -- [Independent review brief](ISSUE_84_INDEPENDENT_REVIEW_BRIEF.md) -- [Structured tool-generation diagnostics](ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md) -- [Local server guide](OPENAI_SERVER.md) -- [Runtime controls](RUNTIME_CONTROLS.md) - -## Root-cause standard - -Do not call a hypothesis the root cause merely because one run succeeds after -changing a setting. A root cause is established only when all of the following -are true: - -1. The exact request, rendered prompt token IDs, generation settings, runtime - settings, model revision, and process state are known. -2. The exact generated token IDs and bytes inside the rejected tool block are - captured privately. -3. One controlled change predicts the first output or logit divergence. -4. An A-B-A check removes the failure and then restores it, or an independent - oracle reproduces the same first divergence. -5. The explanation accounts for the entire failing path without contradicting - successful tool turns. - -Case O and Case H may finish with different root causes. - -## Safety and privacy rules - -- Follow the repository model-process, OS, Swift, disk, memory-pressure, and - completed-model checks before every model session. -- Run only one TurboFieldfare, MLX-LM, app, CLI, server, or model-backed test at - a time. -- Keep the server on `127.0.0.1`; do not proxy, tunnel, or expose it. -- Do not download another checkpoint, duplicate the `.gturbo` model, purge - caches, or create a worktree for this investigation. -- Keep full requests, raw generations, tool schemas, tool results, and token - sequences outside the repository. They may contain private or reversible - information. -- Public logs and issue updates may contain bounded counts and hashes, but not - prompt text, generated text, tool names, arguments, results, or token arrays. -- Do not alter the installed `.gturbo` sidecars. Render template variants - out-of-process and record the template hash. -- A temporary local probe or oracle harness requires separate authorization. - Keep it uncommitted until its output proves that a durable test is needed. - -## Required inputs - -Obtain these separately for Case O and Case H before running a comparison: - -- complete request JSON for every turn, including tool order and full schemas; -- original server command and commit; -- whether the request was streaming; -- client name and version; -- generation options, especially temperature, maximum completion tokens, stop - strings, seed, Top-K, and Top-P; -- server runtime settings and prompt-cache mode; -- complete request-correlated diagnostic line; -- for Case O, all messages returned by the server and tool results appended by - the client during the two successful turns. - -Store the private request unchanged. Generate experimental variants from it so -the original remains a byte-for-byte reference. - -## Evidence record - -Create one row per run in a private results table: - -| Field | Required value | -| --- | --- | -| Run ID | Stable case and variant label | -| Code | `git rev-parse HEAD` and dirty-state summary | -| Host | Hardware, RAM, macOS, Swift version | -| Model | `.gturbo` manifest/revision and tokenizer/template hashes | -| Request | Private request SHA-256 and turn number | -| Prompt | Rendered/effective token counts and token hashes | -| Runtime | Full server command and process freshness | -| Generation | Temperature, limits, stop options, Top-K, Top-P, seed | -| Result | Exit/HTTP status, timing footer, finish reason | -| Structure | Marker counts/offsets, decoder phase, parser cause | -| Output | Private generated token IDs and exact tool-payload bytes | - -If rendered or effective prompt hashes differ between supposedly identical -runs, stop. Fix the harness or explain the changed input before comparing -generated output. - -## Phase 1: Capture the decisive artifact - -### 1.1 Reproduce on current `main` - -Build release once. Start with: - -- temperature `0`; -- prompt cache off; -- prefill on with 128-token chunks; -- 16 expert-cache slots, LFU; -- RDADVISE off; -- the reporter's context and completion limits; -- a fresh server process. - -Submit the exact request three times. Do not run unrelated warmups. For Case O, -replay the complete three-turn flow; do not submit only its final request unless -that final request is independently shown to reproduce with cache off. - -The baseline is usable when either: - -- all three runs fail with the same generated token hash and parser cause; or -- the inputs match but output hashes differ, proving a deterministic-runtime - investigation is required. - -### 1.2 Capture raw token evidence privately - -Capture, before structured parsing: - -- all generated token IDs; -- exact token IDs between the final tool-start and tool-end markers; -- the exact bytes produced from those IDs with special-token skipping disabled; -- incremental decoder deltas around both markers; -- final stop reason and boundary token IDs; -- first parser error and allowed-tool set hash. - -Prefer a debugger breakpoint at the `StructuredAssistantDecoder` call to -`GemmaToolCallParser.parse`. If release optimization prevents inspection, use -the smallest failure-only local probe that writes to a permission-restricted -private file. Do not add raw output to normal server logging. - -### 1.3 Classify the payload - -| Captured payload | Next branch | -| --- | --- | -| Valid native `call:name{...}` rejected | Parser or allowed-tool validation | -| Canonical JSON object | Template/model dialect branch | -| Native prefix with malformed/truncated body | Template, model, or numerical branch | -| Token IDs reconstruct differently by decoding path | Tokenizer/detokenizer branch | -| Payload differs with identical prompt IDs and greedy settings | Runtime nondeterminism branch | - -Do not proceed to a broad runtime matrix until this classification exists. - -## Phase 2: Minimize each reproducible case - -Minimize one dimension at a time while requiring the same parser cause and -payload shape: - -1. Binary-search prior conversation turns. -2. Binary-search the ordered tool list. -3. Remove unused schema properties and descriptions. -4. Reduce tool results and ordinary message content. -5. Check whether tool ordering, one tool name, one schema construct, or one - historical assistant/tool turn is necessary. - -After every reduction, run A-B-A: failing original, candidate reduction, -failing original. Stop minimizing when another reduction changes the failure -class or removes the first divergent token. - -Useful deliverables are: - -- the smallest private reproducer preserving the original failure; -- a sanitized public reproducer if its tokenization and behavior are identical; -- an explicit list of request features that are necessary and unnecessary. - -## Phase 3: Template and protocol comparison - -This phase is especially important for Case O because it fails after successful -tool turns. - -### 3.1 Freeze both templates - -Compare: - -- the pinned template embedded in the installed model; and -- a hash-pinned copy of Google's current canonical Gemma 4 template. - -Record both SHA-256 hashes and a focused diff covering assistant tool calls, -tool responses, turn closures, reasoning/history reinjection, and null schema -handling. Relevant upstream references: - -- -- -- - -Discussion reports are hypotheses, not proof for issue 84. - -### 3.2 Run the template A-B-A - -Render the same messages and tools out-of-process with each template. Preserve -the resulting prompt token IDs as private artifacts. Run: - -1. pinned template; -2. current canonical template; -3. pinned template again. - -Keep the model, runtime, and generation settings fixed. Do not modify the -installed `.gturbo` directory. If TurboFieldfare cannot accept explicit prompt -IDs through an existing validation surface, build the smallest non-production -harness only after authorization. - -Interpretation: - -- Only the canonical template succeeds: prompt framing is causal. -- Both templates generate the same malformed dialect: template revision is not - sufficient; compare against the model oracle. -- Only a historical tool turn is required: inspect its exact rendered closure - and the next model-turn opening. -- The template changes the output but neither result is valid: continue with - first-divergence oracle comparison rather than choosing the nicer output. - -### 3.3 Protocol parser comparison - -Feed the captured payload bytes, without model execution, to: - -- `GemmaToolCallParser`; -- a minimal parser implementing the pinned template's native syntax; and -- MLX-LM's Gemma tool parser where applicable. - -MLX-LM documents native Gemma tool-call support here: -. - -Agreement between parsers establishes payload validity, not model correctness. - -## Phase 4: Small runtime differential matrix - -Run this phase only if the payload is malformed under the pinned template and -the cause remains numerical, cache-related, or nondeterministic. Keep exact -request bytes and generation settings fixed. - -Start from baseline `B0`: - -```text -max-context=65536 -prompt-cache-mode=off -prefill=on -prefill-chunk-tokens=128 -expert-cache-slots=16 -expert-cache-policy=lfu -rdadvise=off -temperature=0 -``` - -Run only the first-level variants initially: - -| ID | Single change from B0 | Question answered | -| --- | --- | --- | -| B1 | `--prefill off` | Does chunked prefill change the first output token? | -| B2 | `--max-context 32768` | Is the failure dependent on the 64K allocation/path? | -| B3 | `--expert-cache-slots 32` | Does expert residency alter greedy output? | -| O1 | `--prompt-cache-mode single-prefix` | Does verified KV reuse change Case O? | - -Use B2 only when the full prompt plus fixed completion limit fits 32K. O1 must -replay the real multi-turn sequence and is not useful as a substitute for raw -payload capture in Case H. - -Escalate only the axis that changes the first divergent token: - -- If B1 differs, test chunk sizes 32, 64, and 128. -- If B2 differs, compare the first logits at identical prompt positions and - inspect position/RoPE and attention-length handling. -- If B3 differs, compare LFU and LRU, then expert loads and routed expert IDs. -- If O1 differs, compare each turn's rendered/effective prompt hashes, cached - token count, KV position, and first post-prefix logits. -- If identical inputs produce different greedy hashes, run three fresh-process - repetitions and three same-process repetitions to distinguish initialization - from mutable process state. - -The bounded diagnostics prove token lineage and accounting only. They do not -prove numerical K/V tensor equality. - -## Phase 5: Independent MLX-LM oracle - -Use the exact pinned checkpoint named in -[Implementation references](IMPLEMENTATION_REFERENCES.md). The checkpoint is -already present on the current investigation host; do not download or copy it. -At planning time, the active shell did not expose an `mlx_lm` runtime, so locate -an existing compatible environment or obtain authorization before installing -anything. - -Run TurboFieldfare and MLX-LM sequentially with: - -- identical explicit prompt token IDs; -- the same pinned weights, tokenizer, and template sidecars; -- greedy decoding; -- the same maximum generation count; -- cache reuse disabled; -- generated token IDs retained privately. - -Compare at every generated position: - -- chosen token ID; -- top-k token IDs and logits; -- top-1/top-2 margin; -- first position at which ranking or token choice differs. - -Interpretation: - -- Same malformed payload from both engines: model/template behavior, not a - TurboFieldfare-only numerical defect. -- MLX emits a valid call and TurboFieldfare diverges with a large logit margin: - investigate TurboFieldfare model math at the first divergent position. -- Different token choice with a near-zero margin: repeat and compare logits; - token equality alone is too strict for floating-point implementations. -- Logits agree through the payload but parsing differs: parser or decoding - defect. - -Do not compare free-form text alone. The oracle is useful only with exact prompt -IDs and token/logit evidence. - -## Phase 6: Focused hard tests - -Add durable tests only after the captured payload or first divergence identifies -the relevant boundary. - -### Parser and tokenizer branch - -- Round-trip native calls through template rendering, tokenization, - token-by-token structured decoding, and parsing. -- Preserve a sanitized minimal failing payload as a regression fixture. -- Test exact token-byte reconstruction separately from ordinary decoded text. -- Cover only observed edge classes: marker boundaries, whitespace, Unicode, - quoted or native object keys, nested values, and allowed tool identifiers. - -### Runtime branch - -- Add the smallest scalar/CPU or bounded-logit comparison that fails at the - first divergent layer or position. -- Avoid a full model-backed package test when an existing local reference test - can express the failing invariant. -- Require the test to fail before the fix and pass after it. - -Do not begin broad parser fuzzing, long soak runs, or performance profiling -without evidence that the corresponding subsystem is involved. - -## Phase 7: Evaluate fixes and PR #107 - -Evaluate a proposed change only after the root-cause branch is known: - -- If the parser rejects a valid supported representation, fix and test the - parser at that representation. -- If the pinned template frames multi-turn tool history incorrectly, update the - pinned template with exact before/after render fixtures. -- If TurboFieldfare logits diverge from the oracle, fix the earliest numerical - defect before adding grammar constraints. -- If both engines naturally emit malformed syntax, grammar-constrained decoding - is a mitigation. Report it as such unless the experiment also proves why the - unconstrained model changed dialect. - -PR #107 must still be checked for incomplete calls, UTF-8, size limits, unknown -tools, completion budgets, prompt-cache semantics, and fail-closed behavior. A -successful grammar-constrained run does not by itself establish root cause. - -## Execution order and stop conditions - -Run the investigation in this order: - -1. Acquire exact Case O and Case H requests. -2. Reproduce and capture private payload token IDs and bytes. -3. Classify and minimize each case. -4. Run the template A-B-A. -5. Run only the relevant first-level runtime variants. -6. Use MLX-LM at the first unresolved model-output boundary. -7. Add one focused regression test and evaluate the smallest fix. - -Stop early when an A-B-A result plus raw evidence establishes the root cause. -Do not complete the remaining matrix merely for coverage. - -Stop and report a blocker when: - -- the full request cannot be obtained and the public reproduction cannot be - recreated; -- preflight checks fail; -- supposedly identical runs have different prompt hashes; -- the private payload cannot be captured safely; -- the oracle would require another checkpoint download; -- a test requires simultaneous model processes. - -## Final report - -Create `docs/ISSUE_84_ROOT_CAUSE_RESULTS.md` only after experiments begin. It -should contain: - -1. commit, host, model revision, and exact commands; -2. protocol deviations and unavailable artifacts; -3. one compact run table; -4. the captured payload classification without private content; -5. first-divergence evidence; -6. separate conclusions for Case O and Case H; -7. confirmed root cause, probable contributors, and rejected hypotheses; -8. fix recommendation and assessment of PR #107; -9. remaining uncertainty. - -Do not publish private prompts, payloads, tool schemas, results, or reversible -token sequences in that report. diff --git a/docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md b/docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md deleted file mode 100644 index 7b2fc5e3..00000000 --- a/docs/ISSUE_84_STRUCTURED_TOOL_DIAGNOSTICS_PLAN.md +++ /dev/null @@ -1,456 +0,0 @@ -# Issue 84: Structured Tool-Generation Diagnostics - -## Goal - -Make failures like issue 84 diagnosable from one server log line, without -logging prompts, generated text, tool arguments, or reversible token -sequences. - -This change diagnoses the failure. It does not attempt to fix model behavior, -cache behavior, retry policy, or response fallback. - -## Confirmed behavior - -The implementation must be based on these established facts: - -1. `runRawCompletion` sets `.toolCalls` only when the model samples - `tokenizer.toolResponseID`. -2. That stop token is intercepted before - `StructuredAssistantDecoder.consume` receives it. -3. The guard in `ServerInference.swift` runs only after no decoder error was - captured, `decoder.finish()` succeeded, and zero complete calls were - decoded. -4. Therefore that guard means specifically that the model emitted - `<|tool_response>` without a preceding complete valid tool call. -5. `RawDecodeResult` already retains everything required for diagnostics: - the effective prompt and committed generated tokens in - `kvBackedTokenIDs`, the final sampled stop or failing token in - `uncommittedBoundaryTokenIDs`, and prompt, cache, completion, stop-reason, - and KV-position fields. - -No decode-loop collector is needed. - -## Scope - -Modify only: - -- `Sources/TurboFieldfareServer/Core/ServerInference.swift` -- `Tests/TurboFieldfareServer/OpenAIValidationTests.swift`, or one small - dedicated diagnostics test file if that is materially clearer - -Do not modify: - -- `RawCompletion.swift` -- `StructuredAssistantDecoder.swift` -- `GemmaToolCallParser.swift` -- `HTTPServer.swift` -- `ServerLog.swift` -- server arguments or environment handling -- prompt-cache logic -- documentation beyond this implementation plan - -The existing `ServerLog.failed` path already provides request-ID correlation -and logs `String(reflecting: error)`. Carry the diagnostic payload in the -thrown error and reuse that path. - -## 1. Add a dedicated structured-generation error - -Add internal types near the server inference types in `ServerInference.swift`. - -Suggested shape: - -```swift -enum StructuredOutputFailureKind: String, Sendable { - case decoderConsume = "decoder_consume" - case decoderFinish = "decoder_finish" - case orphanToolResponse = "orphan_tool_response" -} - -struct StructuredOutputFailureDiagnostics: Equatable, Sendable { - // Scalar fields and hashes. -} - -struct StructuredOutputFailure: Error, CustomDebugStringConvertible, Sendable { - let kind: StructuredOutputFailureKind - let cause: String - let diagnostics: StructuredOutputFailureDiagnostics - - var debugDescription: String { - // Stable, bounded, single-line key=value representation. - } -} -``` - -Keep these types internal. They are server implementation details, not API -types. - -`CustomDebugStringConvertible` is important because `ServerLog.failed` uses -`String(reflecting:)`. Add a test proving that reflection produces the -intended bounded log text. - -### Cause classification - -Do not store or reflect an arbitrary underlying error. Classify known parser -errors: - -| Existing error | Diagnostic cause | -| --- | --- | -| `.malformed` | `malformed` | -| `.unknownTool` | `unknown_tool` | -| `.oversized` | `oversized` | -| Anything unexpected | `unexpected` | -| Orphan-sentinel guard | `none` | - -Do not include the generated unknown tool name. Although request tool names -are validated, a model-generated identifier could otherwise make the log -unnecessarily large or expose internal names. - -## 2. Wrap all post-generation structured-output failures - -There are three distinct failure points after `runRawCompletion` returns. - -### A. Decoder failure during token consumption - -Replace: - -```swift -if let decodingError { throw decodingError } -``` - -with a `StructuredOutputFailure` using: - -- `kind = .decoderConsume` -- the classified parser cause -- diagnostics built from the completed `RawDecodeResult` - -This covers malformed arguments, invalid framing, duplicate starts, unmatched -ends, unknown tools, and oversized calls detected during streaming. - -Do not throw directly from the progress callback. The existing callback -behavior captures the error, requests generation stop, and throws after -`runRawCompletion` returns. That is what makes the final token evidence -available. - -### B. Decoder failure at `finish()` - -Wrap: - -```swift -try decoder?.finish() -``` - -in `do/catch` and throw: - -- `kind = .decoderFinish` -- the classified parser cause -- the same diagnostics - -This primarily identifies an unfinished `<|tool_call>` block at EOS, -end-of-turn, stop-string, or max-token termination. - -### C. Orphan tool-response sentinel - -Replace the current `GemmaToolCallParserError.malformed` throw with: - -- `kind = .orphanToolResponse` -- `cause = "none"` -- the same diagnostics - -This corrects the misleading classification without weakening fail-closed -behavior. - -### Preserve failure semantics - -The wrapper must remain an ordinary non-`ServerRequestError`, so HTTP behavior -remains unchanged: - -- non-streaming requests return HTTP 500 with the generic internal-error - envelope; -- streaming requests emit the existing SSE error and `[DONE]`; -- the prompt cache is invalidated; -- the runner is reset; -- no partial content is converted into a successful response. - -## 3. Reconstruct the exact generated token sequence - -Add one private or internal helper in `ServerInference.swift`. - -The generated sequence is: - -```swift -let committedGenerated = result.kvBackedTokenIDs.dropFirst(result.prefillTokens) -let generatedIDs = - Array(committedGenerated) + result.uncommittedBoundaryTokenIDs -``` - -Why this is correct: - -- `kvBackedTokenIDs` begins with the full effective prompt. -- Nonterminal generated tokens are appended after being accepted into KV. -- The terminal or failing token is retained in - `uncommittedBoundaryTokenIDs`. -- Therefore the concatenation includes the orphan `<|tool_response>` token - and parser-failing boundary tokens. - -Make reconstruction defensive: diagnostics must never crash while reporting -another failure. If `prefillTokens > kvBackedTokenIDs.count`, use a safe -bounded drop and report failed lineage invariants. - -Do not add a generated-token property to `RawDecodeResult`; reconstruction is -only needed on this server failure path. - -## 4. Diagnostic payload - -Emit a stable, single-line, failure-only summary. - -### Request and cache fields - -- `rendered_prompt_tokens`: `promptIDs.count` -- `effective_prompt_tokens`: `effectivePromptIDs.count` -- `result_prompt_tokens`: `result.prefillTokens` -- `cached_prompt_tokens`: `result.cachedPromptTokens` -- `computed_prefill_tokens`: `result.computedPrefillTokens` -- `completion_tokens`: `result.newTokens` -- `max_completion_tokens`: final `config.maxNewTokens` -- `raw_stop`: explicit mapping to `eos`, `end_of_turn`, `max_tokens`, - `stop_string`, or `tool_calls` -- `kv_position`: `result.kvPosition` -- `kv_backed_tokens`: `result.kvBackedTokenIDs.count` -- `boundary_tokens`: `result.uncommittedBoundaryTokenIDs.count` - -### Structured decoder fields - -- `decoded_calls`: `calls.count` -- `visible_bytes`: `content.utf8.count` -- `stop_string_matched`: `stopMatcher.isStopped` - -Use UTF-8 bytes rather than Swift character count. - -### Tool-marker fields - -Scan only the reconstructed generated sequence: - -- `tool_start_count` -- `tool_end_count` -- `tool_response_count` -- `tool_response_end_count` -- `last_tool_start_offset` -- `last_tool_end_offset` -- `last_tool_response_offset` -- `last_tool_response_end_offset` - -Offsets are zero-based within the generated sequence. Use `-1` when absent. - -Do not log arbitrary special-token IDs or a complete special-token trace. -Counts and final offsets are sufficient to distinguish: - -- no call framing followed by a tool response; -- an opened but unfinished call; -- an unmatched close; -- one valid call followed by malformed additional framing; -- repeated response markers. - -### Lineage invariants - -Report separate booleans so a false result is actionable: - -- `effective_count_matches_result`: - `effectivePromptIDs.count == result.prefillTokens` -- `effective_prefix_matches_kv`: enough KV-backed tokens exist and their - prompt prefix exactly equals `effectivePromptIDs` -- `kv_position_matches_history`: - `result.kvPosition == result.kvBackedTokenIDs.count` -- `completion_count_matches_history`: reconstructed generated count equals - `result.newTokens` -- `prefill_accounting_matches`: cached plus computed prefill equals total - prefill - -These checks are failure-only and bounded by the configured context size. - -## 5. Token hashes - -Compute three deterministic SHA-256 fingerprints: - -- `rendered_prompt_i32le_sha256` -- `effective_prompt_i32le_sha256` -- `generated_i32le_sha256` - -Serialization must be defined precisely: - -1. Treat each token as its `UInt32(bitPattern:)`. -2. Serialize four bytes in little-endian order. -3. Concatenate without delimiters. -4. SHA-256 the resulting bytes. -5. Emit lowercase hexadecimal. - -`CryptoKit` is already imported by `ServerInference.swift`; do not add a -dependency or a general hashing utility. - -These hashes allow later cache-on/cache-off comparison without sharing -content. They are fingerprints, not anonymization; do not describe them as -anonymous or secret-safe against dictionary attacks. - -Hashing happens only after a structured generation fails, so it does not -affect successful generation performance. - -## 6. Log format - -Keep one line and follow existing server logging conventions. - -Expected shape: - -```text -error=structured_output_failure kind=orphan_tool_response cause=none rendered_prompt_tokens=27513 effective_prompt_tokens=... cached_prompt_tokens=... computed_prefill_tokens=... completion_tokens=... max_completion_tokens=... raw_stop=tool_calls kv_position=... kv_backed_tokens=... boundary_tokens=1 decoded_calls=0 visible_bytes=0 stop_string_matched=false tool_start_count=0 tool_end_count=0 tool_response_count=1 tool_response_end_count=0 last_tool_start_offset=-1 last_tool_end_offset=-1 last_tool_response_offset=... last_tool_response_end_offset=-1 effective_count_matches_result=true effective_prefix_matches_kv=true kv_position_matches_history=true completion_count_matches_history=true prefill_accounting_matches=true rendered_prompt_i32le_sha256=... effective_prompt_i32le_sha256=... generated_i32le_sha256=... -``` - -Requirements: - -- no newlines; -- stable field order; -- no arrays; -- no request text; -- no decoded generation; -- no tool schemas, names, arguments, or results; -- no arbitrary error descriptions; -- bounded length independent of prompt or completion size. - -Do not change `ServerLog.failed`; its existing prefix supplies the timestamp, -request ID, HTTP status, and request phase. - -## 7. Focused test - -Add one synthetic diagnostics test using `RawDecodeResult`; no model is -required. - -Suggested scenario: - -1. Build a small effective prompt. -2. Put two ordinary generated tokens in `kvBackedTokenIDs`. -3. Put `tokenizer.toolResponseID` in - `uncommittedBoundaryTokenIDs`. -4. Set `reason = .toolCalls`, `newTokens = 3`, nonzero cached prompt tokens, - and consistent KV position and accounting. -5. Construct `.orphanToolResponse` diagnostics. -6. Assert that: - - reconstructed completion includes the boundary token; - - completion count is correct; - - tool-response count is one; - - tool start and end counts are zero; - - response offset is the final generated offset; - - all lineage invariants are true; - - hash output is lowercase 64-character hex; - - `String(reflecting: error)` contains the required fields; - - the reflected error contains no decoded prompt or content text and no - complete token arrays. - -Include a fixed hash vector for a tiny known `[Int32]` sequence so the `i32le` -serialization convention cannot silently change. - -Do not create model-backed server tests or mock `ServerModelSession`; that -would require unnecessary dependency injection for a pure diagnostic -transformation. - -## 8. Validation - -Mandatory checks: - -```bash -Scripts/test.sh --filter 'TurboFieldfareServerTests\.(StructuredOutputDiagnosticsTests|GemmaToolCallTests|ServerPromptCacheTests)' -swift build -c release --product TurboFieldfareServer -git diff --check -``` - -If the new test is added to an existing suite, adjust the filter accordingly. - -Also review the final diff for these properties: - -- only failure paths changed; -- successful completion logging is unaffected; -- no backend protocol signatures changed; -- no new CLI or environment controls exist; -- no prompt-cache behavior changed; -- HTTP and SSE envelopes remain unchanged; -- the existing `defer` still invalidates the cache and resets the runner on - every wrapped failure. - -No model run is required to validate this implementation. - -## 9. Reporter follow-up - -After the diagnostic patch is available, ask the issue 84 reporter to rerun -the same three-turn OpenCode flow and provide: - -- the complete single failure log line; -- server commit; -- whether the request was streaming; -- the unchanged launch command. - -Do not initially ask for the raw prompt or generated text. - -Interpret the result as follows: - -| Evidence | Likely meaning | -| --- | --- | -| `tool_start_count=0`, `tool_end_count=0`, `tool_response_count=1` | Confirmed orphan response sentinel | -| Start count greater than end count and `kind=decoder_finish` | Unfinished tool call | -| `kind=decoder_consume` with matching start and end counts | Malformed call body or invalid tool | -| Any false lineage invariant | Runtime or result-accounting defect; investigate before blaming model or cache | -| Cached tokens greater than zero | Cache path participated; this does not prove it caused divergence | -| Cache-off and cache-on produce identical generated hashes | Behavior is not caused by cache reuse | -| Only cache-on reproduces under deterministic generation | Investigate long-context continuation and KV parity | - -If cache causation remains plausible, perform a separate controlled -reproduction: - -1. Capture the exact sanitized request sequence. -2. Force deterministic generation with temperature `0`. -3. Run sequentially with `--prompt-cache-mode off`. -4. Run sequentially with `--prompt-cache-mode single-prefix`. -5. Follow all repository model-process and memory checks. -6. Compare counts, prompt fingerprints, generated fingerprints, and marker - positions. - -Do not run two model processes simultaneously. - -## Explicit non-goals - -Do not expand this work into: - -- returning raw generation as assistant content; -- retrying automatically; -- exposing hidden thought output; -- adding `--debug`, `--verbose`, or raw-dump flags; -- logging complete token IDs; -- changing `StopReason`; -- changing the decoder grammar; -- changing prompt-cache matching; -- adding a general logging framework; -- claiming the root cause is model drift or KV divergence before comparative - evidence exists. - -A raw-generation dump can be considered later only if the scalar diagnostics -reproduce but cannot distinguish the cause. It must be a separate, explicitly -sensitive opt-in change. - -## Acceptance criteria - -The work is complete when: - -1. The former orphan-sentinel guard throws `orphan_tool_response`, not - `GemmaToolCallParserError.malformed`. -2. All three structured-output failure phases carry the same bounded - diagnostic schema. -3. The existing HTTP logger produces one request-correlated line with actual - completion and cache counts. -4. The terminal boundary token is included in marker counts and the generated - hash. -5. No raw user, model, or tool content is logged. -6. Existing failure, cache invalidation, runner reset, HTTP 500, and SSE - behavior remain unchanged. -7. Focused tests and the release server build pass. - -Suggested single commit: - -```text -Diagnose malformed server tool generations -``` diff --git a/docs/TERMINAL_3D_ENGINE_PLAN.md b/docs/TERMINAL_3D_ENGINE_PLAN.md deleted file mode 100644 index 766203d3..00000000 --- a/docs/TERMINAL_3D_ENGINE_PLAN.md +++ /dev/null @@ -1,886 +0,0 @@ -# Procedural Terminal 3D Engine Plan - -## Goal - -Build a procedural 3D endless runner rendered entirely with terminal symbols. -The engine and game are written in C17 with no runtime assets and no external -runtime dependencies. Geometry, animation, materials, physics, levels, and -rendering are all described in code. - -The first proof of concept is a spinning torus. It must be rendered as a normal -indexed triangle mesh through the same generic pipeline that later renders the -runner, track, and obstacles. The project must not contain a torus-specific -rendering path. - -## Product constraints - -- C17 engine and game. -- Command-line executable for macOS and Linux terminals. -- POSIX platform layer using `termios`, `poll`, `ioctl`, `clock_gettime`, and - `write`. -- No ncurses, Notcurses, SDL, OpenGL, Metal, Lua, ECS framework, or asset - loader in the initial game. -- No OBJ, glTF, textures, configuration files, or resource directory. -- All geometry and animation are procedural. -- Deterministic level generation from a numeric seed. -- No heap allocation during the steady-state frame loop. -- One buffered terminal write per full frame where practical. -- ASCII fallback when the terminal or font cannot display the selected Unicode - symbols safely. - -The intended technical description is: - -> A procedural 3D endless runner rendered entirely with terminal symbols. It is -> written in pure C with no dependencies and no assets; everything from -> geometry and animation to physics and levels is generated in code. - -## Development sources and single-file distribution - -Development must use normal, modular source files. The one-file version is a -generated release artifact, not the canonical source and not a file edited by -hand. This is an amalgamation build, similar to single-file distributions used -by established C projects. - -Canonical development layout: - -```text -src/ - main.c - math.c - math.h - memory.c - memory.h - mesh.c - mesh.h - render.c - render.h - glyph.c - glyph.h - terminal.c - terminal.h - world.c - world.h - game.c - game.h - -tests/ - test_math.c - test_mesh.c - test_render.c - test_glyph.c - test_world.c - -tools/ - amalgamate.py - -dist/ - term3d.c # generated; never edited manually -``` - -The initial torus milestone should use fewer modules if some of these files do -not yet have real content. Empty `world`, `game`, physics, or animation modules -must not be created in advance merely to reserve the names. - -Development build: - -```bash -cc -std=c17 -O3 -c src/math.c -cc -std=c17 -O3 -c src/mesh.c -cc -std=c17 -O3 -c src/render.c -cc -std=c17 -O3 -c src/glyph.c -cc -std=c17 -O3 -c src/terminal.c -cc -std=c17 -O3 -c src/main.c -cc math.o mesh.o render.o glyph.o terminal.o main.o -lm -o term3d -``` - -Release generation and build: - -```bash -python3 tools/amalgamate.py src/main.c --output dist/term3d.c -cc -std=c17 -O3 dist/term3d.c -lm -o term3d -``` - -The amalgamator recursively expands project-local quoted includes, includes -each project header or source once, preserves system includes, and emits `#line` -directives so diagnostics still name the canonical source file: - -```c -#line 1 "src/render.c" -``` - -Amalgamation-specific rules: - -- Prefix externally visible and internal file-scope names with `t3d_` and a - module name where useful. -- Do not define identically named `static` functions in separate `.c` files; - they collide when combined into one translation unit. -- Prefix project macros and `#undef` temporary implementation macros. -- Do not rely on include order or on macros leaking between modules. -- Never edit `dist/term3d.c` directly. -- Build and test both modular and amalgamated forms in CI. -- Compare a deterministic headless frame checksum from both builds. - -Suggested targets: - -```text -make modular development build -make test modular tests -make amalgamate regenerate dist/term3d.c -make dist-test compile and test the generated source -make release produce dist/term3d.c and the release executable -``` - -## Architecture - -```text -fixed-step game update - |-- input actions - |-- procedural animation - |-- simple physics and collision - `-- deterministic track generation - | - v - transforms and meshes - | - v - generic indexed-mesh renderer - transform -> clip -> project - -> cull -> triangle rasterize - | - v - terminal subcell sample target - inverse depth + shade + colour - | - v - glyph resolver - ASCII / shade / half / quadrant / Braille - | - v - packed terminal cells - | - v - ANSI presenter -> buffered write() -``` - -The renderer consumes immutable meshes, transforms, cameras, and materials. It -must not know whether a transform came from animation, physics, procedural -generation, or input. The terminal presenter consumes resolved cells and must -not know anything about triangles or 3D math. - -## Core data - -```c -typedef struct { float x, y; } T3D_Vec2; -typedef struct { float x, y, z; } T3D_Vec3; -typedef struct { float x, y, z, w; } T3D_Vec4; -typedef struct { float m[16]; } T3D_Mat4; - -typedef struct { - T3D_Vec3 position; - T3D_Vec3 normal; -} T3D_Vertex; - -typedef struct { - T3D_Vertex *vertices; - uint32_t *indices; - uint32_t vertex_count; - uint32_t index_count; -} T3D_Mesh; - -typedef struct { - T3D_Vec3 position; - T3D_Vec3 rotation; - T3D_Vec3 scale; -} T3D_Transform; - -typedef struct { - const T3D_Mesh *mesh; - T3D_Mat4 model; - uint32_t material_id; -} T3D_RenderInstance; -``` - -Euler rotation is sufficient for the first spinning torus and procedural -runner limbs. Add quaternions only when composed rotations or interpolation -make them necessary. - -Transformed vertices preserve the fields needed for clipping and later -perspective-correct interpolation: - -```c -typedef struct { - T3D_Vec4 clip_position; - float shade; -} T3D_ClipVertex; - -typedef struct { - float x; - float y; - float inv_w; - float shade_over_w; -} T3D_ScreenVertex; -``` - -## Procedural torus - -Generate the indexed torus once during initialization. Trigonometry must not be -performed once per torus vertex per frame. - -```c -static T3D_Vertex t3d_make_torus_vertex( - float major_radius, - float minor_radius, - float u, - float v) -{ - const float cu = cosf(u); - const float su = sinf(u); - const float cv = cosf(v); - const float sv = sinf(v); - const float ring = major_radius + minor_radius * cv; - - return (T3D_Vertex) { - .position = { - ring * cu, - minor_radius * sv, - ring * su - }, - .normal = { - cv * cu, - sv, - cv * su - } - }; -} -``` - -Connect adjacent rings with wrapped indexed triangles: - -```c -for (uint32_t i = 0; i < major_segments; ++i) { - for (uint32_t j = 0; j < minor_segments; ++j) { - const uint32_t i1 = (i + 1) % major_segments; - const uint32_t j1 = (j + 1) % minor_segments; - - const uint32_t a = i * minor_segments + j; - const uint32_t b = i1 * minor_segments + j; - const uint32_t c = i1 * minor_segments + j1; - const uint32_t d = i * minor_segments + j1; - - *index++ = a; *index++ = b; *index++ = c; - *index++ = a; *index++ = c; *index++ = d; - } -} -``` - -Start with `64 x 24` segments: 1,536 vertices and 3,072 triangles. The same -renderer must also draw a procedurally generated cube before the torus -milestone is accepted; this proves that the implementation is a generic -renderer rather than a disguised donut algorithm. - -## Symbol-native sample target - -Terminal glyphs such as quadrants and Braille encode multiple spatial samples -inside one cell. The renderer therefore writes depth, coverage, shade, and -colour into a small subcell target. These samples exist only to resolve one -terminal glyph; they are not a separate image or texture pipeline. - -```c -typedef struct { - uint16_t cell_cols; - uint16_t cell_rows; - uint8_t samples_x; - uint8_t samples_y; - uint16_t width; - uint16_t height; - - float *inv_depth; - uint8_t *shade; - uint32_t *colour; -} T3D_SampleTarget; -``` - -Allocate capacity for the largest built-in mode, Braille at `2 x 4`, once. -Less detailed modes use smaller active dimensions without reallocating. - -At `120 x 40` terminal cells, Braille mode contains only 38,400 samples, so a -single-threaded CPU rasterizer is sufficient until profiling proves otherwise. - -## Rendering pipeline - -For every frame: - -1. Clear the active inverse-depth and shading arrays. -2. Build model, view, and projection matrices. -3. Transform each unique mesh vertex once. -4. Transform its normal and calculate directional lighting. -5. Assemble indexed triangles. -6. Clip triangles against the near plane. -7. Perform perspective division. -8. Correct projection for terminal-cell aspect ratio. -9. Cull back-facing or degenerate triangles. -10. Rasterize with incremental edge functions. -11. Interpolate inverse depth and shade. -12. Depth-test every active subcell sample. -13. Resolve samples into packed terminal cells. -14. Encode either a full frame or changed runs into the output arena. -15. Present the completed byte stream with a partial-write-safe loop. - -Initial lighting: - -```c -float diffuse = fmaxf(0.0f, t3d_dot3(normal, light_direction)); -float shade = 0.15f + 0.85f * diffuse; -``` - -Use inverse depth so zero represents an empty sample and larger values are -closer: - -```c -if (inv_depth > target->inv_depth[index]) { - target->inv_depth[index] = inv_depth; - target->shade[index] = t3d_quantize_shade(shade); - target->colour[index] = colour; -} -``` - -Rasterize with edge functions and a consistent top-left fill rule: - -```c -static inline int64_t t3d_edge( - int32_t ax, int32_t ay, - int32_t bx, int32_t by, - int32_t px, int32_t py) -{ - return (int64_t)(px - ax) * (by - ay) - - (int64_t)(py - ay) * (bx - ax); -} -``` - -Transform and clip in floating point, then use a fixed-point screen coordinate -representation for the inner raster loop. Once the initial edge values are -known, advance them across rows with additions rather than recalculating the -full expression for every sample. - -The first torus may be placed wholly in front of the near plane, but near-plane -clipping is required before movable cameras or arbitrary procedural scenes. -Do not clamp vertices to the near plane. Full six-plane homogeneous clipping -can follow when off-screen meshes make it necessary. - -## Glyph modes - -Built-in modes: - -| Mode | Samples per cell | Symbols | -|---|---:|---| -| ASCII | `1 x 1` | ` .:-=+*#%@` | -| Dense ASCII | `1 x 1` | `.,-~:;=!*#$@` | -| Shade | `1 x 1` | ` `, `░`, `▒`, `▓`, `█` | -| Half block | `1 x 2` | ` `, `▀`, `▄`, `█` | -| Quadrant | `2 x 2` | sixteen block masks | -| Braille | `2 x 4` | U+2800 through U+28FF | - -Braille bit order is not row-major: - -```c -static const uint8_t t3d_braille_bit[4][2] = { - { 1u << 0, 1u << 3 }, - { 1u << 1, 1u << 4 }, - { 1u << 2, 1u << 5 }, - { 1u << 6, 1u << 7 } -}; -``` - -Precompute all built-in glyphs as UTF-8 during initialization. Braille becomes -a direct table lookup: - -```c -glyph = braille_utf8[coverage_mask]; -``` - -Call `setlocale(LC_CTYPE, "")`, validate custom ramp code points with -`wcwidth() == 1`, and fall back to ASCII for unsupported locales or glyphs. -Reject combining marks, variation selectors, zero-width joiners, and emoji in -custom ramps. - -## Terminal cells and presentation - -Resolve the sample target into packed logical cells before producing terminal -bytes. A 64-bit cell should contain a glyph-table index, foreground colour, -background colour, and flags. Equality then requires one integer comparison. - -Maintain `next` and `shown` cell grids and support: - -```text ---present full ---present diff ---present auto -``` - -- `full` homes the cursor and encodes the complete frame. -- `diff` groups adjacent changed cells into horizontal runs and emits one cursor - movement per run. -- `auto` estimates both byte costs and emits the smaller representation. - -Do not assume differential output is faster. A rotating object or moving camera -may change enough cells that a complete frame is smaller than many cursor -commands. - -Never use `printf`, `putchar`, `fflush`, `snprintf`, or dynamic string growth in -the cell loop. Build a contiguous byte stream and handle partial writes: - -```c -static bool t3d_write_all(int fd, const void *data, size_t size) -{ - const uint8_t *p = data; - - while (size != 0) { - const ssize_t n = write(fd, p, size); - - if (n > 0) { - p += (size_t)n; - size -= (size_t)n; - } else if (n < 0 && errno == EINTR) { - continue; - } else { - return false; - } - } - - return true; -} -``` - -The terminal backend must: - -- Verify that input and output are TTYs for interactive mode. -- Save and restore the exact original terminal attributes. -- Use the alternate screen and hide the cursor. -- Read dimensions with `ioctl(TIOCGWINSZ)`. -- Use `poll` for input and frame waiting. -- Let signal handlers set only `volatile sig_atomic_t` flags. -- Handle `SIGWINCH` in the main loop. -- Restore terminal state after normal exit, Ctrl-C, termination, or errors. -- Avoid writing a newline after the final row. -- Avoid accidental scrolling from the bottom-right cell. - -## Memory policy - -Use one live backing allocation where practical. Divide it into aligned regions -for long-lived and frame buffers: - -```c -size_t bytes = - vertex_bytes + - index_bytes + - transformed_bytes + - depth_bytes + - shade_bytes + - colour_bytes + - cell_bytes * 2 + - output_bytes; - -uint8_t *memory = malloc(bytes); -uint8_t *cursor = memory; - -vertices = t3d_take(&cursor, vertex_bytes, _Alignof(T3D_Vertex)); -indices = t3d_take(&cursor, index_bytes, _Alignof(uint32_t)); -depth = t3d_take(&cursor, depth_bytes, _Alignof(float)); -cells_a = t3d_take(&cursor, cell_bytes, _Alignof(uint64_t)); -cells_b = t3d_take(&cursor, cell_bytes, _Alignof(uint64_t)); -output = t3d_take(&cursor, output_bytes, 1); -``` - -Allowed allocations: - -- Initial backing block. -- A complete replacement block after terminal resize. - -Forbidden during steady-state frames: - -- `malloc` -- `calloc` -- `realloc` -- `free` -- growing containers or strings - -On resize, allocate the entire replacement first, swap only after success, and -then free the old block. Keep the old renderer alive if replacement allocation -fails. A debug allocation counter must verify zero frame-loop allocations. - -## Fixed update and procedural animation - -Use a fixed simulation timestep independent of terminal presentation: - -```c -const double fixed_dt = 1.0 / 60.0; -double accumulator = 0.0; -double previous = t3d_monotonic_seconds(); - -while (!quit_requested) { - const double now = t3d_monotonic_seconds(); - double elapsed = now - previous; - previous = now; - - if (elapsed > 0.25) - elapsed = 0.25; - - accumulator += elapsed; - t3d_poll_input(&input); - - while (accumulator >= fixed_dt) { - t3d_game_update(&game, (float)fixed_dt); - accumulator -= fixed_dt; - } - - t3d_render_scene(&renderer, &game.scene); - t3d_resolve_glyphs(&renderer, glyph_mode); - t3d_present(&terminal, renderer.cells); - t3d_wait_until_next_frame(); -} -``` - -Advance an absolute monotonic deadline rather than sleeping for a complete frame -duration after rendering. - -The torus animation is initially just code: - -```c -demo.rotation.x += 0.7f * dt; -demo.rotation.y += 1.1f * dt; -``` - -## Procedural geometry and character - -Required code-generated primitives: - -```c -T3D_Mesh t3d_make_cube(...); -T3D_Mesh t3d_make_box(...); -T3D_Mesh t3d_make_torus(...); -T3D_Mesh t3d_make_cylinder(...); -T3D_Mesh t3d_make_sphere(...); -T3D_Mesh t3d_make_capsule(...); -T3D_Mesh t3d_make_ramp(...); -T3D_Mesh t3d_make_arch(...); -T3D_Mesh t3d_make_track(...); -``` - -Assemble the runner from primitive instances rather than creating a single -special mesh: - -```c -typedef struct { - T3D_Transform body; - T3D_Transform head; - T3D_Transform arm_l; - T3D_Transform arm_r; - T3D_Transform leg_l; - T3D_Transform leg_r; -} T3D_RunnerPose; -``` - -Initial running animation: - -```c -const float phase = run_time * run_speed; - -pose.arm_l.rotation.x = sinf(phase) * 0.8f; -pose.arm_r.rotation.x = -sinf(phase) * 0.8f; -pose.leg_l.rotation.x = -sinf(phase) * 0.9f; -pose.leg_r.rotation.x = sinf(phase) * 0.9f; -pose.body.position.y = fabsf(sinf(phase * 2.0f)) * 0.04f; -``` - -Game animation states: - -```text -idle -running -jumping -falling -sliding -crashed -``` - -Each state is a small C function that calculates a pose. Do not add a skeletal -animation framework unless procedural poses become insufficient. - -## Deterministic endless track - -Use a small deterministic PRNG: - -```c -typedef struct { - uint32_t state; -} T3D_Rng; - -static uint32_t t3d_rng_next(T3D_Rng *rng) -{ - uint32_t x = rng->state; - x ^= x << 13; - x ^= x >> 17; - x ^= x << 5; - return rng->state = x; -} -``` - -Generate track segments ahead of the camera and recycle segments that pass -behind it: - -```c -typedef enum { - T3D_SEGMENT_EMPTY, - T3D_SEGMENT_BARRIER, - T3D_SEGMENT_GAP, - T3D_SEGMENT_ARCH, - T3D_SEGMENT_COINS, - T3D_SEGMENT_MOVING_OBSTACLE -} T3D_SegmentType; -``` - -The same seed and input sequence must generate the same game state, level, and -score. This enables deterministic tests, replays, and shareable challenges -without storing level files. - -## Minimal physics - -Implement only the behavior required by a lane-based endless runner: - -```c -typedef struct { - T3D_Vec3 position; - T3D_Vec3 velocity; - int lane; - bool on_ground; -} T3D_RunnerBody; -``` - -Required behavior: - -- gravity -- jump impulse -- smooth lane interpolation -- ground collision -- player capsule versus obstacle box collision -- trigger volumes for collectibles -- gradual forward-speed increase - -For tens of active objects, a direct pair scan is acceptable. Do not implement -torque, joints, stacked rigid bodies, arbitrary convex collision, or a general -physics engine before the game demonstrates a need. - -## Fast inverse square root experiment - -Keep the Quake-style fast inverse square root as an optional implementation and -benchmark, not as the default contract: - -```c -#ifdef T3D_QUAKE_RSQRT -#define t3d_rsqrt(x) t3d_fast_rsqrt(x) -#else -#define t3d_rsqrt(x) (1.0f / sqrtf(x)) -#endif -``` - -Procedural primitives should generate analytic unit normals, rotation preserves -their length, and fixed directions are normalized once. The engine should avoid -normalization rather than force the bit hack into the frame loop. Enable it by -default only if a real target benchmark shows an improvement without unacceptable -error. - -## CLI - -```text -term3d - --glyph ascii|dense|shade|half|quadrant|braille - --ramp STRING - --colour mono|16|256|truecolor - --present full|diff|auto - --fps N - --size COLSxROWS - --aspect RATIO - --segments MAJORxMINOR - --seed N - --frames N - --headless - --benchmark - --stats - --dump-frame PATH - --glyph-test -``` - -Initial controls: - -```text -q or Esc quit -1 through 5 select glyph mode -c cycle colour mode -Space pause -Arrow keys rotate while the torus demo is paused -``` - -Later game controls should use discrete left, right, jump, and slide actions. -Traditional terminal input does not reliably report key releases, so game -movement must not require precise press/release state. - -## Milestones - -### 0. Terminal and glyph probe - -Deliver terminal entry/restoration, resize, raw input, UTF-8 tables, a glyph -test grid, and one full-frame buffered write. - -Gate: - -- Normal exit and Ctrl-C restore the terminal. -- Every selected glyph occupies one column. -- ASCII fallback works. -- Sanitizers find no error across small and large terminal sizes. - -### 1. Generic torus renderer - -Deliver math, generic indexed meshes, procedural torus and cube, camera, -backface culling, fixed-point edge rasterization, inverse depth, Lambert -lighting, ASCII/shade modes, and fixed-step rotation. - -Gate: - -- Correct occlusion and no cracks between adjacent triangles. -- No torus-specific branch in the renderer. -- The same path draws a cube. -- Zero steady-state allocations. -- A fixed headless frame produces a stable logical-cell checksum. - -### 2. Symbol-native rendering - -Deliver half-block, quadrant, and Braille resolution, ordered dithering, -runtime mode switching, Unicode fallback, and aspect correction. - -Gate: - -- All 16 quadrant and all 256 Braille masks map correctly. -- Depth is independent for every active subcell. -- Switching modes or resizing leaves no stale samples. - -### 3. Fast presenter - -Deliver packed front/back cells, full and changed-run encoders, automatic byte -cost selection, colour-state caching, and presenter telemetry. - -Gate: - -- No per-cell stdio call. -- Normally one buffered `write` per full frame. -- Interrupted and partial writes are correct. -- Full and differential presentation are chosen from measurements. - -### 4. Robust procedural scene - -Deliver near-plane clipping, multiple instances, movable camera, perspective- -correct attributes, frustum rejection, and the complete primitive set. - -Gate: - -- Triangles crossing the camera plane do not explode. -- Opaque output does not depend on triangle submission order. -- Random off-screen triangles never write outside the buffers. - -### 5. Procedural runner - -Deliver the primitive-composed runner, run/jump/slide poses, lane movement, -simple collision, score, deterministic track segments, and seeded replay. - -Gate: - -- Recorded input reproduces identical state and score. -- Rendering contains no game or physics behavior. -- No external file is required to start a complete game. -- The torus remains as a regression/demo mode. - -### 6. Amalgamated release - -Deliver the generator, checked-in `dist/term3d.c`, modular and amalgamated CI -builds, and a one-command user build. - -Gate: - -- Modular and amalgamated builds pass the same tests. -- Both builds produce the same deterministic headless checksum. -- `dist/term3d.c` contains no unresolved project-local include. -- `cc -std=c17 -O3 dist/term3d.c -lm -o term3d` succeeds on supported systems. - -## Validation and performance reporting - -Measure independently: - -```text -simulation time -vertex-transform time -clipping time -triangle-raster time -glyph-resolution time -ANSI-encoding time -terminal-write time -bytes per frame -changed cells and runs -missed frame deadlines -allocations per frame -``` - -Required benchmark modes: - -```bash -term3d --headless --benchmark --frames 10000 -term3d --present full --frames 1000 --stats -term3d --present diff --frames 1000 --stats -term3d --present auto --frames 1000 --stats -``` - -Suggested builds: - -```make -CFLAGS_DEBUG = -std=c17 -O0 -g3 -Wall -Wextra -Wshadow -Wconversion \ - -fsanitize=address,undefined -CFLAGS_RELEASE = -std=c17 -O3 -DNDEBUG -flto -Wall -Wextra -LDLIBS = -lm -``` - -Use `-march=native` only for local benchmark builds. Do not enable -`-ffast-math`, add SIMD intrinsics, or add raster threads until profiling shows -that scalar rendering rather than terminal output is the bottleneck. - -## Final acceptance criteria - -```text -canonical development sources: modular C files -release source: one generated C file -external runtime dependencies: zero -runtime assets: zero -procedural geometry: all -procedural animation: all -procedural levels: all -steady-state allocations: zero -normal full-frame writes: one -deterministic seed and replay: supported -headless benchmark: supported -``` - -## Explicit non-goals until proven necessary - -- Runtime asset loading. -- Textures or conventional pixel output. -- General ECS framework. -- Scene graph. -- Lua or another scripting VM. -- Skeletal animation framework. -- General rigid-body physics. -- Multithreaded or SIMD rasterization. -- GPU renderer. -- Windows terminal backend. -- Custom build system. - -The plan intentionally keeps reusable boundaries around rendering, glyph -resolution, terminal presentation, and fixed-step game updates while rejecting -speculative subsystems. Future work should add complexity only when a measured -game requirement crosses the current design's stated ceiling. diff --git a/docs/issue84_simulate_payloads.py b/docs/issue84_simulate_payloads.py deleted file mode 100644 index 312a41f9..00000000 --- a/docs/issue84_simulate_payloads.py +++ /dev/null @@ -1,305 +0,0 @@ -#!/usr/bin/env python3 -"""Issue #84 payload simulation. - -Faithful Python port of GemmaToolCallParser (as of acefaf1/main, bugs -included) + the real gemma4 tokenizer. Sweeps candidate payloads a drifting -model might emit between <|tool_call> and and reports which -match the observed diagnostics: exactly 14 payload tokens and a `malformed` -classification (not unknown_tool / not ok). -""" -import json -import re -from tokenizers import Tokenizer - -TOK = Tokenizer.from_file( - "/Users/andreymikhaylov/development/turbo-fieldfare/scratch/gemma4.gturbo/tokenizer/tokenizer.json") - -NUMBER_RE = re.compile(r'^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?$') - - -class Malformed(Exception): - pass - - -class UnknownTool(Exception): - pass - - -class Parser: - """Port of the private Parser struct in GemmaToolCallParser.swift. - - Deliberately reproduces both known bugs: - - identifier() stops at '-' and '.' (letters/digits/_ only) - - jsonString() calls take(), whose skipWhitespace eats ws inside strings - """ - - def __init__(self, text): - self.c = list(text) - self.i = 0 - - def at_end(self): - return self.i == len(self.c) - - def skip_ws(self): - while self.i < len(self.c) and self.c[self.i].isspace(): - self.i += 1 - - def consume(self, literal): - self.skip_ws() - n = len(literal) - if self.c[self.i:self.i + n] != list(literal): - raise Malformed(f"expected {literal!r} at {self.i}") - self.i += n - - def identifier(self): - self.skip_ws() - start = self.i - while self.i < len(self.c): - ch = self.c[self.i] - if not (ch.isalpha() or ch.isdigit() or ch == '_'): - break - self.i += 1 - if self.i == start: - raise Malformed("empty identifier") - return ''.join(self.c[start:self.i]) - - def starts(self, literal): - return self.c[self.i:self.i + len(literal)] == list(literal) - - def take(self, ch): - self.skip_ws() - if self.i < len(self.c) and self.c[self.i] == ch: - self.i += 1 - return True - return False - - def take_word(self, w): - if self.starts(w): - self.i += len(w) - return True - return False - - def object(self): - self.consume('{') - result = {} - self.skip_ws() - if self.take('}'): - return result - while True: - key = self.object_key() - self.consume(':') - result[key] = self.value() - self.skip_ws() - if self.take('}'): - return result - self.consume(',') - - def object_key(self): - self.skip_ws() - start = self.i - while self.i < len(self.c): - ch = self.c[self.i] - if not (ch.isalpha() or ch.isdigit() or ch in '_-.$'): - break - self.i += 1 - if self.i == start: - raise Malformed("empty object key") - return ''.join(self.c[start:self.i]) - - def value(self): - self.skip_ws() - if self.starts('<|"|>'): - return self.gemma_string() - if self.starts('"'): - return self.json_string() - if self.starts('{'): - return self.object() - if self.starts('['): - return self.array() - if self.take_word('true'): - return True - if self.take_word('false'): - return False - if self.take_word('null'): - return None - return self.number() - - def array(self): - self.consume('[') - result = [] - self.skip_ws() - if self.take(']'): - return result - while True: - result.append(self.value()) - self.skip_ws() - if self.take(']'): - return result - self.consume(',') - - def gemma_string(self): - self.consume('<|"|>') - out = '' - while not self.at_end(): - if self.starts('<|"|>'): - self.consume('<|"|>') - return out - if self.c[self.i] == '\\' and self.i + 1 < len(self.c): - self.i += 1 - out += self.escaped_fragment() - else: - out += self.c[self.i] - self.i += 1 - raise Malformed("unterminated gemma string") - - def json_string(self): - # BUG-FAITHFUL: take() skips whitespace each iteration, so spaces, - # tabs and newlines inside the string are silently dropped. - self.consume('"') - out = '' - while not self.at_end(): - if self.take('"'): - return out - if self.take('\\'): - out += self.escaped_fragment() - else: - out += self.c[self.i] - self.i += 1 - raise Malformed("unterminated json string") - - def escaped_fragment(self): - if self.i >= len(self.c): - raise Malformed("dangling escape") - e = self.c[self.i] - self.i += 1 - simple = {'"': '"', '\\': '\\', '/': '/', 'b': '\b', 'f': '\f', - 'n': '\n', 'r': '\r', 't': '\t'} - if e in simple: - return simple[e] - if e == 'u': - first = self.unicode_unit() - if 0xD800 <= first <= 0xDBFF: - if not (self.c[self.i:self.i + 2] == ['\\', 'u']): - raise Malformed("bad surrogate pair") - self.i += 2 - second = self.unicode_unit() - if not (0xDC00 <= second <= 0xDFFF): - raise Malformed("bad low surrogate") - scalar = 0x10000 + ((first - 0xD800) << 10) + (second - 0xDC00) - else: - if 0xDC00 <= first <= 0xDFFF: - raise Malformed("lone low surrogate") - scalar = first - return chr(scalar) - raise Malformed(f"bad escape {e!r}") - - def unicode_unit(self): - if self.i + 4 > len(self.c): - raise Malformed("short \\u") - s = ''.join(self.c[self.i:self.i + 4]) - try: - v = int(s, 16) - except ValueError: - raise Malformed("bad hex") - self.i += 4 - return v - - def number(self): - start = self.i - while self.i < len(self.c) and self.c[self.i] in '-+0123456789.eE': - self.i += 1 - if self.i == start: - raise Malformed("expected value") - lit = ''.join(self.c[start:self.i]) - if not NUMBER_RE.match(lit): - raise Malformed(f"bad number {lit!r}") - return float(lit) - - -def parse(text, allowed): - if len(text.encode()) > 256 * 1024: - return "oversized", None - p = Parser(text) - try: - p.consume('call:') - name = p.identifier() - if name not in allowed: - raise UnknownTool(name) - args = p.object() - p.skip_ws() - if not p.at_end(): - raise Malformed("trailing content") - return "ok", (name, args) - except UnknownTool as e: - return "unknown_tool", str(e) - except Malformed as e: - return "malformed", str(e) - except IndexError: - return "malformed", "index" - - -def ntok(text): - return len(TOK.encode(text, add_special_tokens=False).ids) - - -# --- Candidate sweep ------------------------------------------------------- -# Generic agent-tool names of varying lengths; the real Hermes names are -# unknown, so treat counts as a band, not an exact match. -NAMES = ["search", "read_file", "web_search", "list_files", "get_weather", - "execute_command", "todo-write", "browser.open", "final_answer"] -ALLOWED = set(NAMES) - -candidates = [] - -for name in NAMES: - # 1. Canonical OpenAI JSON drift - candidates += [ - ('canonical json, empty args', f'{{"name": "{name}", "arguments": {{}}}}'), - ('canonical json, compact', f'{{"name":"{name}","arguments":{{}}}}'), - ('canonical json, str args', f'{{"name": "{name}", "arguments": "{{}}"}}'), - ('canonical json, one arg', f'{{"name": "{name}", "arguments": {{"query": "x"}}}}'), - ('canonical json, tool key', f'{{"tool": "{name}", "args": {{}}}}'), - ('bare args object', '{"query": "weather today"}'), - # 2. Native dialect, near misses - ('native, ok empty', f'call:{name}{{}}'), - ('native, ok gemma str', f'call:{name}{{query:<|"|>x<|"|>}}'), - ('native, json-quoted key', f'call:{name}{{"query":"x"}}'), - ('native, capital C', f'Call:{name}{{}}'), - ('native, missing colon', f'call {name}{{}}'), - ('native, paren args', f'call:{name}("x")'), - ('native, single quotes', f"call:{name}{{query:'x'}}"), - ('native, trailing text', f'call:{name}{{}} done'), - ('native, unquoted value', f'call:{name}{{query:x}}'), - ('native, leading-zero num', f'call:{name}{{n:01}}'), - # 3. Other drift formats - ('python style', f'{name}(query="x")'), - ('fenced json', f'```json\n{{"name": "{name}"}}\n```'), - ('tool_code style', f'print({name}(query="x"))'), - ('name colon args', f'{name}: {{"query": "x"}}'), - ] - -print(f'{"verdict":13} {"tok":>3} fits {"kind":28} payload') -print('-' * 100) -rows = [] -for kind, text in candidates: - verdict, detail = parse(text, ALLOWED) - n = ntok(text) - fits = ' *' if 13 <= n <= 15 and verdict == 'malformed' else ' ' - rows.append((verdict, n, fits, kind, text)) - -# Matches first, then by verdict. -for verdict, n, fits, kind, text in sorted(rows, key=lambda r: (r[2] != ' *', r[0])): - print(f'{verdict:13} {n:>3} {fits} {kind:28} {text[:60]!r}') - -# --- Bug demos on valid native calls -------------------------------------- -print('\n--- silent-corruption / latent-bug checks (valid native dialect) ---') -demos = [ - ('space inside json string', 'call:read_file{path:"/tmp/a b c"}'), - ('gemma string keeps space', 'call:read_file{path:<|"|>/tmp/a b c<|"|>}'), - ('hyphenated tool', 'call:todo-write{}'), - ('dotted tool', 'call:browser.open{}'), - ('backslash-space in string', 'call:read_file{path:"a\\ b"}'), -] -for kind, text in demos: - verdict, detail = parse(text, ALLOWED) - print(f'{verdict:13} {ntok(text):>3} {kind:28} {text!r} -> {detail!r}')