The coverage manifest counts files, not directories

cmd/kai/review_commit.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
@@ -760,7 +761,7 @@ func rcRunReviewAgent(ctx context.Context, set *projects.Set, prov provider.Prov
 		FinishReason: string(res.FinishReason),
 		Elapsed:      time.Since(started),
 		Turns:        len(res.Transcript),
-		FilesRead:    rcFilesRead(res.Transcript),
+		FilesRead:    rcFilesRead(res.Transcript, primary.Path),
 	}
 	raw := strings.TrimSpace(res.FinalText)
 	// A run that ran out of road — the soft time budget fired, or the loop
@@ -836,11 +837,24 @@ func rcCoverageOf(inc *rcIncomplete) *rcCoverage {
 	}
 }
 
-// rcFilesRead pulls the distinct paths the run actually opened out of its tool
+// rcFilesRead pulls the distinct FILES the run actually opened out of its tool
 // calls. Deliberately cheap and schema-loose, like rcChangedSymbols: any tool
 // that names a file names it in a "path" or "file_path" field, and a missed one
 // only shortens a list that is already a courtesy.
-func rcFilesRead(transcript []message.Message) []string {
+//
+// Directories are dropped, and root is how. The same "path" argument that a
+// file read uses is also what a directory listing passes, so the manifest
+// published "What I opened — 6 files" over a list whose first two entries were
+// `frontend` and `frontend/dist` (kai-desktop#314), and "4 files" over one
+// containing `cmd/kai` (kai-cli#99). A coverage report that miscounts its own
+// coverage is the one thing this feature cannot afford.
+//
+// Only a CONFIRMED directory is dropped. A path that does not resolve — an
+// absolute path from another tree, a bare name, anything stat cannot answer —
+// is kept, because the cost is asymmetric: an extra entry slightly overstates
+// what was read, while a wrongly dropped one makes changedFilesNotListed
+// accuse the review of skipping a file it actually opened.
+func rcFilesRead(transcript []message.Message, root string) []string {
 	seen := map[string]bool{}
 	var out []string
 	for _, m := range transcript {
@@ -861,6 +875,9 @@ func rcFilesRead(transcript []message.Message) []string {
 					continue
 				}
 				seen[p] = true
+				if rcIsDir(root, p) {
+					continue
+				}
 				out = append(out, p)
 			}
 		}
@@ -869,6 +886,23 @@ func rcFilesRead(transcript []message.Message) []string {
 	return out
 }
 
+// rcIsDir reports whether p names a directory in the reviewed checkout.
+//
+// Answers only what the filesystem confirms: a stat that fails, for any reason,
+// is not a directory as far as this is concerned. See rcFilesRead for why the
+// uncertain case keeps the entry rather than dropping it.
+func rcIsDir(root, p string) bool {
+	full := p
+	if !filepath.IsAbs(full) {
+		if root == "" {
+			return false
+		}
+		full = filepath.Join(root, p)
+	}
+	fi, err := os.Stat(full)
+	return err == nil && fi.IsDir()
+}
+
 // rcIncompleteProse is the review of last resort: not a review at all, but an
 // honest account of a run that read code and ran out of road. It exists so the
 // finding still carries something a human can act on — how long it ran, why it

cmd/kai/review_commit_incomplete_test.go

@@ -2,6 +2,8 @@ package main
 
 import (
 	"encoding/json"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -75,7 +77,9 @@ func TestFilesReadDedupesAcrossToolCallSpellings(t *testing.T) {
 			message.ToolCall{ID: "5", Name: "kai_view", Input: `not json`},
 		},
 	}}
-	got := rcFilesRead(tr)
+	// Empty root: nothing resolves, so nothing is confirmed a directory and
+	// every named path is kept. Directory filtering has its own test below.
+	got := rcFilesRead(tr, "")
 	if len(got) != 2 || got[0] != "a.go" || got[1] != "b.go" {
 		t.Errorf("rcFilesRead = %v, want [a.go b.go] (deduped, sorted, non-file calls ignored)", got)
 	}
@@ -158,3 +162,41 @@ func TestCoverageShipsOnEveryGroundedRun(t *testing.T) {
 		t.Error("rcCoverageOf(nil) invented a manifest for a pass that opened nothing")
 	}
 }
+
+// "What I opened — 6 files" listed `frontend` and `frontend/dist`
+// (kai-desktop#314); "4 files" listed `cmd/kai` (kai-cli#99). A directory
+// listing passes the same "path" argument a file read does, so the manifest
+// counted directories as files it had read. A coverage report that miscounts
+// its own coverage is the one thing this feature cannot afford.
+func TestFilesReadDropsDirectories(t *testing.T) {
+	root := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(root, "frontend", "dist"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "frontend", "dist", "app.js"), []byte("x"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	tr := []message.Message{{
+		Role: message.RoleAssistant,
+		Parts: []message.ContentPart{
+			message.ToolCall{ID: "1", Name: "kai_grep", Input: `{"path":"frontend"}`},
+			message.ToolCall{ID: "2", Name: "kai_grep", Input: `{"path":"frontend/dist"}`},
+			message.ToolCall{ID: "3", Name: "kai_view", Input: `{"file_path":"frontend/dist/app.js"}`},
+			// Never existed here at all: unresolvable, so it is kept. Dropping
+			// it would make changedFilesNotListed accuse the review of
+			// skipping a file it opened.
+			message.ToolCall{ID: "4", Name: "kai_view", Input: `{"file_path":"/elsewhere/vendored.go"}`},
+		},
+	}}
+	got := rcFilesRead(tr, root)
+	want := []string{"/elsewhere/vendored.go", "frontend/dist/app.js"}
+	if len(got) != len(want) {
+		t.Fatalf("rcFilesRead = %v, want %v — directories must not count as files", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("rcFilesRead = %v, want %v", got, want)
+			break
+		}
+	}
+}