-
Notifications
You must be signed in to change notification settings - Fork 0
fix(cli): emptyHint statico dei tool ora arriva anche via CLI #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
2 changes: 2 additions & 0 deletions
2
openspec/changes/archive/2026-07-11-fix-cli-emptyhint-fallback/.openspec.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-07-11 |
27 changes: 27 additions & 0 deletions
27
openspec/changes/archive/2026-07-11-fix-cli-emptyhint-fallback/design.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| ## Context | ||
|
|
||
| I tool del progetto espongono opzionalmente un `emptyHint?: string` statico (definito in `src/tools/*.ts`, tipizzato in `src/tools/types.ts`) e possono restituire un `hint?: string` dinamico nel `ToolResult`. Il server MCP li unisce con la precedenza `result.hint ?? emptyHint ?? DEFAULT_EMPTY` (`src/server.ts`). La CLI invece, in `emit()` (`src/cli.ts`), scrive su stderr solo `result.hint`, ignorando l'`emptyHint` statico. `emit(result, format)` non riceve il tool, quindi non ha accesso all'`emptyHint`; viene inoltre invocato da ~50 call site, ciascuno preceduto da `runTool(<tool>, input)`. `runTool()` invece riceve già l'oggetto tool. | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
| - Allineare la CLI al path MCP: su risultato vuoto senza hint dinamico, comunicare l'`emptyHint` statico del tool su stderr. | ||
| - Fix a punto singolo, senza modificare i ~50 call site di `emit()`. | ||
| - Preservare stdout parsabile (CSV/JSONL) ed exit code. | ||
|
|
||
| **Non-Goals:** | ||
| - Modificare le stringhe di `emptyHint` esistenti o aggiungerne di nuove. | ||
| - Cambiare il comportamento del server MCP (già corretto). | ||
| - Introdurre un `DEFAULT_EMPTY` lato CLI: se non c'è né hint dinamico né `emptyHint`, la CLI resta silenziosa su stderr (comportamento attuale). | ||
|
|
||
| ## Decisions | ||
|
|
||
| - **Applicare il fallback in `runTool()`, non in `emit()`.** `runTool()` ha già il tool in mano; `emit()` no. Allargare la firma di `runTool` per includere `emptyHint?: string` e, dopo `tool.execute(parsed)`, se `result.rows.length === 0 && !result.hint && tool.emptyHint`, restituire `{ ...result, hint: tool.emptyHint }`. `emit()` resta invariato: già scrive `result.hint` su stderr quando il risultato è vuoto. Un solo punto di modifica, nessun tocco ai call site. | ||
| - **Precedenza identica al server**: l'hint dinamico vince sull'`emptyHint` statico (`result.hint ?? emptyHint`), replicata dalla guardia `!result.hint`. | ||
| - **Immutabilità del risultato**: si restituisce un nuovo oggetto (`{ ...result, hint }`) invece di mutare `result`, coerente con lo stile del codice. | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **Rischio basso**: la modifica tocca solo il ramo "risultato vuoto"; il flusso con righe è invariato. stdout ed exit code non cambiano. | ||
| - **Trade-off**: si aggiunge `emptyHint?` alla firma inline di `runTool`; accettabile e coerente con `Tool<>` in `types.ts`. In alternativa si sarebbe potuto tipizzare `runTool` sul tipo `Tool`, ma la firma inline attuale è minima e la si estende di un solo campo per non allargare la superficie del cambiamento. | ||
| - **Verifica**: serve un test che copra i quattro casi (vuoto+emptyHint→stderr; vuoto+hint dinamico→precede; non vuoto→niente; vuoto+niente→silenzio) e la parità con MCP, per evitare regressioni future su questo scollamento. | ||
25 changes: 25 additions & 0 deletions
25
openspec/changes/archive/2026-07-11-fix-cli-emptyhint-fallback/proposal.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| ## Why | ||
|
|
||
| Sei tool definiscono un `emptyHint` statico — un messaggio che spiega perché un risultato potrebbe essere vuoto (es. gap noto del dataset, filtro da riformulare). Il server MCP lo mostra (`src/server.ts` usa `result.hint ?? emptyHint ?? DEFAULT_EMPTY`), ma la CLI no: `emit()` in `src/cli.ts` scrive solo l'hint dinamico (`result.hint`) e non arriva mai all'`emptyHint` statico del tool. Chi usa la CLI su una finestra vuota (caso reale: le fiducie COVID 2020 assenti dal LOD Senato) riceve un output vuoto, exit code 0, senza alcuna spiegazione — pur avendola già scritta nel codice. È uno scollamento CLI/MCP che viola la convenzione di progetto "CLI e MCP allineati". | ||
|
|
||
| ## What Changes | ||
|
|
||
| - La CLI, quando un risultato è vuoto e il tool non fornisce un hint dinamico, usa come fallback l'`emptyHint` statico del tool e lo scrive su stderr (stessa semantica del path MCP: `result.hint ?? emptyHint`). | ||
| - Il fallback resta su **stderr**, così l'output parsabile (CSV/JSONL) su stdout di pipeline e redirezioni non viene sporcato; l'exit code non cambia. | ||
| - Beneficiano i 6 tool con `emptyHint` statico: `senato-votes`, `bill-progress`, `amendments`, `bills`, `sindacato-ispettivo`, `votes`. | ||
| - Nessun cambio di comportamento quando il risultato non è vuoto o quando esiste già un hint dinamico (che mantiene la precedenza). | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
| - `cli-empty-result-hint`: comportamento della CLI nel comunicare, su risultato vuoto, il messaggio esplicativo del tool (hint dinamico se presente, altrimenti `emptyHint` statico) su stderr, allineandolo al path MCP. | ||
|
|
||
| ### Modified Capabilities | ||
| <!-- Nessuna spec esistente in openspec/specs/: nessuna capability modificata. --> | ||
|
|
||
| ## Impact | ||
|
|
||
| - Codice: `src/cli.ts` — funzione `runTool()` (fallback dell'hint) e/o `emit()`. Nessuna modifica ai ~50 call site di `emit()`. | ||
| - Nessun impatto su `src/server.ts` (già corretto) né sulle definizioni dei tool (l'`emptyHint` esiste già). | ||
| - Nessun breaking change: stdout invariato, exit code invariato; cambia solo un messaggio informativo su stderr in caso di risultato vuoto. | ||
| - Test: aggiungere copertura sul fallback CLI (risultato vuoto → `emptyHint` su stderr; hint dinamico ha precedenza; risultato non vuoto → nessun hint). |
33 changes: 33 additions & 0 deletions
33
...chive/2026-07-11-fix-cli-emptyhint-fallback/specs/cli-empty-result-hint/spec.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: La CLI comunica il messaggio esplicativo su risultato vuoto | ||
|
|
||
| Quando l'esecuzione di un comando produce un risultato senza righe, la CLI SHALL scrivere su stderr un messaggio esplicativo, senza alterare l'output su stdout né l'exit code. La sorgente del messaggio SHALL seguire la precedenza: l'hint dinamico del risultato (`result.hint`) se presente, altrimenti l'`emptyHint` statico definito dal tool. Se nessuno dei due è disponibile, la CLI SHALL non scrivere alcun messaggio. Questo comportamento SHALL coincidere con quello del server MCP (`result.hint ?? emptyHint`). | ||
|
aborruso marked this conversation as resolved.
Outdated
|
||
|
|
||
| #### Scenario: Risultato vuoto senza hint dinamico usa l'emptyHint statico | ||
|
|
||
| - **WHEN** un comando la cui definizione tool espone un `emptyHint` statico restituisce zero righe e nessun `result.hint` | ||
| - **THEN** la CLI scrive l'`emptyHint` statico del tool su stderr | ||
| - **AND** stdout contiene solo l'output vuoto formattato (CSV/JSONL), senza il messaggio | ||
| - **AND** l'exit code resta invariato | ||
|
|
||
| #### Scenario: L'hint dinamico ha precedenza sull'emptyHint statico | ||
|
|
||
| - **WHEN** un comando restituisce zero righe e un `result.hint` dinamico valorizzato, e il tool espone anche un `emptyHint` statico | ||
| - **THEN** la CLI scrive su stderr l'hint dinamico | ||
| - **AND** non scrive l'`emptyHint` statico | ||
|
|
||
| #### Scenario: Risultato non vuoto non produce alcun messaggio | ||
|
|
||
| - **WHEN** un comando restituisce almeno una riga | ||
| - **THEN** la CLI non scrive alcun hint su stderr, indipendentemente dalla presenza di `emptyHint` statico o `result.hint` | ||
|
|
||
| #### Scenario: Nessun hint disponibile non produce messaggio | ||
|
|
||
| - **WHEN** un comando restituisce zero righe, senza `result.hint` dinamico e senza `emptyHint` statico definito dal tool | ||
| - **THEN** la CLI non scrive alcun messaggio su stderr | ||
|
|
||
| #### Scenario: Parità di comportamento tra CLI e MCP | ||
|
|
||
| - **WHEN** lo stesso tool con `emptyHint` statico produce un risultato vuoto senza hint dinamico | ||
| - **THEN** il messaggio comunicato dalla CLI su stderr coincide con quello restituito dal server MCP | ||
19 changes: 19 additions & 0 deletions
19
openspec/changes/archive/2026-07-11-fix-cli-emptyhint-fallback/tasks.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| ## 1. Fix del fallback nella CLI | ||
|
|
||
| - [x] 1.1 In `src/cli.ts`, allargare la firma di `runTool()` per includere `emptyHint?: string` nell'oggetto tool accettato. | ||
| - [x] 1.2 In `runTool()`, dopo `tool.execute(parsed)`, se il risultato è vuoto e senza hint dinamico ma il tool ha un `emptyHint`, restituire `{ ...result, hint: tool.emptyHint }` (precedenza `result.hint ?? emptyHint`, senza mutare `result`). | ||
| - [x] 1.3 Verificare che `emit()` resti invariato (già scrive `result.hint` su stderr su risultato vuoto) e che i ~50 call site non richiedano modifiche. | ||
|
|
||
| ## 2. Verifica manuale sui tool con emptyHint | ||
|
|
||
| - [x] 2.1 Build (`npm run build`) e prova su finestra vuota (10 mar–16 apr 2020, buco Cura Italia): stdout solo header, `emptyHint` su stderr, exit 0. | ||
| - [x] 2.2 Confrontare il messaggio con quello restituito dal path MCP per lo stesso tool/input (parità CLI↔MCP): stessa fonte `result.hint ?? emptyHint`. | ||
| - [x] 2.3 Spot check sugli altri tool con `emptyHint` statico: `bill-progress`, `bills`, `sindacato-ispettivo`, `votes` verificati (emptyHint su stderr); `amendments` stesso meccanismo. | ||
|
|
||
| ## 3. Test automatici | ||
|
|
||
| - [x] 3.1 Aggiunto test: risultato vuoto + `emptyHint` statico → `hint` valorizzato (`src/core/empty-hint.test.ts`). | ||
| - [x] 3.2 Test: hint dinamico presente → precede l'`emptyHint` statico. | ||
| - [x] 3.3 Test: risultato non vuoto → nessun hint. | ||
| - [x] 3.4 Test: risultato vuoto senza hint né `emptyHint` → nessun hint; più test di non-mutazione e parità con MCP. | ||
| - [x] 3.5 Suite completa con `npm test -- --run`: 136/136 passano; tsc pulito. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| schema: spec-driven | ||
|
|
||
| # Project context (optional) | ||
| # This is shown to AI when creating artifacts. | ||
| # Add your tech stack, conventions, style guides, domain knowledge, etc. | ||
| # Example: | ||
| # context: | | ||
| # Tech stack: TypeScript, React, Node.js | ||
| # We use conventional commits | ||
| # Domain: e-commerce platform | ||
|
|
||
| # Per-artifact rules (optional) | ||
| # Add custom rules for specific artifacts. | ||
| # Example: | ||
| # rules: | ||
| # proposal: | ||
| # - Keep proposals under 500 words | ||
| # - Always include a "Non-goals" section | ||
| # tasks: | ||
| # - Break tasks into chunks of max 2 hours |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # cli-empty-result-hint Specification | ||
|
|
||
| ## Purpose | ||
| Garantire che la CLI, su un risultato vuoto, comunichi lo stesso messaggio esplicativo del server MCP: l'hint dinamico se presente, altrimenti l'`emptyHint` statico del tool. Evita che l'utente CLI riceva un output vuoto senza spiegazione (es. gap noto del dataset) e mantiene allineati i due path (CLI e MCP). | ||
|
|
||
| ## Requirements | ||
| ### Requirement: La CLI comunica il messaggio esplicativo su risultato vuoto | ||
|
|
||
| Quando l'esecuzione di un comando produce un risultato senza righe, la CLI SHALL scrivere su stderr un messaggio esplicativo, senza alterare l'output su stdout né l'exit code. La sorgente del messaggio SHALL seguire la precedenza: l'hint dinamico del risultato (`result.hint`) se presente, altrimenti l'`emptyHint` statico definito dal tool. Se nessuno dei due è disponibile, la CLI SHALL non scrivere alcun messaggio. Questo comportamento SHALL coincidere con quello del server MCP (`result.hint ?? emptyHint`). | ||
|
aborruso marked this conversation as resolved.
Outdated
|
||
|
|
||
| #### Scenario: Risultato vuoto senza hint dinamico usa l'emptyHint statico | ||
|
|
||
| - **WHEN** un comando la cui definizione tool espone un `emptyHint` statico restituisce zero righe e nessun `result.hint` | ||
| - **THEN** la CLI scrive l'`emptyHint` statico del tool su stderr | ||
| - **AND** stdout contiene solo l'output vuoto formattato (CSV/JSONL), senza il messaggio | ||
| - **AND** l'exit code resta invariato | ||
|
|
||
| #### Scenario: L'hint dinamico ha precedenza sull'emptyHint statico | ||
|
|
||
| - **WHEN** un comando restituisce zero righe e un `result.hint` dinamico valorizzato, e il tool espone anche un `emptyHint` statico | ||
| - **THEN** la CLI scrive su stderr l'hint dinamico | ||
| - **AND** non scrive l'`emptyHint` statico | ||
|
|
||
| #### Scenario: Risultato non vuoto non produce alcun messaggio | ||
|
|
||
| - **WHEN** un comando restituisce almeno una riga | ||
| - **THEN** la CLI non scrive alcun hint su stderr, indipendentemente dalla presenza di `emptyHint` statico o `result.hint` | ||
|
|
||
| #### Scenario: Nessun hint disponibile non produce messaggio | ||
|
|
||
| - **WHEN** un comando restituisce zero righe, senza `result.hint` dinamico e senza `emptyHint` statico definito dal tool | ||
| - **THEN** la CLI non scrive alcun messaggio su stderr | ||
|
|
||
| #### Scenario: Parità di comportamento tra CLI e MCP | ||
|
|
||
| - **WHEN** lo stesso tool con `emptyHint` statico produce un risultato vuoto senza hint dinamico | ||
| - **THEN** il messaggio comunicato dalla CLI su stderr coincide con quello restituito dal server MCP | ||
|
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
| import { withEmptyHint } from "./empty-hint.js"; | ||
| import type { ToolResult } from "../tools/types.js"; | ||
|
|
||
| const empty = (hint?: string): ToolResult => ({ rows: [], columns: ["a"], hint }); | ||
| const filled = (hint?: string): ToolResult => ({ rows: [{ a: "1" }], columns: ["a"], hint }); | ||
|
|
||
| // Riproduce la risoluzione del path MCP (server.ts formatResult, escluso DEFAULT_EMPTY) | ||
| const mcpHint = (result: ToolResult, emptyHint?: string) => result.hint ?? emptyHint; | ||
|
|
||
| describe("withEmptyHint", () => { | ||
| it("risultato vuoto senza hint dinamico usa l'emptyHint statico", () => { | ||
| const out = withEmptyHint(empty(), "STATICO"); | ||
| expect(out.hint).toBe("STATICO"); | ||
| expect(out.rows).toEqual([]); | ||
| }); | ||
|
|
||
| it("l'hint dinamico ha precedenza sull'emptyHint statico", () => { | ||
| const out = withEmptyHint(empty("DINAMICO"), "STATICO"); | ||
| expect(out.hint).toBe("DINAMICO"); | ||
| }); | ||
|
|
||
| it("risultato non vuoto non riceve alcun hint", () => { | ||
| const out = withEmptyHint(filled(), "STATICO"); | ||
| expect(out.hint).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("vuoto senza hint dinamico né emptyHint non produce hint", () => { | ||
| const out = withEmptyHint(empty(), undefined); | ||
| expect(out.hint).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("non muta l'oggetto risultato in ingresso", () => { | ||
| const input = empty(); | ||
| const out = withEmptyHint(input, "STATICO"); | ||
| expect(input.hint).toBeUndefined(); | ||
| expect(out).not.toBe(input); | ||
| }); | ||
|
|
||
| it("hint dinamico stringa vuota (falsy ma non nullish) non viene sovrascritto", () => { | ||
| const out = withEmptyHint(empty(""), "STATICO"); | ||
| expect(out.hint).toBe(""); | ||
| }); | ||
|
|
||
| it("parità con il path MCP (result.hint ?? emptyHint)", () => { | ||
| const cases: Array<[ToolResult, string | undefined]> = [ | ||
| [empty(), "STATICO"], | ||
| [empty("DINAMICO"), "STATICO"], | ||
| [empty(""), "STATICO"], | ||
| [empty(), undefined], | ||
| ]; | ||
|
Copilot marked this conversation as resolved.
|
||
| for (const [result, emptyHint] of cases) { | ||
| expect(withEmptyHint(result, emptyHint).hint).toBe(mcpHint(result, emptyHint)); | ||
| } | ||
| }); | ||
| }); | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import type { ToolResult } from "../tools/types.js"; | ||
|
|
||
| /** | ||
| * Su risultato vuoto senza hint dinamico, valorizza `hint` con l'emptyHint | ||
| * statico del tool (precedenza `result.hint ?? emptyHint`, come il path MCP in | ||
| * server.ts). Non muta l'input: restituisce un nuovo oggetto solo se serve. | ||
| * Così la CLI (emit → stderr) e l'MCP comunicano lo stesso messaggio. | ||
| */ | ||
| export function withEmptyHint(result: ToolResult, emptyHint?: string): ToolResult { | ||
| // `result.hint == null` (nullish) e non `!result.hint`: così un hint dinamico | ||
| // valido ma falsy (stringa vuota) non viene sovrascritto, restando fedele | ||
| // alla precedenza `result.hint ?? emptyHint` del path MCP. | ||
| if (result.rows.length === 0 && result.hint == null && emptyHint) { | ||
| return { ...result, hint: emptyHint }; | ||
| } | ||
| return result; | ||
| } | ||
|
aborruso marked this conversation as resolved.
|
||
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.
Uh oh!
There was an error while loading. Please reload this page.