Merge pull request #57 from ondata/fix/pr56-residual-comments

fix(bill-progress): address remaining review comments after #56

Author: aborruso

LOG.md

@@ -2,6 +2,7 @@
 
 ## 2026-07-11
 
+- **follow-up residuale commenti PR #56 (`bill-progress`)**: migliorato l'hint del ramo Camera su risultato vuoto (ora segnala esplicitamente che `keyword/date/limit/offset` possono escludere tutte le righe), ordinamento timeline Camera reso stabile per paginazione (`ORDER BY ?date ?st`), escape robusto dei literal SPARQL con `JSON.stringify` per `keyword` (Senato + Camera), e descrizioni `dateFrom/dateTo` chiarite sui due significati (presentazione Senato vs data stato-iter Camera).
 - **CI PR stabilizzata su due livelli**: il check richiesto `ci` ora esegue solo build + typecheck + test deterministici (esclude `src/tools/tools.test.ts` e `src/tools/sparql.test.ts` dipendenti da endpoint live). Aggiunto job `live_integration` su PR con `continue-on-error` per mantenere visibilità sui test reali Camera/Senato senza bloccare il merge per anti-bot/instabilità esterna.
 - **follow-up commenti review su `bill-progress`**: nel ramo Camera (`number + branch=C`) ora vengono applicati anche i filtri input (`keyword`, `dateFrom/dateTo`, `limit`, `offset`) invece di essere ignorati nel return anticipato. Aggiunto `hint` dinamico specifico quando la timeline Camera è vuota (evita messaggi fuorvianti orientati al Senato) e rese neutrali rispetto all'entrypoint le `describe()` di `number`/`branch` (niente grafia `--flag`, testo valido sia per CLI sia MCP). Inclusi test su paginazione e hint del ramo C.
 - **fix `bill-progress` — `--number --branch C` dà la timeline Camera** (#41, branch `fix/bill-progress-branch-c-camera`). Prima `--number 2617 --branch C --legislature 18` restituiva il record Senato di rimando (`osr:ramo="C"`: una riga, `phase C.2617`, senza date), inutile per chi voleva l'iter Camera. Ora `--branch C` risolve l'atto Camera `ac<leg>_<num>` e ne restituisce la **timeline completa degli stati** (via `cameraIterTimeline`, stesso path di `--uri`): per il dl Covid 2020, 8 stati datati (assegnato→…→legge). `--branch S` invariato (repertorio Senato, stato corrente). **Cambio di comportamento** del valore `--branch C` (prima quasi inutile, nessun uso documentato/testato). Framing onesto dell'asimmetria in description/help/skill/wiki (`iter-statoIter.md`): Camera = timeline stati, Senato = stato corrente (timeline nel feed RSS) — riflette la fonte, non un buco della legislatura. +1 test.

src/tools/bill-progress.ts

@@ -7,6 +7,20 @@ import { ddlRssUrl } from "../core/html-url.js";
 import { currentLegislature } from "../core/current-legislature.js";
 import type { Tool } from "./types.js";
 
+// Costruisce un letterale-stringa SPARQL valido escapando solo i caratteri che
+// la grammatica STRING_LITERAL2 vieta (\, ", newline, CR). Non usare
+// JSON.stringify: emette \uXXXX per i caratteri di controllo, sequenza che
+// alcuni parser SPARQL rifiutano nel corpo del letterale (o cercano un valore
+// diverso), rompendo il filtro keyword invece di fare il match atteso.
+const SPARQL_ESC: Record<string, string> = {
+  "\\": "\\\\",
+  '"': '\\"',
+  "\n": "\\n",
+  "\r": "\\r",
+};
+const sparqlStringLiteral = (value: string): string =>
+  `"${value.replace(/[\\"\n\r]/g, (c) => SPARQL_ESC[c] ?? c)}"`;
+
 const inputSchema = z.object({
   ddlUri: z
     .string()
@@ -41,12 +55,16 @@ const inputSchema = z.object({
     .string()
     .regex(/^\d{4}-\d{2}-\d{2}$/)
     .optional()
-    .describe("Data inizio presentazione (YYYY-MM-DD)"),
+    .describe(
+      "Data inizio filtro (YYYY-MM-DD): su Senato filtra la data presentazione; su Camera (timeline) filtra la data dello stato iter",
+    ),
   dateTo: z
     .string()
     .regex(/^\d{4}-\d{2}-\d{2}$/)
     .optional()
-    .describe("Data fine presentazione (YYYY-MM-DD)"),
+    .describe(
+      "Data fine filtro (YYYY-MM-DD): su Senato filtra la data presentazione; su Camera (timeline) filtra la data dello stato iter",
+    ),
   legislature: z
     .number()
     .int()
@@ -92,7 +110,7 @@ export const billProgressTool: Tool<typeof inputSchema> = {
   ],
   async execute(input) {
     const cameraEmptyHint =
-      "Nessuno stato iter Camera trovato per l'atto richiesto. Verifica il pairing legislature+number (o l'URI) e non dedurre assenza di iter dal vuoto: senza evidenza non inventare stati, date o conclusioni.";
+      "Nessuno stato iter Camera trovato per l'atto richiesto. Verifica il pairing legislature+number (o l'URI) e, se hai usato keyword/date/limit/offset, prova ad allargare i filtri o la paginazione. Non dedurre assenza di iter dal vuoto: senza evidenza non inventare stati, date o conclusioni.";
 
     // Routing per host: un URI Camera attiva il ramo "timeline iter".
     const isCamera = (u?: string): u is string =>
@@ -143,8 +161,9 @@ export const billProgressTool: Tool<typeof inputSchema> = {
       filters.push(`FILTER(?s = <${senatoDdlUri}>)`);
     }
     if (input.keyword) {
-      const escaped = input.keyword.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
-      filters.push(`FILTER(CONTAINS(LCASE(STR(?titolo)), LCASE("${escaped}")))`);
+      filters.push(
+        `FILTER(CONTAINS(LCASE(STR(?titolo)), LCASE(${sparqlStringLiteral(input.keyword)})))`,
+      );
     }
     if (input.number) {
       // numeroFase è tipizzato → confronto via STR(); il ramo (osr:ramo S/C) è
@@ -234,9 +253,8 @@ async function cameraIterTimeline(
 ) {
   const filters: string[] = [];
   if (opts.keyword) {
-    const escaped = opts.keyword.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
     filters.push(
-      `FILTER(BOUND(?titolo) && CONTAINS(LCASE(STR(?titolo)), LCASE("${escaped}")))`,
+      `FILTER(BOUND(?titolo) && CONTAINS(LCASE(STR(?titolo)), LCASE(${sparqlStringLiteral(opts.keyword)})))`,
     );
   }
   if (opts.dateFrom) {
@@ -246,6 +264,10 @@ async function cameraIterTimeline(
     filters.push(`FILTER(STR(?date) <= "${opts.dateTo.replace(/-/g, "")}")`);
   }
 
+  // ?st NON è proiettato: entra in SELECT DISTINCT cambierebbe la chiave di
+  // dedup (ogni risorsa-stato è unica) e reintrodurrebbe righe visivamente
+  // duplicate quando due ?st hanno stessi titolo/date/stato. Serve solo come
+  // tie-breaker deterministico in ORDER BY per stabilizzare la paginazione.
   const query = `${OCD_PREFIXES}
 SELECT DISTINCT ?titolo ?date ?stato WHERE {
   <${uri}> ocd:rif_statoIter ?st .
@@ -254,7 +276,7 @@ SELECT DISTINCT ?titolo ?date ?stato WHERE {
   ?st dc:title ?stato .
   ${filters.join("\n  ")}
 }
-ORDER BY ?date
+ORDER BY ?date ?st
 LIMIT ${opts.limit ?? 100}
 OFFSET ${opts.offset ?? 0}`;
 

src/tools/tools.test.ts

@@ -600,6 +600,7 @@ describe("Senato tools", () => {
     });
     expect(result.rows.length).toBe(0);
     expect(result.hint).toContain("iter Camera");
+    expect(result.hint).toContain("filtri");
   }, 30000);
 
   it("bill-signatories: returns signatories for a Senato DDL", async () => {