Skip to content

Commit a6daeea

Browse files
committed
fix: address open review findings across skills, vector, and docs
- Move the generated-by header inside the SKILL.md frontmatter fence so frontmatter-based skill discovery keeps working; files now begin with "---" and Classify reads the header from line two. - Tighten the embedding 4xx skip classifier: a size word must pair with an input word and "content" with "policy", so auth ("invalid token") and media-type ("unsupported content type") failures abort the build instead of skip-stamping the corpus. - Double-wrap query-encode failures so context.Canceled/DeadlineExceeded stay matchable through db.ErrSemanticTransient. - Set USERPROFILE alongside HOME in skills CLI tests for Windows runs. - Spell out --since units (m = months, not minutes) in flag help, the session API tables, and the skill template. - Correct internals docs: skip-and-stamp taxonomy now matches the 400/413/422 allowlist, and the mirror-corpus note no longer claims parity with unfiltered FTS. - Regenerate the frontend API client (search intent header, embeddings routes, and earlier drift) and update two activity test fixtures for the now-required projects field.
1 parent 3ba6048 commit a6daeea

35 files changed

Lines changed: 529 additions & 90 deletions

cmd/agentsview/embed_scheduler.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,10 @@ func translateSearchError(err error) error {
301301
case errors.Is(err, vector.ErrNoActiveGeneration):
302302
return db.ErrSemanticUnavailable
303303
case errors.As(err, &queryEncErr):
304-
return fmt.Errorf("%w: %v", db.ErrSemanticTransient, queryEncErr.Err)
304+
// Double-wrap so callers can still match the underlying cause —
305+
// notably context.Canceled/DeadlineExceeded from a dead client —
306+
// alongside the transient sentinel.
307+
return fmt.Errorf("%w: %w", db.ErrSemanticTransient, queryEncErr.Err)
305308
default:
306309
return err
307310
}

cmd/agentsview/embed_scheduler_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"mime"
78
"net"
89
"net/http"
@@ -449,6 +450,15 @@ func TestTranslateSearchErrorMapsVectorErrorsToSemanticUnavailable(t *testing.T)
449450
"a query-time endpoint failure must not read as semantic search being disabled")
450451
assert.Contains(t, got.Error(), "connection refused")
451452
})
453+
t.Run("query encode failure preserves the underlying cause chain", func(t *testing.T) {
454+
queryErr := &vector.QueryEncodeError{
455+
Err: fmt.Errorf("encoding query: %w", context.Canceled),
456+
}
457+
got := translateSearchError(queryErr)
458+
assert.ErrorIs(t, got, db.ErrSemanticTransient)
459+
assert.ErrorIs(t, got, context.Canceled,
460+
"context errors must stay matchable so cancellation handling still fires")
461+
})
452462
}
453463

454464
// --- integration: real serve/server construction path ---

cmd/agentsview/session_list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func newSessionListCommand() *cobra.Command {
143143
flags.StringVar(&activeSince, "active-since", "",
144144
"Filter sessions active since RFC3339 timestamp")
145145
flags.StringVar(&since, "since", "",
146-
"Only sessions active since a relative duration (3m, 14d, 12h, 1y) or YYYY-MM-DD")
146+
"Only sessions active since a relative duration (12h, 14d, 2w, 3m = 3 months, 1y) or YYYY-MM-DD")
147147
flags.IntVar(&minMessages, "min-messages", 0,
148148
"Minimum total message count")
149149
flags.IntVar(&maxMessages, "max-messages", 0,

cmd/agentsview/session_search.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func newSessionSearchCommand() *cobra.Command {
108108
flags.StringVar(&dateTo, "date-to", "", "Sessions on or before YYYY-MM-DD")
109109
flags.StringVar(&activeSince, "active-since", "", "Active since RFC3339 timestamp")
110110
flags.StringVar(&since, "since", "",
111-
"Only sessions active since a relative duration (3m, 14d, 12h, 1y) or YYYY-MM-DD")
111+
"Only sessions active since a relative duration (12h, 14d, 2w, 3m = 3 months, 1y) or YYYY-MM-DD")
112112
flags.BoolVar(&includeChildren, "include-children", false, "Include subagent sessions")
113113
flags.BoolVar(&includeAutomated, "include-automated", false, "Include automated sessions")
114114
flags.BoolVar(&includeOneShot, "include-one-shot", false, "Include one-shot sessions")

cmd/agentsview/skills_test.go

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,29 @@ func writeSkillFile(t *testing.T, path, content string) {
5656
require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
5757
}
5858

59+
// setTestHome points the process home at dir for both Unix (HOME) and
60+
// Windows (USERPROFILE), since os.UserHomeDir reads a different variable
61+
// per platform.
62+
func setTestHome(t *testing.T, dir string) {
63+
t.Helper()
64+
t.Setenv("HOME", dir)
65+
t.Setenv("USERPROFILE", dir)
66+
}
67+
5968
// staleClaudeContent returns a well-formed generated-by file whose recorded
6069
// hash matches an older body, so Classify reports StateStale.
6170
func staleClaudeContent() string {
62-
oldBody := "an earlier revision of the skill body, no longer current\n"
71+
oldBody := "---\nname: agentsview-finding-history\n---\n" +
72+
"an earlier revision of the skill body, no longer current\n"
6373
header := fmt.Sprintf(skillHeaderFormat, "0.0.1", sha256Hex(oldBody))
64-
return header + "\n" + oldBody
74+
return "---\n" + header + "\n" + strings.TrimPrefix(oldBody, "---\n")
6575
}
6676

6777
// modifiedClaudeContent returns a fresh render whose body was hand-edited
6878
// after the header hash was recorded, so Classify reports StateModified.
6979
func modifiedClaudeContent(t *testing.T) string {
7080
t.Helper()
71-
fresh := freshClaudeSkill(t)
72-
headerLine, body, ok := strings.Cut(fresh.Content, "\n")
73-
require.True(t, ok)
74-
return headerLine + "\n" + body + "\nan uninvited local edit\n"
81+
return freshClaudeSkill(t).Content + "\nan uninvited local edit\n"
7582
}
7683

7784
const foreignClaudeContent = "# Just a hand-written file\n\nNo generated-by header here.\n"
@@ -126,7 +133,7 @@ func TestSkillsInstall_StatesAndForce(t *testing.T) {
126133
for _, force := range []bool{false, true} {
127134
t.Run(fmt.Sprintf("%s/force=%v", tt.name, force), func(t *testing.T) {
128135
home := t.TempDir()
129-
t.Setenv("HOME", home)
136+
setTestHome(t, home)
130137
path := claudeSkillPath(home)
131138

132139
var seedContent string
@@ -175,7 +182,7 @@ func readFileString(t *testing.T, path string) string {
175182

176183
func TestSkillsInstall_DefaultHarnessesInstallBoth(t *testing.T) {
177184
home := t.TempDir()
178-
t.Setenv("HOME", home)
185+
setTestHome(t, home)
179186

180187
out, err := executeCommand(newRootCommand(), "skills", "install")
181188
require.NoError(t, err, "output: %s", out)
@@ -188,7 +195,7 @@ func TestSkillsInstall_DefaultHarnessesInstallBoth(t *testing.T) {
188195

189196
func TestSkillsInstall_UnknownHarnessErrors(t *testing.T) {
190197
home := t.TempDir()
191-
t.Setenv("HOME", home)
198+
setTestHome(t, home)
192199

193200
_, err := executeCommand(newRootCommand(), "skills", "install", "--harness", "bogus")
194201
require.Error(t, err)
@@ -197,7 +204,7 @@ func TestSkillsInstall_UnknownHarnessErrors(t *testing.T) {
197204

198205
func TestSkillsInstall_RefusalStillInstallsOtherTargets(t *testing.T) {
199206
home := t.TempDir()
200-
t.Setenv("HOME", home)
207+
setTestHome(t, home)
201208
writeSkillFile(t, claudeSkillPath(home), foreignClaudeContent)
202209

203210
out, err := executeCommand(newRootCommand(), "skills", "install")
@@ -212,7 +219,7 @@ func TestSkillsInstall_RefusalStillInstallsOtherTargets(t *testing.T) {
212219

213220
func TestSkillsInstall_FilePermissions(t *testing.T) {
214221
home := t.TempDir()
215-
t.Setenv("HOME", home)
222+
setTestHome(t, home)
216223

217224
_, err := executeCommand(newRootCommand(), "skills", "install", "--harness", "claude")
218225
require.NoError(t, err)
@@ -268,7 +275,7 @@ func TestSkillsList_ReportsEachState(t *testing.T) {
268275
for _, tt := range tests {
269276
t.Run(tt.name, func(t *testing.T) {
270277
home := t.TempDir()
271-
t.Setenv("HOME", home)
278+
setTestHome(t, home)
272279
path := claudeSkillPath(home)
273280
if tt.seed != nil {
274281
tt.seed(t, path)
@@ -296,7 +303,7 @@ func TestSkillsList_ReportsEachState(t *testing.T) {
296303

297304
func TestSkillsList_HumanTableHasHeaderAndColumns(t *testing.T) {
298305
home := t.TempDir()
299-
t.Setenv("HOME", home)
306+
setTestHome(t, home)
300307

301308
out, err := executeCommand(newRootCommand(), "skills", "list")
302309
require.NoError(t, err, "output: %s", out)
@@ -327,7 +334,7 @@ func initTestGitRepo(t *testing.T) string {
327334

328335
func TestSkillsInstall_ProjectFlagUsesGitRoot(t *testing.T) {
329336
home := t.TempDir()
330-
t.Setenv("HOME", home)
337+
setTestHome(t, home)
331338

332339
repo := initTestGitRepo(t)
333340
nested := filepath.Join(repo, "a", "b")
@@ -346,7 +353,7 @@ func TestSkillsInstall_ProjectFlagUsesGitRoot(t *testing.T) {
346353

347354
func TestSkillsList_ProjectFlagReportsProjectLevel(t *testing.T) {
348355
home := t.TempDir()
349-
t.Setenv("HOME", home)
356+
setTestHome(t, home)
350357

351358
repo := initTestGitRepo(t)
352359
t.Chdir(repo)
@@ -365,7 +372,7 @@ func TestSkillsList_ProjectFlagReportsProjectLevel(t *testing.T) {
365372

366373
func TestSkillsInstall_ProjectFlagOutsideRepoFallsBackToCWD(t *testing.T) {
367374
home := t.TempDir()
368-
t.Setenv("HOME", home)
375+
setTestHome(t, home)
369376

370377
outsideRepo := t.TempDir()
371378
resolvedOutside, err := filepath.EvalSymlinks(outsideRepo)

docs/semantic-search-internals.md

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ special-casing during the swap instead of a plain re-scan afterward.
2929
### The `vector_messages` mirror table
3030

3131
One row per embeddable message (`role IN ('user','assistant')`, non-system,
32-
non-system-prefixed — the same universe FTS uses before `--exclude-system`).
33-
Columns: `doc_key` (primary key), `session_id`, `source_uuid`, `ordinal`,
34-
`content` (copied text), `content_hash` (sha256 of content, kit's revision
35-
column), `embed_gen`.
32+
non-system-prefixed — the corpus semantic and hybrid search operate on; the
33+
hybrid FTS leg applies the same predicate, while plain FTS content search is
34+
broader unless filters narrow it). Columns: `doc_key` (primary key),
35+
`session_id`, `source_uuid`, `ordinal`, `content` (copied text), `content_hash`
36+
(sha256 of content, kit's revision column), `embed_gen`.
3637

3738
### `doc_key` scheme
3839

@@ -97,14 +98,18 @@ re-embedding.
9798

9899
Fill embeds every pending document (content changed, or never embedded, for the
99100
active generation). A document whose encode call fails with a permanent error —
100-
any 4xx except 429, e.g. token-window overflow or a content-policy rejection —
101-
is not retried in that fill or the next one: it's stamped for the generation
102-
with no vectors at its current `content_hash`, which marks it non-pending. It's
103-
logged (doc key plus the underlying error) and counted in the build summary's
104-
skipped count, but there is no separate poison list or periodic retry — the only
105-
way it embeds again is if the message's content itself changes later (a new
106-
`content_hash`, so a new pending row). All other failures (5xx, network errors,
107-
timeouts, 429) abort the fill and are retried on the next scheduled build.
101+
a 400, 413, or 422 whose error body describes the input itself, e.g. a
102+
token/context-length overflow or a content-policy rejection — is not retried in
103+
that fill or the next one: it's stamped for the generation with no vectors at
104+
its current `content_hash`, which marks it non-pending. It's logged (doc key
105+
plus the underlying error) and counted in the build summary's skipped count, but
106+
there is no separate poison list or periodic retry — the only way it embeds
107+
again is if the message's content itself changes later (a new `content_hash`, so
108+
a new pending row). Every other failure — 5xx, network errors, timeouts, 429,
109+
and any 4xx that looks like an auth, route, model, or media-type problem rather
110+
than a rejection of this document — aborts the fill and is retried on the next
111+
scheduled build, so a config mistake can't silently stamp the whole corpus as
112+
embedded-with-no-vectors.
108113

109114
### Scope (`include_automated`)
110115

@@ -178,14 +183,16 @@ single embedded template, `internal/skills/templates/finding-history.md.tmpl`
178183
via `go:embed` — the same pattern `internal/web` uses for the frontend — with no
179184
per-harness copies checked in. `Render` fills in a harness-specific delegation
180185
phrase (whether the harness can dispatch a search subagent or must run the
181-
bounded probes itself) and prepends a `generated-by` header carrying the CLI
182-
version and a sha256 hash of the rendered body. Staleness and tamper detection
183-
are hash-authoritative, not version-authoritative: `Classify` compares a file's
184-
recorded hash against its own body hash to detect modification, and against a
185-
fresh render's hash to detect staleness, and never consults the version string,
186-
because dev builds all report version `"dev"` and would otherwise be
187-
indistinguishable from one another. There is deliberately no Claude Code
188-
plugin/marketplace packaging: that would tie distribution to one harness's
186+
bounded probes itself) and inserts a `generated-by` header — carrying the CLI
187+
version and a sha256 hash of the pure template render — as a YAML comment on
188+
line two, just inside the frontmatter fence, so the file still begins with `---`
189+
and frontmatter-based skill discovery keeps working. Staleness and tamper
190+
detection are hash-authoritative, not version-authoritative: `Classify` compares
191+
a file's recorded hash against its own body hash to detect modification, and
192+
against a fresh render's hash to detect staleness, and never consults the
193+
version string, because dev builds all report version `"dev"` and would
194+
otherwise be indistinguishable from one another. There is deliberately no Claude
195+
Code plugin/marketplace packaging: that would tie distribution to one harness's
189196
install mechanism, whereas the goal is a single `SKILL.md` artifact that any
190197
`.agents/skills`-reading harness can consume the same way, installed directly by
191198
the `agentsview` binary rather than a separate package manager.

docs/semantic-search.md

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -315,14 +315,15 @@ agentsview skills list # show install state per harness
315315
the working directory itself outside a repo), writing to `.claude/skills/...`
316316
and `.agents/skills/...` instead.
317317

318-
Every rendered file starts with a `generated-by` header carrying a content hash.
319-
`install` overwrites a file whose hash still matches its header (unmodified
320-
since the last install) but refuses a file that was hand-edited or was never
321-
generated by `agentsview`, printing which paths it refused and exiting non-zero;
322-
pass `--force` to overwrite anyway. Re-run `agentsview skills install` after
323-
upgrading `agentsview` to pick up skill content changes — the header records the
324-
CLI version for humans, but the content hash, not the version, decides whether a
325-
reinstall is a no-op.
318+
Every rendered file carries a `generated-by` header with a content hash, written
319+
as a YAML comment just inside the frontmatter fence so the file still starts
320+
with `---` and harnesses keep discovering it. `install` overwrites a file whose
321+
hash still matches its header (unmodified since the last install) but refuses a
322+
file that was hand-edited or was never generated by `agentsview`, printing which
323+
paths it refused and exiting non-zero; pass `--force` to overwrite anyway.
324+
Re-run `agentsview skills install` after upgrading `agentsview` to pick up skill
325+
content changes — the header records the CLI version for humans, but the content
326+
hash, not the version, decides whether a reinstall is a no-op.
326327

327328
`agentsview skills list [--project] [--format json]` reports each harness's
328329
install state — `missing`, `current`, `stale` (unmodified but older than the

docs/session-api.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ One-shot and automated sessions are excluded by default. Use the
226226
| `--date-from` | `date_from` | `YYYY-MM-DD` |
227227
| `--date-to` | `date_to` | `YYYY-MM-DD` |
228228
| `--active-since` | `active_since` | RFC3339 timestamp |
229-
| `--since` | `active_since` | Relative (`3m`, `14d`, `12h`, `1y`) or `YYYY-MM-DD`; resolved against now and mutually exclusive with `--active-since` |
229+
| `--since` | `active_since` | Relative `Nh` hours, `Nd` days, `Nw` weeks, `Nm` calendar months (not minutes), `Ny` years — or `YYYY-MM-DD`; resolved against now and mutually exclusive with `--active-since` |
230230
| `--resume` | `active_since` | CLI shortcut for sessions active in the last 15 minutes |
231231
| `--active` | `active_since` | Alias for `--resume` |
232232
| `--min-messages` | `min_messages` | int |
@@ -568,7 +568,7 @@ default; opt back in with `--include-one-shot`,
568568
| `--date-from` | `date_from` | `YYYY-MM-DD` |
569569
| `--date-to` | `date_to` | `YYYY-MM-DD` |
570570
| `--active-since` | `active_since` | RFC3339 timestamp |
571-
| `--since` | `active_since` | Relative (`3m`, `14d`, `12h`, `1y`) or `YYYY-MM-DD`; resolved against now and mutually exclusive with `--active-since` |
571+
| `--since` | `active_since` | Relative `Nh` hours, `Nd` days, `Nw` weeks, `Nm` calendar months (not minutes), `Ny` years — or `YYYY-MM-DD`; resolved against now and mutually exclusive with `--active-since` |
572572
| `--include-children` | `include_children` | bool |
573573
| `--include-automated` | `include_automated` | bool |
574574
| `--include-one-shot` | `include_one_shot` | bool |

frontend/src/lib/api/generated/index.ts

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/src/lib/api/generated/models/ActivityReport.ts

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)