From 3178003a4a6418d18200cdfa64aedc03ae7cdfe1 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 3 May 2026 13:22:57 +0800 Subject: [PATCH] polish(estimate_paragraph_for_page): closes #143 #145 #146 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundle C from PR #114 6-AI verify follow-up. Three orthogonal polish items in a single tool, single code path: #143 (P2) — Confidence label calibration Pre-fix: caller-supplied chars_per_page DOWNGRADED confidence to "low" because layoutBasis switched to "caller_chars_per_page" and the medium predicate required "section_properties". Inverted semantics — caller calibration is ground truth, not heuristic noise. Post-fix ladder (confidence + new confidence_reason field): high / caller_provided_chars_per_page — caller calibration trusted medium / default_heuristic_long_doc — paragraph_count >= 10 low / default_heuristic_short_doc — short docs low / page_beyond_estimated_document — out-of-range page low / empty_document — 0 paragraphs Threshold tightened from 3 to 10 paragraphs for medium per verify P2 (3-paragraph docs are below per-paragraph noise floor). #145 (P3) — Error format unification Three calibration errors previously returned plain-text "Error: estimate_paragraph_for_page: ..." while WordError went through MCP runtime as structured. Caller parser had to fork. Post-fix: all 3 invalid-parameter paths return structured JSON via new estimateValidationError helper: { "error": "invalid_parameter", "tool": "estimate_paragraph_for_page", "field": "page" | "chars_per_page" | "context_paragraphs", "reason": "must be 1..100000" | ..., "received": } empty_document path also normalized: now returns method / layout_basis / confidence / confidence_reason fields so callers can parse uniformly without branching on error key first. echo received value also satisfies #129 within this tool's scope. #146 (P3) — Schema description polish Schema description now enumerates the confidence ladder, documents the default chars_per_page derivation formula (charsPerLine * linesPerPage, ≈1189 for A4 / ≈1134 for Letter / 3000-3500 for English IEEE), and lists the JSON error format. Per-arg descriptions added clamp ranges (page 1..100000, chars_per_page 1..200000, context_paragraphs 0..1024). Tests: 7 → 12 in Issue89EstimateParagraphForPageTests - existing 7 updated to assert structured JSON instead of plain-text Error: - testCallerProvidedCharsPerPageGivesHighConfidence (P0 for #143 fix) - testLongDocumentDefaultHeuristicGivesMediumConfidence - testShortDocumentDefaultHeuristicGivesLowConfidence - testBeyondDocumentRequestKeepsLowConfidence - testEmptyDocumentReturnsStructuredErrorWithFullSchema (#145) Full suite: 266 → 271, 0 failures, 9 pre-existing skips. Refs #143 #145 #146 --- Sources/CheWordMCP/Server.swift | 104 ++++++++++-- ...Issue89EstimateParagraphForPageTests.swift | 148 ++++++++++++++++-- 2 files changed, 225 insertions(+), 27 deletions(-) diff --git a/Sources/CheWordMCP/Server.swift b/Sources/CheWordMCP/Server.swift index 1cfb4ef..716585e 100644 --- a/Sources/CheWordMCP/Server.swift +++ b/Sources/CheWordMCP/Server.swift @@ -4119,7 +4119,30 @@ actor WordMCPServer { ), Tool( name: "estimate_paragraph_for_page", - description: "估算 Word UI 第 N 頁大約落在哪些 get_paragraphs 段落索引。OOXML 不儲存頁面邊界；此工具使用 section page size / margins 與字元數啟發式估計，回傳 JSON、confidence 與 warning（支援 Direct Mode）。", + description: """ + 估算 Word UI 第 N 頁大約落在哪些 get_paragraphs 段落索引（支援 Direct Mode）。 + + OOXML 不儲存頁面邊界；此工具用 char-count heuristic + section page size / margins 估計，回傳 JSON 含 estimated_paragraph_range / raw_estimated_paragraph_range / confidence / confidence_reason / warning 等欄位。 + + Confidence ladder（confidence + confidence_reason 兩個欄位一起讀）： + - high / caller_provided_chars_per_page — caller 提供 chars_per_page，視為 ground truth + - medium / default_heuristic_long_doc — 預設 heuristic + paragraph_count >= 10 + - low / default_heuristic_short_doc — 預設 heuristic + 文件較短 + - low / page_beyond_estimated_document — 請求頁碼超過估算總頁數 + - low / empty_document — 文件 0 段落 + + 錯誤回傳格式（JSON）： + - invalid_parameter — 含 field / reason / received，例如 page > 100000、chars_per_page <= 0、context_paragraphs > 1024 + - empty_document — 文件 0 段落（仍回完整 JSON 含 confidence/method 等欄位，error 標 'empty_document'） + + 預設 chars_per_page 推導（內部）： + charsPerLine = max(20, usableWidthTwips / 220) + linesPerPage = max(10, usableHeightTwips / 480) + default = max(400, charsPerLine * linesPerPage) + ≈ A4 default margins → ~1189 chars/page（CJK thesis 友好） + ≈ US Letter default margins → ~1134 chars/page + 英文 IEEE / ACM single-column 約 3000-3500 chars/page，建議用 chars_per_page 校準。 + """, inputSchema: .object([ "type": .string("object"), "properties": .object([ @@ -4133,15 +4156,15 @@ actor WordMCPServer { ]), "page": .object([ "type": .string("integer"), - "description": .string("Word UI 頁碼（1-based；page=1 表示第一頁）") + "description": .string("Word UI 頁碼（1-based；page=1 表示第一頁；上限 100000，超過會回 invalid_parameter 防 Int overflow）") ]), "chars_per_page": .object([ "type": .string("integer"), - "description": .string("可選校準值。若提供，直接使用此每頁字元數；否則依 section page size / margins 估算。") + "description": .string("可選校準值（範圍 1..200000）。若提供，使用此每頁字元數並升 confidence 為 'high'（caller_provided_chars_per_page）；否則依 section page size / margins 估算（CJK thesis ≈ 1189，英文 IEEE 建議 ≈ 3000-3500）。") ]), "context_paragraphs": .object([ "type": .string("integer"), - "description": .string("可選，向前/向後擴張的段落數（預設 2；設 0 可取得純估計範圍）") + "description": .string("可選，向前/向後擴張的段落數（預設 2，範圍 0..1024；設 0 可取得純估計範圍）") ]) ]), "required": .array([.string("page")]) @@ -11571,7 +11594,11 @@ actor WordMCPServer { // from triggering Int overflow trap on caller-controlled Int.max input. // 100_000 pages exceeds any real document by ~3 orders of magnitude. guard page >= 1 && page <= 100_000 else { - return "Error: estimate_paragraph_for_page: page must be 1..100000, got \(page)" + return try Self.estimateValidationError( + field: "page", + reason: "must be 1..100000", + received: page + ) } let charsPerPage: Int @@ -11580,7 +11607,11 @@ actor WordMCPServer { // Same overflow concern: page * charsPerPage with charsPerPage=Int.max. // 200_000 chars/page is far beyond any plausible single-page density. guard override > 0 && override <= 200_000 else { - return "Error: estimate_paragraph_for_page: chars_per_page must be 1..200000, got \(override)" + return try Self.estimateValidationError( + field: "chars_per_page", + reason: "must be 1..200000", + received: override + ) } charsPerPage = override layoutBasis = "caller_chars_per_page" @@ -11593,16 +11624,23 @@ actor WordMCPServer { // Upper bound prevents rawStart - contextParagraphs underflow and // rawEnd + contextParagraphs overflow on Int.max input. guard contextParagraphs >= 0 && contextParagraphs <= 1024 else { - return "Error: estimate_paragraph_for_page: context_paragraphs must be 0..1024, got \(contextParagraphs)" + return try Self.estimateValidationError( + field: "context_paragraphs", + reason: "must be 0..1024", + received: contextParagraphs + ) } let paragraphs = doc.getParagraphs() guard !paragraphs.isEmpty else { return try Self.renderJSONString([ "error": "empty_document", + "reason": "document has 0 paragraphs", "estimated_paragraph_range": [], "confidence": "low", + "confidence_reason": "empty_document", "method": "char_count_heuristic", + "layout_basis": layoutBasis, "assumed_chars_per_page": charsPerPage, "warning": Self.pageEstimateWarning, ]) @@ -11634,14 +11672,41 @@ actor WordMCPServer { let startIndex = max(0, rawStart - contextParagraphs) let endIndex = min(paragraphs.count - 1, rawEnd + contextParagraphs) - let confidence = (!beyondEstimatedDocument && layoutBasis == "section_properties" && paragraphs.count >= 3) - ? "medium" - : "low" + + // Confidence semantics (see schema description for the full ladder): + // + // high — caller provided a calibrated chars_per_page; we trust their + // ground-truth knowledge of their document over the heuristic. + // medium — default heuristic on section-derived layout with enough + // paragraphs (≥10) to smooth out per-paragraph noise. + // low — short docs, beyond-document extrapolation, fallback layout, + // or empty document. + // + // Pre-fix (#143): caller-supplied chars_per_page was DOWNGRADED to "low" + // because layoutBasis switched to "caller_chars_per_page", failing the + // medium predicate. Inverted semantics — caller calibration is the most + // reliable input we have. Fixed: caller-provided is now "high". + let confidence: String + let confidenceReason: String + if beyondEstimatedDocument { + confidence = "low" + confidenceReason = "page_beyond_estimated_document" + } else if layoutBasis == "caller_chars_per_page" { + confidence = "high" + confidenceReason = "caller_provided_chars_per_page" + } else if paragraphs.count >= 10 { + confidence = "medium" + confidenceReason = "default_heuristic_long_doc" + } else { + confidence = "low" + confidenceReason = "default_heuristic_short_doc" + } return try Self.renderJSONString([ "estimated_paragraph_range": [startIndex, endIndex], "raw_estimated_paragraph_range": [rawStart, rawEnd], "confidence": confidence, + "confidence_reason": confidenceReason, "method": "char_count_heuristic", "layout_basis": layoutBasis, "page": page, @@ -11655,6 +11720,25 @@ actor WordMCPServer { ]) } + /// Build the structured-JSON validation-error response shared by all + /// `estimate_paragraph_for_page` argument guards. Replaces the previous + /// plain-text `"Error: estimate_paragraph_for_page: ..."` strings (#145). + /// `received` is echoed so an LLM caller can self-correct without guessing + /// what it sent (#129 same pattern as PR #115's match_options validators). + private static func estimateValidationError( + field: String, + reason: String, + received: Int + ) throws -> String { + return try renderJSONString([ + "error": "invalid_parameter", + "tool": "estimate_paragraph_for_page", + "field": field, + "reason": reason, + "received": received, + ]) + } + private static let pageEstimateWarning = "OOXML does not store page boundaries; this is an estimate based on character density and section page setup. Actual page boundaries depend on rendering, fonts, margins, line spacing, images, tables, and Word layout." private static func estimatedLayoutCharacterCount(for paragraph: Paragraph) -> Int { diff --git a/Tests/CheWordMCPTests/Issue89EstimateParagraphForPageTests.swift b/Tests/CheWordMCPTests/Issue89EstimateParagraphForPageTests.swift index fd2325a..476bddc 100644 --- a/Tests/CheWordMCPTests/Issue89EstimateParagraphForPageTests.swift +++ b/Tests/CheWordMCPTests/Issue89EstimateParagraphForPageTests.swift @@ -69,7 +69,10 @@ final class Issue89EstimateParagraphForPageTests: XCTestCase { "page": .int(0) ] ) - XCTAssertTrue(textOf(invalidPage).contains("page must be")) + let invalidPageObj = try jsonObject(from: textOf(invalidPage)) + XCTAssertEqual(invalidPageObj["error"] as? String, "invalid_parameter") + XCTAssertEqual(invalidPageObj["field"] as? String, "page") + XCTAssertEqual((invalidPageObj["received"] as? NSNumber)?.intValue, 0) let invalidCalibration = await server.invokeToolForTesting( name: "estimate_paragraph_for_page", @@ -79,7 +82,10 @@ final class Issue89EstimateParagraphForPageTests: XCTestCase { "chars_per_page": .int(0) ] ) - XCTAssertTrue(textOf(invalidCalibration).contains("chars_per_page must be")) + let invalidCalibrationObj = try jsonObject(from: textOf(invalidCalibration)) + XCTAssertEqual(invalidCalibrationObj["error"] as? String, "invalid_parameter") + XCTAssertEqual(invalidCalibrationObj["field"] as? String, "chars_per_page") + XCTAssertEqual((invalidCalibrationObj["received"] as? NSNumber)?.intValue, 0) } // MARK: - Int.max overflow regression (P1 from 6-AI verify) @@ -87,7 +93,8 @@ final class Issue89EstimateParagraphForPageTests: XCTestCase { func testEstimateParagraphForPageRejectsHugePage() async throws { // Pre-fix: `(page - 1) * charsPerPage` and `page * charsPerPage` with // page = Int.max trapped on arithmetic overflow → MCP server actor - // crashed. Post-fix clamps page to 1..100_000. + // crashed. Post-fix clamps page to 1..100_000 and returns a structured + // invalid_parameter JSON error (#145 unification). let url = try docxWithFixedParagraphs(count: 1, charsPerParagraphBeforeBreak: 10) defer { try? FileManager.default.removeItem(at: url) } @@ -99,11 +106,12 @@ final class Issue89EstimateParagraphForPageTests: XCTestCase { "page": .int(.max) ] ) - let text = textOf(result) - XCTAssertTrue( - text.contains("page must be") && text.contains("100000"), - "expected structured upper-bound rejection, got: \(text)" - ) + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["error"] as? String, "invalid_parameter") + XCTAssertEqual(obj["field"] as? String, "page") + XCTAssertEqual(obj["reason"] as? String, "must be 1..100000") + // received echoes the offending value so an LLM caller can self-correct (#129). + XCTAssertEqual((obj["received"] as? NSNumber)?.intValue, .max) } func testEstimateParagraphForPageRejectsHugeCharsPerPage() async throws { @@ -120,11 +128,10 @@ final class Issue89EstimateParagraphForPageTests: XCTestCase { "chars_per_page": .int(.max) ] ) - let text = textOf(result) - XCTAssertTrue( - text.contains("chars_per_page must be") && text.contains("200000"), - "expected structured upper-bound rejection, got: \(text)" - ) + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["error"] as? String, "invalid_parameter") + XCTAssertEqual(obj["field"] as? String, "chars_per_page") + XCTAssertEqual(obj["reason"] as? String, "must be 1..200000") } func testEstimateParagraphForPageRejectsHugeContextParagraphs() async throws { @@ -142,11 +149,118 @@ final class Issue89EstimateParagraphForPageTests: XCTestCase { "context_paragraphs": .int(.max) ] ) - let text = textOf(result) - XCTAssertTrue( - text.contains("context_paragraphs must be") && text.contains("1024"), - "expected structured upper-bound rejection, got: \(text)" + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["error"] as? String, "invalid_parameter") + XCTAssertEqual(obj["field"] as? String, "context_paragraphs") + XCTAssertEqual(obj["reason"] as? String, "must be 0..1024") + } + + // MARK: - Confidence label calibration (#143) + + func testCallerProvidedCharsPerPageGivesHighConfidence() async throws { + // Pre-fix: caller-supplied chars_per_page DOWNGRADED confidence to + // "low" because layoutBasis switched to "caller_chars_per_page" and + // the medium predicate required "section_properties". Inverted — + // caller calibration is the most reliable input we have. + let url = try docxWithFixedParagraphs(count: 5, charsPerParagraphBeforeBreak: 200) + defer { try? FileManager.default.removeItem(at: url) } + + let server = await WordMCPServer() + let result = await server.invokeToolForTesting( + name: "estimate_paragraph_for_page", + arguments: [ + "source_path": .string(url.path), + "page": .int(1), + "chars_per_page": .int(500) + ] + ) + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["confidence"] as? String, "high") + XCTAssertEqual(obj["confidence_reason"] as? String, "caller_provided_chars_per_page") + XCTAssertEqual(obj["layout_basis"] as? String, "caller_chars_per_page") + } + + func testLongDocumentDefaultHeuristicGivesMediumConfidence() async throws { + // 12 paragraphs (>= 10 threshold) on default heuristic → medium. + let url = try docxWithFixedParagraphs(count: 12, charsPerParagraphBeforeBreak: 200) + defer { try? FileManager.default.removeItem(at: url) } + + let server = await WordMCPServer() + let result = await server.invokeToolForTesting( + name: "estimate_paragraph_for_page", + arguments: [ + "source_path": .string(url.path), + "page": .int(1) + ] + ) + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["confidence"] as? String, "medium") + XCTAssertEqual(obj["confidence_reason"] as? String, "default_heuristic_long_doc") + } + + func testShortDocumentDefaultHeuristicGivesLowConfidence() async throws { + // 3 paragraphs (< 10 threshold) on default heuristic → low. + let url = try docxWithFixedParagraphs(count: 3, charsPerParagraphBeforeBreak: 100) + defer { try? FileManager.default.removeItem(at: url) } + + let server = await WordMCPServer() + let result = await server.invokeToolForTesting( + name: "estimate_paragraph_for_page", + arguments: [ + "source_path": .string(url.path), + "page": .int(1) + ] + ) + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["confidence"] as? String, "low") + XCTAssertEqual(obj["confidence_reason"] as? String, "default_heuristic_short_doc") + } + + func testBeyondDocumentRequestKeepsLowConfidence() async throws { + // page beyond estimated total → "low" + page_beyond_estimated_document + // regardless of layout basis. Caller calibration cannot rescue an + // out-of-range page. + let url = try docxWithFixedParagraphs(count: 5, charsPerParagraphBeforeBreak: 50) + defer { try? FileManager.default.removeItem(at: url) } + + let server = await WordMCPServer() + let result = await server.invokeToolForTesting( + name: "estimate_paragraph_for_page", + arguments: [ + "source_path": .string(url.path), + "page": .int(99), + "chars_per_page": .int(100) + ] + ) + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["confidence"] as? String, "low") + XCTAssertEqual(obj["confidence_reason"] as? String, "page_beyond_estimated_document") + XCTAssertEqual(obj["requested_page_beyond_estimated_document"] as? Bool, true) + } + + // MARK: - Empty document path documented (#145) + + func testEmptyDocumentReturnsStructuredErrorWithFullSchema() async throws { + // Empty doc still returns method / layout_basis / confidence so callers + // can parse uniformly without branching on error key first. + let url = try docxWithFixedParagraphs(count: 0, charsPerParagraphBeforeBreak: 0) + defer { try? FileManager.default.removeItem(at: url) } + + let server = await WordMCPServer() + let result = await server.invokeToolForTesting( + name: "estimate_paragraph_for_page", + arguments: [ + "source_path": .string(url.path), + "page": .int(1) + ] ) + let obj = try jsonObject(from: textOf(result)) + XCTAssertEqual(obj["error"] as? String, "empty_document") + XCTAssertEqual(obj["confidence"] as? String, "low") + XCTAssertEqual(obj["confidence_reason"] as? String, "empty_document") + XCTAssertNotNil(obj["method"]) + XCTAssertNotNil(obj["layout_basis"]) + XCTAssertNotNil(obj["warning"]) } func testEstimateParagraphForPageSchemaDocumentsHeuristicWarning() throws {