Repository navigation
Conversation
Clippy 0.1.98 flags two new lints that break the CI step
`cargo clippy --all-targets -- -D warnings` on stable:
- chunks_exact_to_as_chunks (attachments.rs): iterate the
/Names leaf with `as_chunks::<2>()`. The trailing odd element
is still skipped, as with `chunks_exact(2)`.
- collapsible_match (layout.rs): move the "no trailing space"
check into the TJ match guards. A failing guard falls through
to `_ => {}`, which is what the inner `if` did.
No behaviour change. `as_chunks` needs Rust 1.88 or newer.
docugraph ships a binary, so the lockfile belongs in the repo. Without it every build resolves dependencies again and a fixed commit does not pin rmcp, lopdf, tokio and the rest. The lock is the one resolved for 2832efa (182 packages, rmcp 3.3.0, lopdf 0.45.0, tokio 1.53.1) and matches Cargo.toml under `--locked`.
test_real_pdf_loading_and_extraction read a hardcoded Windows path and returned early when it was missing, so it reported "ok" without testing anything on every machine but one. Read the path from DOCUGRAPH_TEST_PDF instead and mark the test #[ignore] with a reason, so it shows up as ignored by default. Run it with `DOCUGRAPH_TEST_PDF=... cargo test -- --ignored`; a missing variable or file now fails loudly.
Now that Cargo.lock is tracked, fail CI when it is out of date with Cargo.toml instead of silently rewriting it. There is no separate build step: clippy is the first step that resolves dependencies and `cargo test` builds the crate, so both get --locked. Without it on clippy the lock would be rewritten before the test step runs.
DocuGraphServer::new() reads DOCUGRAPH_CACHE_DIR and falls back to a relative .docugraph_cache, so the protocol and scanned-page tests wrote cache JSON into the working directory on every run. Add DocuGraphServer::with_cache_dir and point those tests at a tempdir.
Tools returned plain strings, so rmcp wrapped every failure as a successful result: an unknown document, section, page or attachment came back with isError=false, and document_info even built a fake "Document Not Found" object with zero pages. Add ToolError, which becomes CallToolResult::error, and return Result<String, ToolError> from every tool that looks something up. Not-found errors name what exists instead: the available document ids or the document's attachments. A page number or page filter outside the document is an error too.
document_search, document_search_hybrid, document_get_context and document_get_evidence fell back to every indexed document when the document_id did not exist, so a typo in an id with accents returned hits from another book without any warning. An unknown document_id is now a tool error that lists the available ids. Without document_id the tools still search every document.
document_read_pages walked every number from page_start to page_end, looked each one up with a linear scan and only stopped once max_chars was reached. With page_end=4294967295 and a start near the end of a real 224-page book the call did not answer within 10 s and would have held a core for about 5-6 minutes. page_start=0 or an inverted range returned an empty header instead of an error. - Validate 1 <= page_start <= page_end, clamp page_end to the last page and return a tool error when the range holds no pages. - Add Document::pages_in_range, which walks the extracted pages. Use it in read_pages and in section expansion, where a malformed outline can also claim that a section ends at u32::MAX. - Cap limit (50), max_tokens (16000), max_chunks and max_items (20), max_chars (64000) and max_bytes (1 MiB), and document the caps in the tool schemas. - Use saturating_mul for limit * 3 and max_chunks * 2, which panicked in debug builds and wrapped around in release.
Without DOCUGRAPH_CACHE_DIR the cache is .docugraph_cache relative to the working directory, and MCP clients start servers from the project folder. The server then saw no documents: document_list answered [] and searches said "No documents available" without saying where they had looked. - DiskCache stores its directory as an absolute path and DocumentStore exposes it. - document_list, and the search tools with an empty store, name that directory and DOCUGRAPH_CACHE_DIR. For the search tools an empty store is a tool error. - docugraph serve prints a WARNING on stderr when the configured directory is relative, cannot be opened or holds no documents.
The MCP tests called the tool methods directly, so nothing checked the JSON-RPC framing, the isError flag that clients actually receive or that stdout stays free of logs. tests/mcp_stdio_test.rs spawns the binary (CARGO_BIN_EXE_docugraph) with a temporary cache, runs initialize and tools/list, and calls tools that fail: an unknown document, an unknown document_id in a search and a page range past the end. It checks isError, that page_end=u32::MAX answers quickly, that every stdout line is a JSON-RPC 2.0 message and that serve warns on stderr about a relative, empty cache.
Document the isError behaviour, the size caps and the page_end clamp in the README tool section, set DOCUGRAPH_CACHE_DIR in both client configuration examples and explain why it must be absolute. Add an Unreleased entry to the CHANGELOG.
Owner
Author
|
Se descarta: los arreglos de docugraph los está haciendo el autor en su copia local y estos cambios chocarían con ellos. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problema
Medido con el binario instalado de
main(2832efa) y dos libros reales indexados (224 y 437 páginas), por stdio con un cliente JSON-RPC:document_read_pagespodía dejar un núcleo al 100 % durante minutos. Recorría cada número depage_startapage_end, buscaba cada página con un recorrido lineal y solo paraba al llenarmax_chars. Conpage_start=223ypage_end=4294967295no respondió en 10 s; conpage_end=20000000tardó 1,6 s, así queu32::MAXserían unos 5-6 minutos.page_start=0o un rango invertido devolvían una cabecera vacía sin error.isError: falsey un texto o un JSON{"error": ...}.document_infoinventaba un objeto «Document Not Found» contotal_pages: 0. Un agente o un hook que mireisErrorno los distingue de una respuesta buena.document_idcon errata buscaba en todos los documentos endocument_search,document_search_hybrid,document_get_contextydocument_get_evidence, sin avisar.limit,max_tokens,max_chunks/max_items,max_charsymax_bytesno tenían máximo, ylimit * 3se desbordaba (pánico en debug, vuelta silenciosa en release).DOCUGRAPH_CACHE_DIR,serveusa.docugraph_cacherelativa al directorio de trabajo, que en un cliente MCP suele ser la carpeta del proyecto.document_listrespondía[], las búsquedas decían «No documents available» sin decir dónde habían buscado, yserveno avisaba..docugraph_cachedentro del repo.Cambios
test: keep server test caches out of the repoDocuGraphServer::with_cache_dir.mcp_protocol_testyscanned_document_detection_testusan untempdiren vez deDocuGraphServer::new(), que escribía la caché en el directorio de trabajo.fix(mcp): return isError for domain errorsToolError(src/mcp/error.rs), que se envía comoCallToolResult::error. Las herramientas que buscan algo devuelvenResult<String, ToolError>. Si el documento no existe, el error lista los ids disponibles; si no existe el adjunto, lista los adjuntos. Una página fuera de rango enrender_page,get_linksoget_formsda error con el rango válido. Desaparece el objeto falso dedocument_info.fix(mcp): reject unknown document_id in searchget_documentsdevuelve error con la lista de ids si eldocument_idno existe. Sindocument_idsigue buscando en todos.fix(mcp): bound page ranges and tool limitsdocument_read_pagesvalida1 <= page_start <= page_end, ajustapage_enda la última página (y lo dice en la respuesta) y da error si el rango queda vacío.Document::pages_in_rangerecorre solo las páginas extraídas; lo usanread_pagesy la expansión de secciones, porque un outline roto también puede decir que una sección acaba enu32::MAX. Topes:limit50,max_tokens16000,max_chunks/max_items20,max_chars64000 ymax_bytes1 MiB; los valores se ajustan a1..=topey los topes figuran en el esquema de cada herramienta.saturating_mulenlimit * 3ymax_chunks * 2.fix(serve): explain an empty or misplaced cacheDiskCacheguarda su carpeta como ruta absoluta yDocumentStore::cache_dirla expone.document_listsin documentos, y las búsquedas con la caché vacía (estas conisError), dicen en qué carpeta buscaron y que se fija conDOCUGRAPH_CACHE_DIR.serveescribeWARNING:en stderr si la carpeta es relativa, no se puede abrir o no tiene documentos.test: drive docugraph serve over stdiotests/mcp_stdio_test.rslanzaenv!("CARGO_BIN_EXE_docugraph") servecon una caché temporal y haceinitialize,tools/listytools/callque fallan: documento inexistente,document_idcon errata y rango pasado el final. CompruebaisError, quepage_end=u32::MAXresponde enseguida, que cada línea de stdout es un mensaje JSON-RPC 2.0 y los avisos de stderr con una caché relativa y vacía.docs: describe tool errors, limits and cache dirDOCUGRAPH_CACHE_DIRen los dos ejemplos de configuración y por qué debe ser absoluta. EntradaUnreleaseden el CHANGELOG.Sin dependencias nuevas:
std::path::absolutees de la biblioteca estándar ytempfileya estaba endev-dependencies.Cómo se verificó
En local, con rustc/cargo 1.98.1, clippy 0.1.98 y rustfmt 1.9.0:
cargo fmt --all -- --checkcargo clippy --locked --all-targets -- -D warningscargo test --locked, los 14 objetivos de integración más--lib --binsy--doc--ignored)El
/tmpde la máquina donde compilé es un tmpfs casi lleno, así que en local compilé en perfil dev sin información de depuración (CARGO_PROFILE_DEV_DEBUG=0,strip) y un objetivo de test cada vez. El CI de esta PR ejecuta la suite normal concargo test --all --locked.Tests nuevos, además de adaptar los existentes a
ToolResult:mcp_tools_suite_test: errores de dominio, conversión aisError: trueen el JSON que sale por el cable,document_idcon errata en las cuatro búsquedas,page_end=u32::MAXen menos de 1 s, rangos inválidos,usize::MAXen todos los tamaños y una sección conpage_end=u32::MAX.mcp_protocol_test: mensaje de caché vacía con la ruta absoluta, almacén en memoria y avisos de arranque.retrieval_hybrid_test:searchconlimit=usize::MAXsin desbordamiento.multimodal_render_test,links_and_outlines_testyattachments_and_embedded_files_test: página y adjunto inexistentes.mcp_stdio_test: los dos tests por stdio descritos arriba.Batería con PDFs reales
Indexé dos libros técnicos reales (224 y 437 páginas; licencia personal, no se suben al repo) con
docugraph indexen una caché temporal (DOCUGRAPH_CACHE_DIR). Lancé la misma batería de llamadas MCP por stdio contra el binario instalado demainy contra un build release de esta rama, sin otros procesos pesados en marcha. Los ids de los libros salen como<libro-224p>y<libro-437p>. Cuando una llamada no responde en 10 s, el cliente mata el servidor y sigue con uno nuevo.Cifras de antes y después
/usr/local/bin/docugraph, 2832efa, release)document_read_pages223..4294967295 (libro de 224 págs.)document_read_pages300..4294967295isError: true: «Page range 300-4294967295 is empty: document '' has 224 pages.»document_read_pages223..20000000document_read_pages1..4294967295,max_chars1500max_chars), cabecera «pp. 1-4294967295»document_read_pagespage_start=0isError: false, cabecera vacíaisError: true: «page_start must be at least 1: pages are numbered from 1.»document_read_pagesrango invertido 5..2isError: false, cabecera vacíaisError: true: «Invalid page range 5-2: page_start must not be greater than page_end.»document_infocon id inexistenteisError: false, objeto «Document Not Found» contotal_pages: 0isError: true: «Document 'no-existe' not found. Available document ids: '', ''.»isError: true(info, outline, sección, render, links de página 9999, adjuntos, adjunto)document_get_linksconpage: 9999isError: false, lista vacíaisError: true: «Page 9999 does not exist in document '': valid pages are 1-224.»document_idcon errata en search, search_hybrid, get_context y get_evidenceisError: truey la lista de idsdocument_search_hybridconlimit= usize::MAXdocument_searchconlimit= usize::MAXdocument_get_contextconmax_chunksymax_tokens= usize::MAXdocument_get_evidenceconmax_itemsymax_tokens= usize::MAXdocument_read_pages1..437 conmax_chars= usize::MAXdocument_search_hybridnormal (sindocument_id,limit3)document_listsinDOCUGRAPH_CACHE_DIR, desde una carpeta de proyecto vacía[]docugraph index <pdf>and set DOCUGRAPH_CACHE_DIR to that same absolute directory for bothindexandserve.»document_searchcon la caché vacíaisError: false: «No documents available for search.»isError: true, mismo texto quedocument_listserveen stderr con la caché relativa y vacía: antes ninguno; después dos líneasWARNING:(carpeta relativa, con su ruta absoluta, y carpeta sin documentos). Con la caché absoluta y los libros: ninguno.document_search_hybridsindocument_idpasa de 314 a 337 ms (ruido entre ejecuciones) ydocument_get_contextcon los topes tarda 184 ms. Undocument_idinexistente responde en 6-7 ms, porque para listar los ids lee los metadatos de la caché.Cambios de comportamiento a tener en cuenta
"error"en el JSON o el prefijoError:en el texto debe mirarisError; el mensaje va encontent[0].text.document_listsin documentos devuelve un texto explicativo en vez de[].1..=tope:limit: 0devuelve ahora un resultado.DocuGraphServerdevuelvenToolResulten vez deString, salvodocument_pingydocument_list.DOCUGRAPH_CACHE_DIR,servesigue creando.docugraph_cacheen el directorio de trabajo (ahora avisa), ydocument_render_pagesigue devolviendo el PNG como texto base64. Quedan para otras PR.