diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index b3f866c75..5c1d79fe1 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -27,6 +27,26 @@ CLI history before v0.3 is recorded in the project root fields (previously 7), grouped into 10 presentation sections. - `--all-pages` / `--page-size` on `search docs` and `search sessions` (catching up with `session list` / `doc list` canon from v0.3+v0.4). +- `weknora doc list` gains `--keyword` / `--file-type` / `--source` / + `--tag-id` / `--start-time` / `--end-time` (RFC3339) — matches the + SDK's `KnowledgeListFilter` surface. Time flags reject malformed + input with `input.invalid_argument`. +- MCP `doc_list` tool gains the same 5 filter fields (`keyword`, + `file_type`, `source`, `tag_id`, `start_time`, `end_time`) so agents + have parity with the CLI. +- `weknora session view --full` (with `--limit`, default 50, bounds + 1..1000) loads chat history via `LoadMessages` and renders messages + inline after session metadata. JSON mode projects messages into a + `messages` array. `--limit` without `--full` errors with + `input.invalid_argument`. +- `weknora kb view` human render now includes `TYPE`, `PINNED` (badge, + only when set), `TEMPORARY` (badge), `PROCESSING` (with doc count, + only when active), `SUMMARY MODEL`, and `CREATED`. Nested config + structs stay JSON-only. +- `weknora doc view` human render expands to include `TITLE` (when + distinct from filename), `DESC`, `SOURCE`, `CHANNEL`, `TAG`, + `STORAGE` (human-readable bytes), `SUMMARY`, `ENABLED`, and `HASH` + (12-char prefix). All omit-empty. #### Fixed - MCP `search_chunks` tool: `limit` arg now correctly threads into @@ -42,6 +62,14 @@ CLI history before v0.3 is recorded in the project root - `cli/AGENTS.md` adds §"Command surface design SOP" and §"CRUD command flag canon" for v0.6+ contributors. - `cli/go.mod`: adds `gopkg.in/yaml.v3` for `agent create --config-file`. +- `weknora search docs` now applies the keyword filter server-side via + `ListKnowledgeWithFilter` (was: page through every doc and substring- + match client-side). Smaller wire payload on large KBs. **Semantics + shift**: the match is now case-sensitive (server uses `LIKE %keyword%`), + whereas the previous client-side path lowered both sides. Callers that + relied on case-insensitive matching (e.g. `search docs Q3` finding + `q3 retro`) must lower-case the query, or fall back to `weknora api` + with a custom filter. ### v0.4 — output contract hardening and mainstream alignment diff --git a/cli/cmd/doc/list.go b/cli/cmd/doc/list.go index e2a541e22..662a945a0 100644 --- a/cli/cmd/doc/list.go +++ b/cli/cmd/doc/list.go @@ -37,8 +37,20 @@ type ListOptions struct { // AllPages walks server pages internally, accumulating items until // total exhausted or --limit hit. AllPages bool + // Additional server-side filters (each maps 1:1 to a sdk.KnowledgeListFilter + // field). Empty / zero values are omitted from the request. + Keyword string + FileType string + Source string + TagID string + StartTime string // raw RFC3339; parsed into filter.StartTime + EndTime string // raw RFC3339; parsed into filter.EndTime } +// rfc3339Example is the canonical RFC3339 hint surfaced when --start-time / +// --end-time fail to parse. Picked to match Go's reference time docs. +const rfc3339Example = "2006-01-02T15:04:05Z" + // docListStatusValues mirrors internal/types/knowledge.go ParseStatus* // constants - these are the values the server accepts on the // ?parse_status= query. Kept in sync manually since the SDK doesn't @@ -92,6 +104,12 @@ backend storage order is not guaranteed and varies between deployments.`, cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum results to return (0 = no cap, 1..10000 = explicit)") cmd.Flags().BoolVar(&opts.AllPages, "all-pages", false, "Walk all server pages until exhausted (or --limit hit)") cmd.Flags().StringVar(&opts.Status, "status", "", "Filter by parse status: pending | processing | completed | failed") + cmd.Flags().StringVar(&opts.Keyword, "keyword", "", "Server-side substring match against title / file_name (case-sensitive)") + cmd.Flags().StringVar(&opts.FileType, "file-type", "", `Filter by file extension (e.g. "pdf", "md")`) + cmd.Flags().StringVar(&opts.Source, "source", "", `Filter by ingestion source (e.g. "api", "web")`) + cmd.Flags().StringVar(&opts.TagID, "tag-id", "", "Filter by tag association") + cmd.Flags().StringVar(&opts.StartTime, "start-time", "", "Include docs with updated_at >= this RFC3339 timestamp (e.g. 2006-01-02T15:04:05Z)") + cmd.Flags().StringVar(&opts.EndTime, "end-time", "", "Include docs with updated_at <= this RFC3339 timestamp (e.g. 2006-01-02T15:04:05Z)") cmdutil.AddJSONFlags(cmd, docListFields) return cmd } @@ -116,7 +134,29 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, strings.Join(docListStatusValues, " | "), opts.Status), } } - filter := sdk.KnowledgeListFilter{ParseStatus: opts.Status} + filter := sdk.KnowledgeListFilter{ + ParseStatus: opts.Status, + Keyword: opts.Keyword, + FileType: opts.FileType, + Source: opts.Source, + TagID: opts.TagID, + } + if opts.StartTime != "" { + t, err := time.Parse(time.RFC3339, opts.StartTime) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + fmt.Sprintf("--start-time must be RFC3339 (e.g. %s), got %q", rfc3339Example, opts.StartTime)) + } + filter.StartTime = t + } + if opts.EndTime != "" { + t, err := time.Parse(time.RFC3339, opts.EndTime) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + fmt.Sprintf("--end-time must be RFC3339 (e.g. %s), got %q", rfc3339Example, opts.EndTime)) + } + filter.EndTime = t + } // Pagination is always 1-indexed internally. --all-pages walks; the // non-walking path returns the first page only. diff --git a/cli/cmd/doc/list_test.go b/cli/cmd/doc/list_test.go index 326bb95f1..2d3104b84 100644 --- a/cli/cmd/doc/list_test.go +++ b/cli/cmd/doc/list_test.go @@ -339,3 +339,110 @@ func TestList_AllPages_WithLimit_StopsAtLimit(t *testing.T) { // Should have called pages 1..3 (60 items) then capped at 50. assert.LessOrEqual(t, len(svc.calls), 3, "should not walk past the page that fills --limit") } + +// ----- C11: richer filter flags ----- + +func TestList_Keyword_PassedToFilter(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeListSvc{} + opts := &ListOptions{PageSize: 20, Keyword: "spec"} + require.NoError(t, runList(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "spec", svc.got.filter.Keyword) +} + +func TestList_FileType_PassedToFilter(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeListSvc{} + opts := &ListOptions{PageSize: 20, FileType: "pdf"} + require.NoError(t, runList(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "pdf", svc.got.filter.FileType) +} + +func TestList_Source_PassedToFilter(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeListSvc{} + opts := &ListOptions{PageSize: 20, Source: "api"} + require.NoError(t, runList(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "api", svc.got.filter.Source) +} + +func TestList_TagID_PassedToFilter(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeListSvc{} + opts := &ListOptions{PageSize: 20, TagID: "tag_42"} + require.NoError(t, runList(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "tag_42", svc.got.filter.TagID) +} + +func TestList_StartTime_RFC3339Parses(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeListSvc{} + want := "2026-05-01T00:00:00Z" + opts := &ListOptions{PageSize: 20, StartTime: want} + require.NoError(t, runList(context.Background(), opts, nil, svc, "kb_xxx")) + parsed, err := time.Parse(time.RFC3339, want) + require.NoError(t, err) + assert.True(t, svc.got.filter.StartTime.Equal(parsed), + "--start-time must be parsed into filter.StartTime; got %v want %v", + svc.got.filter.StartTime, parsed) +} + +func TestList_EndTime_RFC3339Parses(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeListSvc{} + want := "2026-06-30T23:59:59Z" + opts := &ListOptions{PageSize: 20, EndTime: want} + require.NoError(t, runList(context.Background(), opts, nil, svc, "kb_xxx")) + parsed, err := time.Parse(time.RFC3339, want) + require.NoError(t, err) + assert.True(t, svc.got.filter.EndTime.Equal(parsed)) +} + +func TestList_StartTime_InvalidFormat_Rejected(t *testing.T) { + _, _ = iostreams.SetForTest(t) + opts := &ListOptions{PageSize: 20, StartTime: "tomorrow"} + err := runList(context.Background(), opts, nil, &fakeListSvc{}, "kb_xxx") + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Contains(t, typed.Message, "--start-time") + assert.Contains(t, typed.Message, "RFC3339") +} + +func TestList_EndTime_InvalidFormat_Rejected(t *testing.T) { + _, _ = iostreams.SetForTest(t) + opts := &ListOptions{PageSize: 20, EndTime: "2026-05-01"} // date-only, not RFC3339 + err := runList(context.Background(), opts, nil, &fakeListSvc{}, "kb_xxx") + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Contains(t, typed.Message, "--end-time") +} + +// TestList_AllFiltersCombined drives every new filter flag at once to confirm +// they all land on the same filter struct (AND combine on the server). +func TestList_AllFiltersCombined(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeListSvc{} + opts := &ListOptions{ + PageSize: 20, + Status: "completed", + Keyword: "spec", + FileType: "pdf", + Source: "api", + TagID: "tag_42", + StartTime: "2026-01-01T00:00:00Z", + EndTime: "2026-12-31T23:59:59Z", + } + require.NoError(t, runList(context.Background(), opts, nil, svc, "kb_xxx")) + f := svc.got.filter + assert.Equal(t, "completed", f.ParseStatus) + assert.Equal(t, "spec", f.Keyword) + assert.Equal(t, "pdf", f.FileType) + assert.Equal(t, "api", f.Source) + assert.Equal(t, "tag_42", f.TagID) + assert.False(t, f.StartTime.IsZero()) + assert.False(t, f.EndTime.IsZero()) +} diff --git a/cli/cmd/doc/upload.go b/cli/cmd/doc/upload.go index d4b5d760b..bf0b5ef4d 100644 --- a/cli/cmd/doc/upload.go +++ b/cli/cmd/doc/upload.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "strings" "github.com/spf13/cobra" @@ -13,9 +14,10 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// uploadChannel is the ingestion-channel tag the server records for CLI uploads. -// Distinct from "web" (browser UI), "browser_extension" (one-click capture), -// and "wechat" (mini-program). The server uses this only for analytics. +// uploadChannel is the default ingestion-channel tag the server records for +// CLI uploads. Distinct from "web" (browser UI), "browser_extension" +// (one-click capture), and "wechat" (mini-program). The server uses this only +// for analytics. Users can override via --channel for cross-tool replay. const uploadChannel = "api" // docUploadFields enumerates the fields surfaced for `--json` discovery on @@ -34,6 +36,25 @@ type UploadOptions struct { Recursive bool // --recursive: positional arg is a directory; walk + upload each match Glob string // --glob: filename pattern under --recursive (default "*") FromURL string // --from-url: ingest a remote URL via SDK CreateKnowledgeFromURL + + // EnableMultimodel toggles server-side multimodal extraction + // (e.g. images-in-PDF → OCR'd text). nil means "server default" - + // the flag was not set. true/false explicitly override. + EnableMultimodel *bool + + // Metadata is the raw --metadata key=value list. Parsed into a map + // at run-time; empty values allowed, duplicate keys last-wins. + Metadata []string + + // Channel overrides the ingestion-channel tag recorded server-side. + // Empty ⇒ uploadChannel ("api"). Free-form: server validates. + Channel string + + // URL-mode only fields. RunE-side validation rejects these if + // --from-url is not set (positional file path or --recursive). + Title string // --title: display title (URL mode) + FileType string // --file-type: extension hint for extension-less URLs + TagID string // --tag-id: associate the new knowledge entry with a tag } // UploadService is the narrow SDK surface this command depends on. @@ -70,20 +91,47 @@ has a generic name like "report.pdf" but you want to surface it as e.g. The three input modes (positional file / --recursive directory walk / --from-url remote ingest) are mutually exclusive - pass exactly one. -Use --recursive --glob to upload a directory tree (see Examples).`, +Use --recursive --glob to upload a directory tree (see Examples). + +Server-side ingestion knobs apply to all modes: + + --enable-multimodel Toggle multimodal extraction (image-in-PDF → text). + Unset ⇒ server default; pass true or false to override. + --metadata key=value Attach arbitrary key/value metadata. Repeatable. + Empty value allowed; duplicate keys ⇒ last-wins. + Malformed values (no '=', empty key) ⇒ + input.invalid_argument. + --channel Override the ingestion-channel tag (default "api"). + +URL mode (--from-url) additionally accepts --title, --file-type, and --tag-id. +Passing any of those without --from-url is rejected as input.invalid_argument.`, Example: ` weknora doc upload report.pdf weknora doc upload notes.md --kb a32a63ff-fb36-4874-bcaa-30f48570a694 weknora doc upload notes.md --kb my-kb weknora doc upload q3.pdf --name "Q3 Marketing Report.pdf" - weknora doc upload ./docs --recursive --glob '*.pdf' + weknora doc upload report.pdf --enable-multimodel --metadata team=alpha --metadata sprint=Q4 + weknora doc upload ./docs --recursive --glob '*.pdf' --metadata team=alpha weknora doc upload --from-url https://example.com/whitepaper.pdf - weknora doc upload --from-url https://example.com/article.html --name "Q3 Article"`, + weknora doc upload --from-url https://example.com/no-ext --file-type pdf --title "Whitepaper" + weknora doc upload --from-url https://example.com/article.html --name "Q3 Article" --tag-id tag_abc`, Args: cobra.MaximumNArgs(1), RunE: func(c *cobra.Command, args []string) error { jopts, err := cmdutil.CheckJSONFlags(c) if err != nil { return err } + // Translate the tri-state --enable-multimodel flag into the + // *bool the SDK expects. Cobra's BoolVar can't distinguish + // "unset" from "false", so we register a String flag and read + // Changed() + the raw value here. + if c.Flags().Changed("enable-multimodel") { + raw, _ := c.Flags().GetString("enable-multimodel") + v, perr := parseTriBool(raw) + if perr != nil { + return perr + } + opts.EnableMultimodel = &v + } if err := validateUploadFlags(opts, args); err != nil { return err } @@ -114,13 +162,71 @@ Use --recursive --glob to upload a directory tree (see Examples).`, cmd.Flags().BoolVar(&opts.Recursive, "recursive", false, "Treat the positional argument as a directory to walk") cmd.Flags().StringVar(&opts.Glob, "glob", "*", "Filename pattern to filter when --recursive (e.g. '*.pdf')") cmd.Flags().StringVar(&opts.FromURL, "from-url", "", "Ingest a remote `URL` (HTTP/HTTPS) instead of a local file") + // Tri-state flag: unset ⇒ server default, "true"/"false" override. The + // raw string is decoded into opts.EnableMultimodel in RunE. + cmd.Flags().String("enable-multimodel", "", "Toggle multimodal extraction (true|false); unset ⇒ server default") + cmd.Flags().Lookup("enable-multimodel").NoOptDefVal = "true" + cmd.Flags().StringSliceVar(&opts.Metadata, "metadata", nil, "Attach metadata `key=value` (repeatable; empty value allowed, last-wins on duplicate keys)") + cmd.Flags().StringVar(&opts.Channel, "channel", "", "Ingestion-channel tag recorded server-side (default \"api\")") + cmd.Flags().StringVar(&opts.Title, "title", "", "Display title for the new entry (--from-url only)") + cmd.Flags().StringVar(&opts.FileType, "file-type", "", "File-type hint such as \"pdf\" when the URL has no extension (--from-url only)") + cmd.Flags().StringVar(&opts.TagID, "tag-id", "", "Tag id to associate with the new entry (--from-url only)") cmdutil.AddJSONFlags(cmd, docUploadFields) return cmd } +// parseTriBool parses the raw --enable-multimodel string into a bool. Bare +// --enable-multimodel (no value) is treated as "true" via NoOptDefVal at +// registration time; callers gate on Changed() so an unset flag never gets +// here. An explicit empty string (e.g. --enable-multimodel="" from an +// uninterpolated shell variable) is rejected as input.invalid_argument +// rather than silently coerced. +func parseTriBool(raw string) (bool, error) { + switch strings.ToLower(strings.TrimSpace(raw)) { + case "true", "1", "yes": + return true, nil + case "false", "0", "no": + return false, nil + default: + return false, cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + fmt.Sprintf("--enable-multimodel expects true|false, got %q", raw)) + } +} + +// parseMetadataKV converts the raw --metadata key=value slice into a map. +// Empty values are allowed. Duplicate keys ⇒ last-wins. Returns nil when +// the slice is empty so callers pass nil through to the SDK unchanged. +func parseMetadataKV(raw []string) (map[string]string, error) { + if len(raw) == 0 { + return nil, nil + } + out := make(map[string]string, len(raw)) + for _, kv := range raw { + eq := strings.IndexByte(kv, '=') + if eq <= 0 { + return nil, cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + fmt.Sprintf("--metadata expects key=value (got %q)", kv)) + } + out[kv[:eq]] = kv[eq+1:] + } + return out, nil +} + +// effectiveChannel returns the channel string to send to the SDK. Empty +// opts.Channel falls back to the default "api" so the wire payload is +// identical to the pre-flag behavior. +func effectiveChannel(opts *UploadOptions) string { + if opts.Channel != "" { + return opts.Channel + } + return uploadChannel +} + // validateUploadFlags enforces mutual exclusion between the three input // modes (positional file path / --recursive directory walk / --from-url -// remote ingest) and validates the URL when --from-url is set. +// remote ingest) and validates the URL when --from-url is set. It also +// rejects the URL-mode-only flags (--title, --file-type, --tag-id) when +// --from-url isn't set so misuse fails fast with a typed code. func validateUploadFlags(opts *UploadOptions, args []string) error { hasPath := len(args) == 1 hasURL := opts.FromURL != "" @@ -139,17 +245,39 @@ func validateUploadFlags(opts *UploadOptions, args []string) error { return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "a file path is required (or pass --from-url)") } + // --title / --file-type / --tag-id are URL-mode only. Reject silently + // ignoring them in file mode to avoid the "set X, server did nothing" + // surprise. + if opts.Title != "" { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + "--title is only valid with --from-url") + } + if opts.FileType != "" { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + "--file-type is only valid with --from-url") + } + if opts.TagID != "" { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + "--tag-id is only valid with --from-url") + } return nil } // runUploadFromURL ingests a remote URL via SDK CreateKnowledgeFromURL. // `--name` becomes the FileName hint so the server's "known file extension" // detection upgrades crawl-mode to file-download-mode when appropriate. +// Server-side knobs (--enable-multimodel, --metadata via Title/TagID/FileType) +// propagate when set; the SDK request struct omits empty fields via +// `json:",omitempty"` tags so wire payload stays minimal. func runUploadFromURL(ctx context.Context, opts *UploadOptions, jopts *cmdutil.JSONOptions, svc UploadService, kbID string) error { req := sdk.CreateKnowledgeFromURLRequest{ - URL: opts.FromURL, - FileName: opts.Name, - Channel: uploadChannel, + URL: opts.FromURL, + FileName: opts.Name, + FileType: opts.FileType, + EnableMultimodel: opts.EnableMultimodel, + Title: opts.Title, + TagID: opts.TagID, + Channel: effectiveChannel(opts), } k, err := svc.CreateKnowledgeFromURL(ctx, kbID, req) if err != nil { @@ -209,7 +337,11 @@ func validateUploadPath(path string) error { } func runUpload(ctx context.Context, opts *UploadOptions, jopts *cmdutil.JSONOptions, svc UploadService, kbID, path string) error { - k, err := svc.CreateKnowledgeFromFile(ctx, kbID, path, nil /*metadata*/, nil /*enableMultimodel*/, opts.Name, uploadChannel) + meta, err := parseMetadataKV(opts.Metadata) + if err != nil { + return err + } + k, err := svc.CreateKnowledgeFromFile(ctx, kbID, path, meta, opts.EnableMultimodel, opts.Name, effectiveChannel(opts)) if err != nil { return cmdutil.WrapHTTP(err, "upload %s", path) } diff --git a/cli/cmd/doc/upload_recursive.go b/cli/cmd/doc/upload_recursive.go index 46425fa11..18e0f120b 100644 --- a/cli/cmd/doc/upload_recursive.go +++ b/cli/cmd/doc/upload_recursive.go @@ -32,6 +32,34 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, jopts *cmdutil Hint: "drop --name or upload files one at a time", } } + // URL-mode-only flags are not meaningful for a directory walk; reject + // them so misuse fails fast (mirrors the file-mode path's check in + // validateUploadFlags - that path runs before --recursive dispatches). + if opts.Title != "" { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: "--title is only valid with --from-url", + } + } + if opts.FileType != "" { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: "--file-type is only valid with --from-url", + } + } + if opts.TagID != "" { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: "--tag-id is only valid with --from-url", + } + } + // Parse --metadata up front so a malformed value aborts before the + // first SDK call - otherwise a typo in `key=value` would only surface + // per-file as repeated identical errors. + meta, err := parseMetadataKV(opts.Metadata) + if err != nil { + return err + } info, err := os.Stat(dir) if err != nil { if os.IsNotExist(err) { @@ -70,8 +98,9 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, jopts *cmdutil var uploaded, failed []uploadOutcome var firstFailCode cmdutil.ErrorCode + channel := effectiveChannel(opts) for _, p := range matches { - k, err := svc.CreateKnowledgeFromFile(ctx, kbID, p, nil, nil, "", uploadChannel) + k, err := svc.CreateKnowledgeFromFile(ctx, kbID, p, meta, opts.EnableMultimodel, "", channel) if err != nil { code := cmdutil.ClassifyHTTPError(err) if firstFailCode == "" { diff --git a/cli/cmd/doc/upload_recursive_test.go b/cli/cmd/doc/upload_recursive_test.go index 1a1c1265d..58d181ec9 100644 --- a/cli/cmd/doc/upload_recursive_test.go +++ b/cli/cmd/doc/upload_recursive_test.go @@ -25,16 +25,25 @@ type scriptedUploadSvc struct { err error } called []string + + // Captures from the most-recent call (every recursive iteration writes + // these; tests that want all-rows can extend to slices later). + lastMetadata map[string]string + lastEnableMultimodel *bool + lastChannel string } func (s *scriptedUploadSvc) CreateKnowledgeFromFile( _ context.Context, _, filePath string, - _ map[string]string, - _ *bool, - _, _ string, + metadata map[string]string, + enableMultimodel *bool, + _, channel string, ) (*sdk.Knowledge, error) { s.called = append(s.called, filepath.Base(filePath)) + s.lastMetadata = metadata + s.lastEnableMultimodel = enableMultimodel + s.lastChannel = channel r, ok := s.results[filepath.Base(filePath)] if !ok { return &sdk.Knowledge{ID: "doc_" + filepath.Base(filePath), FileName: filepath.Base(filePath)}, nil @@ -157,6 +166,68 @@ func TestUploadRecursive_RejectsNameFlag(t *testing.T) { assert.Contains(t, typed.Message, "--name") } +func TestUploadRecursive_PropagatesMultimodelAndMetadata(t *testing.T) { + _, _ = iostreams.SetForTest(t) + dir := t.TempDir() + mkTree(t, dir, "a.pdf") + + svc := &scriptedUploadSvc{} + mm := true + opts := &UploadOptions{ + Recursive: true, + Glob: "*", + EnableMultimodel: &mm, + Metadata: []string{"team=alpha"}, + Channel: "browser_extension", + } + require.NoError(t, runUploadRecursive(context.Background(), opts, nil, svc, "kb_xxx", dir)) + + require.NotNil(t, svc.lastEnableMultimodel) + assert.True(t, *svc.lastEnableMultimodel) + assert.Equal(t, map[string]string{"team": "alpha"}, svc.lastMetadata) + assert.Equal(t, "browser_extension", svc.lastChannel) +} + +func TestUploadRecursive_MetadataInvalid_NoCalls(t *testing.T) { + _, _ = iostreams.SetForTest(t) + dir := t.TempDir() + mkTree(t, dir, "a.pdf") + + svc := &scriptedUploadSvc{} + opts := &UploadOptions{Recursive: true, Glob: "*", Metadata: []string{"badformat"}} + err := runUploadRecursive(context.Background(), opts, nil, svc, "kb_xxx", dir) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Empty(t, svc.called, "must fail before any per-file call") +} + +func TestUploadRecursive_RejectsURLOnlyFlags(t *testing.T) { + _, _ = iostreams.SetForTest(t) + dir := t.TempDir() + mkTree(t, dir, "a.pdf") + for _, tc := range []struct { + name string + opts *UploadOptions + want string + }{ + {"title", &UploadOptions{Recursive: true, Glob: "*", Title: "x"}, "--title"}, + {"file-type", &UploadOptions{Recursive: true, Glob: "*", FileType: "pdf"}, "--file-type"}, + {"tag-id", &UploadOptions{Recursive: true, Glob: "*", TagID: "t"}, "--tag-id"}, + } { + t.Run(tc.name, func(t *testing.T) { + svc := &scriptedUploadSvc{} + err := runUploadRecursive(context.Background(), tc.opts, nil, svc, "kb_xxx", dir) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Contains(t, typed.Message, tc.want) + }) + } +} + func TestUploadRecursive_JSON_BareObject(t *testing.T) { out, _ := iostreams.SetForTest(t) dir := t.TempDir() diff --git a/cli/cmd/doc/upload_test.go b/cli/cmd/doc/upload_test.go index 01ab5e4bc..a3922ffc6 100644 --- a/cli/cmd/doc/upload_test.go +++ b/cli/cmd/doc/upload_test.go @@ -258,3 +258,206 @@ func TestValidateUploadFlags_NoPathOrURL_Rejected(t *testing.T) { require.ErrorAs(t, err, &typed) assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) } + +// --- C10 expanded flags: multimodel / metadata / channel / URL-mode extras --- + +func TestUpload_EnableMultimodel_Set_True(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "mm.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_mm", FileName: "mm.pdf"}} + mm := true + opts := &UploadOptions{EnableMultimodel: &mm} + require.NoError(t, runUpload(context.Background(), opts, nil, svc, "kb_xxx", path)) + require.NotNil(t, svc.got.enableMultimodel, "expected non-nil *bool when flag set") + assert.True(t, *svc.got.enableMultimodel) +} + +func TestUpload_EnableMultimodel_Set_False(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "mm.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_mm", FileName: "mm.pdf"}} + mm := false + opts := &UploadOptions{EnableMultimodel: &mm} + require.NoError(t, runUpload(context.Background(), opts, nil, svc, "kb_xxx", path)) + require.NotNil(t, svc.got.enableMultimodel, "explicit false must still surface as non-nil *bool") + assert.False(t, *svc.got.enableMultimodel) +} + +// TestParseTriBool pins the empty-string-rejects behavior. Bare +// --enable-multimodel maps to "true" via NoOptDefVal before the flag reaches +// parseTriBool, so an empty value here always indicates an explicit +// --enable-multimodel="" (e.g. uninterpolated $VAR). Silently coercing +// empty to true used to surprise users. +func TestParseTriBool(t *testing.T) { + for _, c := range []struct { + in string + want bool + wantErr bool + }{ + {"true", true, false}, + {"1", true, false}, + {"yes", true, false}, + {"false", false, false}, + {"0", false, false}, + {"no", false, false}, + {"", false, true}, // explicit empty rejected + {" ", false, true}, // whitespace rejected + {"maybe", false, true}, + } { + t.Run(c.in, func(t *testing.T) { + got, err := parseTriBool(c.in) + if c.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), "input.invalid_argument") + return + } + require.NoError(t, err) + assert.Equal(t, c.want, got) + }) + } +} + +func TestUpload_Metadata_ParseKV(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "m.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} + opts := &UploadOptions{Metadata: []string{"foo=bar", "baz=qux"}} + require.NoError(t, runUpload(context.Background(), opts, nil, svc, "kb_xxx", path)) + assert.Equal(t, map[string]string{"foo": "bar", "baz": "qux"}, svc.got.metadata) +} + +func TestUpload_Metadata_EmptyValueAllowed(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "m.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} + opts := &UploadOptions{Metadata: []string{"foo="}} + require.NoError(t, runUpload(context.Background(), opts, nil, svc, "kb_xxx", path)) + assert.Equal(t, map[string]string{"foo": ""}, svc.got.metadata) +} + +func TestUpload_Metadata_LastWins(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "m.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} + opts := &UploadOptions{Metadata: []string{"k=v1", "k=v2"}} + require.NoError(t, runUpload(context.Background(), opts, nil, svc, "kb_xxx", path)) + assert.Equal(t, map[string]string{"k": "v2"}, svc.got.metadata) +} + +func TestUpload_Metadata_InvalidFormat_NoEquals(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "m.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} + opts := &UploadOptions{Metadata: []string{"foo"}} + err := runUpload(context.Background(), opts, nil, svc, "kb_xxx", path) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) +} + +func TestUpload_Metadata_InvalidFormat_EmptyKey(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "m.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} + opts := &UploadOptions{Metadata: []string{"=bar"}} + err := runUpload(context.Background(), opts, nil, svc, "kb_xxx", path) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) +} + +func TestUpload_Channel_Override(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "c.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_c", FileName: "c.pdf"}} + opts := &UploadOptions{Channel: "browser_extension"} + require.NoError(t, runUpload(context.Background(), opts, nil, svc, "kb_xxx", path)) + assert.Equal(t, "browser_extension", svc.got.channel) +} + +func TestUpload_Channel_DefaultStillAPI(t *testing.T) { + _, _ = iostreams.SetForTest(t) + path := writeTempFile(t, "c.pdf") + svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_c", FileName: "c.pdf"}} + // Empty Channel is the runUpload contract for "use default". + opts := &UploadOptions{} + require.NoError(t, runUpload(context.Background(), opts, nil, svc, "kb_xxx", path)) + assert.Equal(t, uploadChannel, svc.got.channel) +} + +// URL-mode metadata happy paths + +func TestUploadFromURL_Title(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} + opts := &UploadOptions{FromURL: "https://example.com/a.pdf", Title: "My Title"} + require.NoError(t, runUploadFromURL(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "My Title", svc.got.urlReq.Title) +} + +func TestUploadFromURL_FileType(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} + opts := &UploadOptions{FromURL: "https://example.com/no-ext", FileType: "pdf"} + require.NoError(t, runUploadFromURL(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "pdf", svc.got.urlReq.FileType) +} + +func TestUploadFromURL_TagID(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} + opts := &UploadOptions{FromURL: "https://example.com/a.pdf", TagID: "tag_99"} + require.NoError(t, runUploadFromURL(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "tag_99", svc.got.urlReq.TagID) +} + +func TestUploadFromURL_EnableMultimodel_Forwarded(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} + mm := true + opts := &UploadOptions{FromURL: "https://example.com/a.pdf", EnableMultimodel: &mm} + require.NoError(t, runUploadFromURL(context.Background(), opts, nil, svc, "kb_xxx")) + require.NotNil(t, svc.got.urlReq.EnableMultimodel) + assert.True(t, *svc.got.urlReq.EnableMultimodel) +} + +func TestUploadFromURL_Channel_Override(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} + opts := &UploadOptions{FromURL: "https://example.com/a.pdf", Channel: "web"} + require.NoError(t, runUploadFromURL(context.Background(), opts, nil, svc, "kb_xxx")) + assert.Equal(t, "web", svc.got.urlReq.Channel) +} + +// URL-only flag misuse: error when used without --from-url. +// validateUploadFlags should reject --title/--file-type/--tag-id paired +// with a positional file path (i.e., no --from-url). + +func TestValidateUploadFlags_Title_RequiresFromURL(t *testing.T) { + err := validateUploadFlags(&UploadOptions{Title: "x"}, []string{"/tmp/x.pdf"}) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Contains(t, typed.Message, "--title") +} + +func TestValidateUploadFlags_FileType_RequiresFromURL(t *testing.T) { + err := validateUploadFlags(&UploadOptions{FileType: "pdf"}, []string{"/tmp/x.pdf"}) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Contains(t, typed.Message, "--file-type") +} + +func TestValidateUploadFlags_TagID_RequiresFromURL(t *testing.T) { + err := validateUploadFlags(&UploadOptions{TagID: "tag_x"}, []string{"/tmp/x.pdf"}) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Contains(t, typed.Message, "--tag-id") +} diff --git a/cli/cmd/doc/view.go b/cli/cmd/doc/view.go index eacb6d753..3f5ad4c21 100644 --- a/cli/cmd/doc/view.go +++ b/cli/cmd/doc/view.go @@ -65,21 +65,58 @@ func runView(ctx context.Context, opts *ViewOptions, jopts *cmdutil.JSONOptions, w := iostreams.IO.Out fmt.Fprintf(w, "ID: %s\n", doc.ID) fmt.Fprintf(w, "NAME: %s\n", text.KnowledgeDisplayName(doc.FileName, doc.Title, doc.ID)) + // Title is rendered as a separate line only when it adds info over + // NAME (i.e. FileName is set AND differs from Title). When FileName + // is empty, KnowledgeDisplayName already used Title for NAME so a + // duplicate TITLE line would be noise. + if doc.Title != "" && doc.FileName != "" && doc.Title != doc.FileName { + fmt.Fprintf(w, "TITLE: %s\n", doc.Title) + } if doc.KnowledgeBaseID != "" { fmt.Fprintf(w, "KB: %s\n", doc.KnowledgeBaseID) } + if doc.TagID != "" { + fmt.Fprintf(w, "TAG: %s\n", doc.TagID) + } + if doc.Description != "" { + fmt.Fprintf(w, "DESC: %s\n", doc.Description) + } if doc.FileType != "" { fmt.Fprintf(w, "TYPE: %s\n", doc.FileType) } + if doc.Source != "" { + fmt.Fprintf(w, "SOURCE: %s\n", doc.Source) + } + if doc.Channel != "" { + fmt.Fprintf(w, "CHANNEL: %s\n", doc.Channel) + } if doc.FileSize > 0 { fmt.Fprintf(w, "SIZE: %s\n", formatSize(doc.FileSize)) } + if doc.StorageSize > 0 { + fmt.Fprintf(w, "STORAGE: %s\n", formatSize(doc.StorageSize)) + } if doc.ParseStatus != "" { fmt.Fprintf(w, "STATUS: %s\n", doc.ParseStatus) } + if doc.SummaryStatus != "" { + fmt.Fprintf(w, "SUMMARY: %s\n", doc.SummaryStatus) + } + if doc.EnableStatus != "" { + fmt.Fprintf(w, "ENABLED: %s\n", doc.EnableStatus) + } if doc.EmbeddingModelID != "" { fmt.Fprintf(w, "EMBEDDING: %s\n", doc.EmbeddingModelID) } + if doc.FileHash != "" { + // Git-SHA-style 12-char prefix is enough for de-duplication + // while keeping the line short. + h := doc.FileHash + if len(h) > 12 { + h = h[:12] + } + fmt.Fprintf(w, "HASH: %s\n", h) + } if !doc.CreatedAt.IsZero() { fmt.Fprintf(w, "CREATED: %s\n", doc.CreatedAt.Format("2006-01-02 15:04:05")) } diff --git a/cli/cmd/doc/view_test.go b/cli/cmd/doc/view_test.go index 8a92125bf..81f32eaf7 100644 --- a/cli/cmd/doc/view_test.go +++ b/cli/cmd/doc/view_test.go @@ -122,3 +122,143 @@ func TestView_NotFound_ClassifiedAs404(t *testing.T) { t.Errorf("expected resource.not_found, got %v", err) } } + +// --- expanded human render: title/desc/source/channel/etc. --- + +func TestView_Title_RendersWhenDifferentFromFileName(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ + ID: "doc_t", FileName: "raw.pdf", Title: "Quarterly Plan", + }} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_t"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "TITLE:") || !strings.Contains(got, "Quarterly Plan") { + t.Errorf("expected TITLE line:\n%s", got) + } +} + +// When Title and FileName are equal, the TITLE line is redundant with NAME +// and should be omitted. +func TestView_Title_OmittedWhenSameAsFileName(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ + ID: "doc_t", FileName: "policy.pdf", Title: "policy.pdf", + }} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_t"); err != nil { + t.Fatalf("runView: %v", err) + } + for _, l := range strings.Split(out.String(), "\n") { + if strings.HasPrefix(l, "TITLE:") { + t.Errorf("TITLE line should be omitted when same as filename: %q", l) + } + } +} + +func TestView_Description(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ID: "doc_d", FileName: "x.pdf", Description: "Annual review"}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_d"); err != nil { + t.Fatalf("runView: %v", err) + } + if !strings.Contains(out.String(), "Annual review") { + t.Errorf("expected description text:\n%s", out.String()) + } +} + +func TestView_SourceAndChannel(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ + ID: "doc_s", FileName: "x.pdf", Source: "https://example.com/x", Channel: "web", + }} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_s"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + for _, want := range []string{"SOURCE:", "https://example.com/x", "CHANNEL:", "web"} { + if !strings.Contains(got, want) { + t.Errorf("missing %q in:\n%s", want, got) + } + } +} + +func TestView_SummaryAndEnableStatus(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ + ID: "doc_st", FileName: "x.pdf", + SummaryStatus: "completed", + EnableStatus: "disabled", + }} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_st"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + for _, want := range []string{"SUMMARY:", "completed", "ENABLED:", "disabled"} { + if !strings.Contains(got, want) { + t.Errorf("missing %q in:\n%s", want, got) + } + } +} + +func TestView_TagID(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ID: "doc_t", FileName: "x.pdf", TagID: "tag_abc"}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_t"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "TAG:") || !strings.Contains(got, "tag_abc") { + t.Errorf("expected TAG line:\n%s", got) + } +} + +func TestView_StorageSize_Human(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ID: "doc_sz", FileName: "x.pdf", StorageSize: 2 * 1024 * 1024}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_sz"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "STORAGE:") || !strings.Contains(got, "MB") { + t.Errorf("expected STORAGE line with human-readable bytes:\n%s", got) + } +} + +func TestView_FileHash_Prefix12(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ + ID: "doc_h", + FileName: "x.pdf", + FileHash: "abcdef1234567890fedcba0987654321", + }} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_h"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "HASH:") || !strings.Contains(got, "abcdef123456") { + t.Errorf("expected HASH line with 12-char prefix:\n%s", got) + } + if strings.Contains(got, "abcdef1234567890fedcba0987654321") { + t.Errorf("full hash should be truncated, got:\n%s", got) + } +} + +func TestView_ErrorMessage_WarnPrefix(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewSvc{doc: &sdk.Knowledge{ + ID: "doc_e", FileName: "x.pdf", ErrorMessage: "parser failed at offset 4096", + }} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "doc_e"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "parser failed at offset 4096") { + t.Errorf("expected error message rendered:\n%s", got) + } + // Either ERROR: or a WARN-prefixed label is acceptable as a "warn" + // signal — assert at least one. + if !strings.Contains(got, "ERROR:") && !strings.Contains(got, "WARN") { + t.Errorf("expected ERROR or WARN prefix on error line:\n%s", got) + } +} diff --git a/cli/cmd/kb/view.go b/cli/cmd/kb/view.go index 81d54fc22..38cbefba8 100644 --- a/cli/cmd/kb/view.go +++ b/cli/cmd/kb/view.go @@ -63,18 +63,38 @@ func runView(ctx context.Context, opts *ViewOptions, jopts *cmdutil.JSONOptions, if jopts.Enabled() { return jopts.Emit(iostreams.IO.Out, kb) } - // Human: KEY: VALUE + // Human: KEY: VALUE. Nested config structs (chunking_config, vlm_config, + // etc.) are intentionally omitted from the human render — those are for + // `--json | jq '.chunking_config'` workflows. w := iostreams.IO.Out fmt.Fprintf(w, "ID: %s\n", kb.ID) fmt.Fprintf(w, "NAME: %s\n", kb.Name) + if kb.Type != "" { + fmt.Fprintf(w, "TYPE: %s\n", kb.Type) + } if kb.Description != "" { fmt.Fprintf(w, "DESC: %s\n", kb.Description) } + if kb.IsPinned { + fmt.Fprintf(w, "PINNED: yes\n") + } + if kb.IsTemporary { + fmt.Fprintf(w, "TEMPORARY: yes\n") + } fmt.Fprintf(w, "DOCS: %s\n", text.Pluralize(int(kb.KnowledgeCount), "doc")) fmt.Fprintf(w, "CHUNKS: %s\n", text.Pluralize(int(kb.ChunkCount), "chunk")) + if kb.IsProcessing { + fmt.Fprintf(w, "PROCESSING: %s\n", text.Pluralize(int(kb.ProcessingCount), "doc")) + } if kb.EmbeddingModelID != "" { fmt.Fprintf(w, "EMBEDDING: %s\n", kb.EmbeddingModelID) } + if kb.SummaryModelID != "" { + fmt.Fprintf(w, "SUMMARY MODEL: %s\n", kb.SummaryModelID) + } + if !kb.CreatedAt.IsZero() { + fmt.Fprintf(w, "CREATED: %s\n", kb.CreatedAt.Format("2006-01-02 15:04:05")) + } if !kb.UpdatedAt.IsZero() { // Detail page favors absolute time; FuzzyAgo is reserved for list views. fmt.Fprintf(w, "UPDATED: %s\n", kb.UpdatedAt.Format("2006-01-02 15:04:05")) diff --git a/cli/cmd/kb/view_test.go b/cli/cmd/kb/view_test.go index 4d2c0c7cc..c397f5f99 100644 --- a/cli/cmd/kb/view_test.go +++ b/cli/cmd/kb/view_test.go @@ -5,6 +5,7 @@ import ( "errors" "strings" "testing" + "time" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" @@ -62,3 +63,105 @@ func TestGet_NotFound(t *testing.T) { t.Errorf("expected resource.not_found, got %v", err) } } + +// --- expanded human render: badges + extra KV lines --- + +func TestView_Pinned_RendersPinnedLine(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ID: "kb1", Name: "Pinned", IsPinned: true}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb1"); err != nil { + t.Fatalf("runView: %v", err) + } + if !strings.Contains(out.String(), "PINNED:") { + t.Errorf("expected PINNED line for IsPinned=true:\n%s", out.String()) + } +} + +func TestView_NotPinned_OmitsPinnedLine(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ID: "kb1", Name: "Plain", IsPinned: false}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb1"); err != nil { + t.Fatalf("runView: %v", err) + } + for _, l := range strings.Split(out.String(), "\n") { + if strings.HasPrefix(l, "PINNED:") { + t.Errorf("PINNED line should be omitted when IsPinned=false: %q", l) + } + } +} + +func TestView_Temporary_RendersTempLine(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ID: "kb_t", Name: "Tmp", IsTemporary: true}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb_t"); err != nil { + t.Fatalf("runView: %v", err) + } + if !strings.Contains(out.String(), "TEMPORARY:") { + t.Errorf("expected TEMPORARY line:\n%s", out.String()) + } +} + +func TestView_SummaryModel(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ID: "kb1", Name: "X", SummaryModelID: "summary-model-x"}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb1"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "SUMMARY MODEL:") || !strings.Contains(got, "summary-model-x") { + t.Errorf("expected SUMMARY MODEL line:\n%s", got) + } +} + +func TestView_TypeAndSource(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ID: "kb1", Name: "X", Type: "general", Description: "d"}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb1"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "TYPE:") || !strings.Contains(got, "general") { + t.Errorf("expected TYPE line for non-empty Type:\n%s", got) + } +} + +func TestView_Processing(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ID: "kb_p", Name: "Busy", IsProcessing: true, ProcessingCount: 3}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb_p"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "PROCESSING:") || !strings.Contains(got, "3") { + t.Errorf("expected PROCESSING line with count:\n%s", got) + } +} + +func TestView_NotProcessing_OmitsProcessingLine(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ID: "kb_idle", Name: "Idle", IsProcessing: false}} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb_idle"); err != nil { + t.Fatalf("runView: %v", err) + } + for _, l := range strings.Split(out.String(), "\n") { + if strings.HasPrefix(l, "PROCESSING:") { + t.Errorf("PROCESSING line should be omitted: %q", l) + } + } +} + +func TestView_CreatedAt_AlwaysRendered(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeGetSvc{kb: &sdk.KnowledgeBase{ + ID: "kb1", Name: "X", + CreatedAt: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC), + }} + if err := runView(context.Background(), &ViewOptions{}, nil, svc, "kb1"); err != nil { + t.Fatalf("runView: %v", err) + } + got := out.String() + if !strings.Contains(got, "CREATED:") || !strings.Contains(got, "2026-01-01") { + t.Errorf("expected CREATED line:\n%s", got) + } +} diff --git a/cli/cmd/search/docs.go b/cli/cmd/search/docs.go index bbd1a9eac..519d68dcc 100644 --- a/cli/cmd/search/docs.go +++ b/cli/cmd/search/docs.go @@ -16,9 +16,9 @@ import ( ) // docsPageSize is the default --page-size on `search docs`: how many -// entries to pull per ListKnowledge round-trip when paging through a KB -// to filter client-side. Server caps page_size at 1000 (per the doc/list -// bound this branch already added). Tunable via --page-size in 1..1000. +// entries to pull per ListKnowledgeWithFilter round-trip. The server +// applies the keyword filter pre-pagination, so most KBs return in a +// single page even at conservative sizes. Server caps page_size at 1000. const docsPageSize = 200 // docsMaxPageSize bounds the --page-size flag, matching session/doc list canon. @@ -39,10 +39,10 @@ type DocsSearchOptions struct { KB string // raw --kb (UUID or name) KBID string // resolved id; populated before listing Limit int - // PageSize is the server batch size per ListKnowledge call (1..1000, - // default 200). Tunable so a caller searching a small KB can fetch - // everything in one round-trip, or a caller on flaky network can - // shorten the batch. + // PageSize is the server batch size per ListKnowledgeWithFilter call + // (1..1000, default 200). Tunable so a caller searching a small KB + // can fetch everything in one round-trip, or a caller on flaky + // network can shorten the batch. PageSize int // AllPages walks server pages internally until total exhausted or // --limit accumulated. Default true preserves v0.4 behavior; setting @@ -51,25 +51,32 @@ type DocsSearchOptions struct { } // DocsSearchService is the narrow SDK surface this command depends on. -// Server has no fuzzy-document-name endpoint, so the CLI pages through -// ListKnowledge and filters by Title / FileName client-side. +// The server applies the keyword filter pre-pagination via the +// ?keyword= query param, so the CLI just forwards opts.Query as +// filter.Keyword and accumulates the (already-filtered) pages. type DocsSearchService interface { - ListKnowledge(ctx context.Context, kbID string, page, pageSize int, tagID string) ([]sdk.Knowledge, int64, error) + ListKnowledgeWithFilter(ctx context.Context, kbID string, page, pageSize int, filter sdk.KnowledgeListFilter) ([]sdk.Knowledge, int64, error) } // NewCmdDocs builds `weknora search docs "" --kb `. // Pages through the KB's documents and surfaces every entry whose title -// or filename contains the query (case-insensitive). Useful for finding -// a specific upload to download or delete. +// or file_name contains the query as a server-side case-sensitive LIKE +// match. Useful for finding a specific upload to download or delete. func NewCmdDocs(f *cmdutil.Factory) *cobra.Command { opts := &DocsSearchOptions{} cmd := &cobra.Command{ Use: `docs ""`, - Short: "Find documents in a knowledge base by name (client-side substring match)", - Long: `Pages through the KB's documents and surfaces every entry whose title or -filename contains the query (case-insensitive). Useful for finding a + Short: "Find documents in a knowledge base by keyword (server-side filter)", + Long: `Pages through the KB's documents, forwarding the query as the server-side +keyword filter (matched against title / file_name). Useful for finding a specific upload to download or delete by id. +The query is a case-sensitive server-side LIKE filter (the server runs +` + "`LIKE %keyword%`" + ` against title and file_name). For case-insensitive +matching, lower-case the query yourself, e.g. +` + "`weknora search docs \"$(printf %s YOUR_QUERY | tr 'A-Z' 'a-z')\"`" + `, or +fall back to ` + "`weknora api`" + ` with a custom filter. + By default, --all-pages=true walks every server page until --limit is reached or the KB is exhausted (matching v0.4 behavior). Pass --all-pages=false to stop after one page.`, @@ -115,24 +122,24 @@ func runDocsSearch(ctx context.Context, opts *DocsSearchOptions, jopts *cmdutil. return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("--page-size must be in 1..%d, got %d", docsMaxPageSize, opts.PageSize)) } - needle := strings.ToLower(opts.Query) + filter := sdk.KnowledgeListFilter{Keyword: opts.Query} var matches []sdk.Knowledge // Page through the KB until limit matches found or pagination exhausted. + // The server applies the keyword filter pre-pagination, so every item + // returned is already a match - no client-side filter needed. // --all-pages=true (default) walks every server page; --all-pages=false // stops after the first page. The server returns total; stop when // page*pageSize >= total. for page := 1; ; page++ { - items, total, err := svc.ListKnowledge(ctx, opts.KBID, page, opts.PageSize, "") + items, total, err := svc.ListKnowledgeWithFilter(ctx, opts.KBID, page, opts.PageSize, filter) if err != nil { return cmdutil.WrapHTTP(err, "list documents") } for _, k := range items { - if matchKnowledge(k, needle) { - matches = append(matches, k) - if opts.Limit > 0 && len(matches) >= opts.Limit { - goto done - } + matches = append(matches, k) + if opts.Limit > 0 && len(matches) >= opts.Limit { + goto done } } if !opts.AllPages { @@ -164,12 +171,6 @@ done: return tw.Flush() } -// matchKnowledge reports whether title or filename contains needle (already -// lowercased by caller). -func matchKnowledge(k sdk.Knowledge, needle string) bool { - return text.ContainsFold(needle, k.Title, k.FileName) -} - // sortKnowledgeByRecency sorts in place by UpdatedAt desc. func sortKnowledgeByRecency(items []sdk.Knowledge) { sort.Slice(items, func(i, j int) bool { diff --git a/cli/cmd/search/docs_test.go b/cli/cmd/search/docs_test.go index 97744471c..1b5c5f2c9 100644 --- a/cli/cmd/search/docs_test.go +++ b/cli/cmd/search/docs_test.go @@ -15,17 +15,20 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// fakeDocsSearchSvc scripts paginated ListKnowledge responses. Pages are -// indexed 1-based; items keyed by page. +// fakeDocsSearchSvc scripts paginated ListKnowledgeWithFilter responses. +// Pages are indexed 1-based; items keyed by page. The last-received filter +// is captured so tests can assert opts.Query was threaded as filter.Keyword. type fakeDocsSearchSvc struct { - pages map[int][]sdk.Knowledge - total int64 - err error - calls []int // page numbers requested, for assertions + pages map[int][]sdk.Knowledge + total int64 + err error + calls []int // page numbers requested, for assertions + lastFilter sdk.KnowledgeListFilter } -func (f *fakeDocsSearchSvc) ListKnowledge(_ context.Context, kbID string, page, pageSize int, tagID string) ([]sdk.Knowledge, int64, error) { +func (f *fakeDocsSearchSvc) ListKnowledgeWithFilter(_ context.Context, kbID string, page, pageSize int, filter sdk.KnowledgeListFilter) ([]sdk.Knowledge, int64, error) { f.calls = append(f.calls, page) + f.lastFilter = filter if f.err != nil { return nil, 0, f.err } @@ -34,21 +37,23 @@ func (f *fakeDocsSearchSvc) ListKnowledge(_ context.Context, kbID string, page, func TestDocsSearch_Substring(t *testing.T) { out, _ := iostreams.SetForTest(t) + // Server applies the keyword filter pre-pagination; the fake simulates + // that by only returning the matching items (d1/d3, not d2). svc := &fakeDocsSearchSvc{ pages: map[int][]sdk.Knowledge{ 1: { {ID: "d1", Title: "Q3 Forecast", FileName: "q3.pdf", UpdatedAt: mustTime(t, "2026-05-10T00:00:00Z")}, - {ID: "d2", Title: "Random Notes", FileName: "notes.md", UpdatedAt: mustTime(t, "2026-05-12T00:00:00Z")}, {ID: "d3", Title: "Q3 retro", FileName: "retro.pdf", UpdatedAt: mustTime(t, "2026-05-11T00:00:00Z")}, }, }, - total: 3, + total: 2, } require.NoError(t, runDocsSearch(context.Background(), &DocsSearchOptions{Query: "q3", KBID: "kb1", Limit: 20, PageSize: docsPageSize, AllPages: true}, nil, svc)) + assert.Equal(t, "q3", svc.lastFilter.Keyword, "query must be threaded as filter.Keyword") got := out.String() assert.Contains(t, got, "d1") assert.Contains(t, got, "d3") - assert.NotContains(t, got, "d2") // "Random Notes" doesn't contain q3 + assert.NotContains(t, got, "d2") } func TestDocsSearch_MatchesFileName(t *testing.T) { @@ -61,20 +66,24 @@ func TestDocsSearch_MatchesFileName(t *testing.T) { assert.Contains(t, out.String(), "d1") } +// TestDocsSearch_PaginatesUntilTotal walks server-paginated results. +// Server-side filter has already been applied, so every returned item +// is in the result set; the runner just walks pages until total exhausted +// or --limit hit. With limit > total matches, we expect 2 pages. func TestDocsSearch_PaginatesUntilTotal(t *testing.T) { out, _ := iostreams.SetForTest(t) page1 := make([]sdk.Knowledge, docsPageSize) for i := range page1 { - page1[i] = sdk.Knowledge{ID: "p1", Title: "no match"} + page1[i] = sdk.Knowledge{ID: "p1", Title: "needle"} } page2 := []sdk.Knowledge{{ID: "found", Title: "needle here"}} svc := &fakeDocsSearchSvc{ pages: map[int][]sdk.Knowledge{1: page1, 2: page2}, total: int64(docsPageSize) + 1, } - require.NoError(t, runDocsSearch(context.Background(), &DocsSearchOptions{Query: "needle", KBID: "kb1", Limit: 20, PageSize: docsPageSize, AllPages: true}, nil, svc)) + require.NoError(t, runDocsSearch(context.Background(), &DocsSearchOptions{Query: "needle", KBID: "kb1", Limit: docsPageSize + 1, PageSize: docsPageSize, AllPages: true}, nil, svc)) assert.Contains(t, out.String(), "found") - assert.Equal(t, []int{1, 2}, svc.calls, "must page past the first batch when no match on page 1") + assert.Equal(t, []int{1, 2}, svc.calls, "must page past the first batch when more items reported") } func TestDocsSearch_StopsAtLimit(t *testing.T) { @@ -145,6 +154,22 @@ func TestSearchDocs_AllPagesFalse_StopsAtFirstPage(t *testing.T) { assert.Len(t, svc.calls, 1, "must stop at first page when --all-pages=false") } +// TestSearchDocs_KeywordPassedToFilter pins the v0.5 switch from client-side +// substring filtering to server-side ?keyword= via ListKnowledgeWithFilter. +// The query argument must arrive on the filter struct (not a discarded +// client-side variable). +func TestSearchDocs_KeywordPassedToFilter(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeDocsSearchSvc{pages: map[int][]sdk.Knowledge{1: {{ID: "d1"}}}, total: 1} + require.NoError(t, runDocsSearch(context.Background(), &DocsSearchOptions{Query: "my-query", KBID: "kb1", Limit: 20, PageSize: docsPageSize, AllPages: true}, nil, svc)) + assert.Equal(t, "my-query", svc.lastFilter.Keyword, "Query must be threaded as filter.Keyword on ListKnowledgeWithFilter") + // Other filter fields must be empty - search docs only forwards the keyword. + assert.Empty(t, svc.lastFilter.ParseStatus) + assert.Empty(t, svc.lastFilter.FileType) + assert.Empty(t, svc.lastFilter.Source) + assert.Empty(t, svc.lastFilter.TagID) +} + // TestSearchDocs_PageSizeBound asserts the 1..1000 range guard mirrors the // session/doc list canon. Out-of-range values must produce // input.invalid_argument and never reach the SDK. diff --git a/cli/cmd/session/view.go b/cli/cmd/session/view.go index ace973439..783d5e38e 100644 --- a/cli/cmd/session/view.go +++ b/cli/cmd/session/view.go @@ -12,30 +12,59 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) +const ( + defaultFullLimit = 50 + maxFullLimit = 1000 +) + // sessionViewFields enumerates the fields surfaced for `--json` discovery on -// `session view`. Mirrors sdk.Session json tags. +// `session view`. Mirrors sdk.Session json tags; adds the synthesized +// `messages` projection surfaced by `--full`. var sessionViewFields = []string{ "id", "tenant_id", "title", "description", "created_at", "updated_at", + "messages", } -type ViewOptions struct{} +type ViewOptions struct { + // Full instructs runView to fetch chat history via LoadMessages and + // render it after the session metadata. + Full bool + // Limit caps the number of messages loaded when Full is true. + // Must be 1..maxFullLimit. + Limit int + // LimitSet records whether the caller explicitly set --limit, so we + // can reject `--limit` without `--full` (vs. silently ignoring the + // default). + LimitSet bool +} -// ViewService is the narrow SDK surface this command depends on. +// ViewService is the narrow SDK surface this command depends on. LoadMessages +// is only invoked under --full but lives on the same interface so the runView +// dependency surface stays minimal. type ViewService interface { GetSession(ctx context.Context, id string) (*sdk.Session, error) + LoadMessages(ctx context.Context, sessionID string, limit int, beforeTime *time.Time) ([]sdk.Message, error) } // NewCmdView builds `weknora session view `. Renders session metadata -// only (title/description/timestamps). Full chat-history retrieval is a -// separate concern (the SDK has LoadMessages / GetMessagesBefore for it); -// surfacing it as `session view --full` is queued for v0.6. +// only by default. With `--full`, also loads the chat history via +// `LoadMessages` and renders messages (or projects them into the JSON +// payload under `messages`). func NewCmdView(f *cmdutil.Factory) *cobra.Command { - opts := &ViewOptions{} + opts := &ViewOptions{Limit: defaultFullLimit} cmd := &cobra.Command{ Use: "view ", Short: "Show a chat session by ID", - Args: cobra.ExactArgs(1), + Long: `Show a chat session. + +By default renders the session metadata (id, title, description, timestamps). + +Pass --full to also load the chat history (LoadMessages SDK call). Use +--limit to cap the number of messages loaded (1..1000, default 50). +--limit without --full is rejected as input.invalid_argument.`, + Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { + opts.LimitSet = c.Flags().Changed("limit") jopts, err := cmdutil.CheckJSONFlags(c) if err != nil { return err @@ -47,18 +76,58 @@ func NewCmdView(f *cmdutil.Factory) *cobra.Command { return runView(c.Context(), opts, jopts, cli, args[0]) }, } + cmd.Flags().BoolVar(&opts.Full, "full", false, "Also load chat history via LoadMessages") + cmd.Flags().IntVar(&opts.Limit, "limit", defaultFullLimit, "Max messages to load when --full is set (1..1000)") cmdutil.AddJSONFlags(cmd, sessionViewFields) return cmd } func runView(ctx context.Context, opts *ViewOptions, jopts *cmdutil.JSONOptions, svc ViewService, id string) error { + if !opts.Full && opts.LimitSet { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: "--limit requires --full", + } + } + if opts.Full { + if opts.Limit < 1 || opts.Limit > maxFullLimit { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: fmt.Sprintf("--limit must be in 1..%d, got %d", maxFullLimit, opts.Limit), + } + } + } + s, err := svc.GetSession(ctx, id) if err != nil { return cmdutil.WrapHTTP(err, "get session %q", id) } - if jopts.Enabled() { - return jopts.Emit(iostreams.IO.Out, s) + + var msgs []sdk.Message + if opts.Full { + msgs, err = svc.LoadMessages(ctx, id, opts.Limit, nil) + if err != nil { + return cmdutil.WrapHTTP(err, "load messages for session %q", id) + } + if msgs == nil { + msgs = []sdk.Message{} + } } + + if jopts.Enabled() { + if !opts.Full { + return jopts.Emit(iostreams.IO.Out, s) + } + // Project session + messages into a single bare object. Use the + // SDK json tags via an embedded *Session so existing keys stay + // stable. + payload := struct { + *sdk.Session + Messages []sdk.Message `json:"messages"` + }{Session: s, Messages: msgs} + return jopts.Emit(iostreams.IO.Out, payload) + } + w := iostreams.IO.Out fmt.Fprintf(w, "ID: %s\n", s.ID) if s.Title != "" { @@ -77,6 +146,22 @@ func runView(ctx context.Context, opts *ViewOptions, jopts *cmdutil.JSONOptions, } else if s.UpdatedAt != "" { fmt.Fprintf(w, "UPDATED: %s\n", s.UpdatedAt) } + + if opts.Full { + fmt.Fprintln(w) + fmt.Fprintf(w, "Messages (%d):\n", len(msgs)) + for _, m := range msgs { + fmt.Fprintln(w) + ts := "" + if !m.CreatedAt.IsZero() { + ts = " " + m.CreatedAt.Format("2006-01-02 15:04:05") + } + fmt.Fprintf(w, "[%s]%s\n", m.Role, ts) + if m.Content != "" { + fmt.Fprintln(w, m.Content) + } + } + } return nil } diff --git a/cli/cmd/session/view_test.go b/cli/cmd/session/view_test.go index d2d440bd3..d4db9e973 100644 --- a/cli/cmd/session/view_test.go +++ b/cli/cmd/session/view_test.go @@ -5,6 +5,7 @@ import ( "errors" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -14,11 +15,18 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// fakeViewService scripts a GetSession response. +// fakeViewService scripts a GetSession + LoadMessages response. type fakeViewService struct { - s *sdk.Session - err error - gotID string + s *sdk.Session + err error + gotID string + msgs []sdk.Message + msgsErr error + loadCall struct { + sessionID string + limit int + called bool + } } func (f *fakeViewService) GetSession(_ context.Context, id string) (*sdk.Session, error) { @@ -26,6 +34,13 @@ func (f *fakeViewService) GetSession(_ context.Context, id string) (*sdk.Session return f.s, f.err } +func (f *fakeViewService) LoadMessages(_ context.Context, sessionID string, limit int, _ *time.Time) ([]sdk.Message, error) { + f.loadCall.called = true + f.loadCall.sessionID = sessionID + f.loadCall.limit = limit + return f.msgs, f.msgsErr +} + func TestView_Human(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeViewService{s: &sdk.Session{ @@ -74,3 +89,88 @@ func TestView_OmitsEmptyDescription(t *testing.T) { } } } + +// --- --full / --limit tests --- + +func TestView_Full_LoadsMessages(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewService{ + s: &sdk.Session{ID: "s_abc", Title: "Chat"}, + msgs: []sdk.Message{ + {ID: "m1", Role: "user", Content: "What is RAG?", CreatedAt: time.Date(2026, 5, 15, 14, 32, 0, 0, time.UTC)}, + {ID: "m2", Role: "assistant", Content: "RAG stands for retrieval-augmented generation.", CreatedAt: time.Date(2026, 5, 15, 14, 32, 5, 0, time.UTC)}, + }, + } + require.NoError(t, runView(context.Background(), &ViewOptions{Full: true, Limit: 50}, nil, svc, "s_abc")) + got := out.String() + assert.True(t, svc.loadCall.called, "expected LoadMessages to be called") + assert.Equal(t, "s_abc", svc.loadCall.sessionID) + assert.Equal(t, 50, svc.loadCall.limit) + for _, want := range []string{"Messages (2)", "[user]", "[assistant]", "What is RAG?", "retrieval-augmented generation"} { + assert.Contains(t, got, want) + } +} + +func TestView_Full_NoMessages(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewService{ + s: &sdk.Session{ID: "s_empty", Title: "Empty"}, + msgs: []sdk.Message{}, + } + require.NoError(t, runView(context.Background(), &ViewOptions{Full: true, Limit: 50}, nil, svc, "s_empty")) + got := out.String() + assert.Contains(t, got, "Messages (0)") +} + +func TestView_Full_LimitInvalid_Zero(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeViewService{s: &sdk.Session{ID: "s"}} + err := runView(context.Background(), &ViewOptions{Full: true, Limit: 0}, nil, svc, "s") + require.Error(t, err) + assert.Contains(t, err.Error(), "input.invalid_argument") +} + +func TestView_Full_LimitInvalid_TooLarge(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeViewService{s: &sdk.Session{ID: "s"}} + err := runView(context.Background(), &ViewOptions{Full: true, Limit: 1001}, nil, svc, "s") + require.Error(t, err) + assert.Contains(t, err.Error(), "input.invalid_argument") +} + +// --limit without --full is rejected with input.invalid_argument — same +// pattern as `--title` requires `--from-url` in `doc upload`. +func TestView_LimitWithoutFull(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeViewService{s: &sdk.Session{ID: "s"}} + err := runView(context.Background(), &ViewOptions{Full: false, Limit: 100, LimitSet: true}, nil, svc, "s") + require.Error(t, err) + assert.Contains(t, err.Error(), "input.invalid_argument") + assert.Contains(t, err.Error(), "--limit") + assert.Contains(t, err.Error(), "--full") +} + +func TestView_Full_JSON_HasMessages(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewService{ + s: &sdk.Session{ID: "s_abc", Title: "T"}, + msgs: []sdk.Message{ + {ID: "m1", Role: "user", Content: "hi"}, + }, + } + require.NoError(t, runView(context.Background(), &ViewOptions{Full: true, Limit: 50}, &cmdutil.JSONOptions{}, svc, "s_abc")) + body := out.String() + assert.Contains(t, body, `"messages":`) + assert.Contains(t, body, `"id":"m1"`) + assert.Contains(t, body, `"role":"user"`) +} + +// Without --full, the LoadMessages SDK call must not fire and the JSON +// payload must not contain a `messages` key. +func TestView_NoFull_DoesNotCallLoadMessages(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeViewService{s: &sdk.Session{ID: "s_abc"}} + require.NoError(t, runView(context.Background(), &ViewOptions{}, &cmdutil.JSONOptions{}, svc, "s_abc")) + assert.False(t, svc.loadCall.called, "LoadMessages must not be called without --full") + assert.NotContains(t, out.String(), `"messages":`) +} diff --git a/cli/internal/mcp/tools.go b/cli/internal/mcp/tools.go index e75326085..8a38a3f30 100644 --- a/cli/internal/mcp/tools.go +++ b/cli/internal/mcp/tools.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "strings" + "time" mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" @@ -122,10 +123,16 @@ func addKBView(server *mcpsdk.Server, svc knowledgeBaseService) { // ---- doc_list ------------------------------------------------------------ type docListInput struct { - KBID string `json:"kb_id" jsonschema:"knowledge base ID"` - Page int `json:"page,omitempty" jsonschema:"1-indexed page number; defaults to 1"` - PageSize int `json:"page_size,omitempty" jsonschema:"items per page (1..1000); defaults to 20"` - Status string `json:"status,omitempty" jsonschema:"filter by parse status: pending | processing | completed | failed"` + KBID string `json:"kb_id" jsonschema:"knowledge base ID"` + Page int `json:"page,omitempty" jsonschema:"1-indexed page number; defaults to 1"` + PageSize int `json:"page_size,omitempty" jsonschema:"items per page (1..1000); defaults to 20"` + Status string `json:"status,omitempty" jsonschema:"filter by parse status: pending | processing | completed | failed"` + Keyword string `json:"keyword,omitempty" jsonschema:"server-side substring filter (case-sensitive LIKE against title / file_name); leave empty to skip"` + FileType string `json:"file_type,omitempty" jsonschema:"filter by file extension (e.g. pdf, md)"` + Source string `json:"source,omitempty" jsonschema:"filter by ingestion source (e.g. api, web)"` + TagID string `json:"tag_id,omitempty" jsonschema:"filter by tag association"` + StartTime string `json:"start_time,omitempty" jsonschema:"include docs with updated_at >= this RFC3339 timestamp (e.g. 2006-01-02T15:04:05Z)"` + EndTime string `json:"end_time,omitempty" jsonschema:"include docs with updated_at <= this RFC3339 timestamp (e.g. 2006-01-02T15:04:05Z)"` } type docListOutput struct { @@ -138,7 +145,7 @@ type docListOutput struct { func addDocList(server *mcpsdk.Server, svc knowledgeService) { mcpsdk.AddTool(server, &mcpsdk.Tool{ Name: "doc_list", - Description: "List documents in a knowledge base, with pagination and optional parse-status filter. Returns items[] with id, file_name, title, parse_status, size, updated_at - plus the page/total metadata.", + Description: "List documents in a knowledge base, with pagination and optional filters (parse-status, keyword, file_type, source, tag_id, start_time/end_time on updated_at). Returns items[] with id, file_name, title, parse_status, size, updated_at - plus the page/total metadata.", }, func(ctx context.Context, _ *mcpsdk.CallToolRequest, in docListInput) (*mcpsdk.CallToolResult, docListOutput, error) { if in.KBID == "" { return nil, docListOutput{}, fmt.Errorf("kb_id is required") @@ -154,8 +161,28 @@ func addDocList(server *mcpsdk.Server, svc knowledgeService) { if size > 1000 { return nil, docListOutput{}, fmt.Errorf("page_size must be in 1..1000") } - items, total, err := svc.ListKnowledgeWithFilter(ctx, in.KBID, page, size, - sdk.KnowledgeListFilter{ParseStatus: in.Status}) + filter := sdk.KnowledgeListFilter{ + ParseStatus: in.Status, + Keyword: in.Keyword, + FileType: in.FileType, + Source: in.Source, + TagID: in.TagID, + } + if in.StartTime != "" { + t, err := time.Parse(time.RFC3339, in.StartTime) + if err != nil { + return nil, docListOutput{}, fmt.Errorf("start_time must be RFC3339 (e.g. 2006-01-02T15:04:05Z), got %q", in.StartTime) + } + filter.StartTime = t + } + if in.EndTime != "" { + t, err := time.Parse(time.RFC3339, in.EndTime) + if err != nil { + return nil, docListOutput{}, fmt.Errorf("end_time must be RFC3339 (e.g. 2006-01-02T15:04:05Z), got %q", in.EndTime) + } + filter.EndTime = t + } + items, total, err := svc.ListKnowledgeWithFilter(ctx, in.KBID, page, size, filter) if err != nil { return nil, docListOutput{}, fmt.Errorf("list documents: %w", err) } diff --git a/cli/internal/mcp/tools_test.go b/cli/internal/mcp/tools_test.go index 99a8e622b..cd0a2626e 100644 --- a/cli/internal/mcp/tools_test.go +++ b/cli/internal/mcp/tools_test.go @@ -269,6 +269,59 @@ func TestTool_DocList_StatusFilter_Forwarded(t *testing.T) { } } +// TestTool_DocList_PassesFilterFields drives every C11 filter field at once +// and asserts they all land on filter struct (AND-combined server-side). +func TestTool_DocList_PassesFilterFields(t *testing.T) { + svc := &fakeSvc{} + c, _ := newTestServer(t, svc) + args := map[string]any{ + "kb_id": "kb_x", + "status": "completed", + "keyword": "spec", + "file_type": "pdf", + "source": "api", + "tag_id": "tag_42", + "start_time": "2026-01-01T00:00:00Z", + "end_time": "2026-12-31T23:59:59Z", + } + callTool(t, c, "doc_list", args, nil) + f := svc.calls.docListFilter + assert.Equal(t, "completed", f.ParseStatus) + assert.Equal(t, "spec", f.Keyword) + assert.Equal(t, "pdf", f.FileType) + assert.Equal(t, "api", f.Source) + assert.Equal(t, "tag_42", f.TagID) + assert.False(t, f.StartTime.IsZero(), "start_time RFC3339 must populate filter.StartTime") + assert.False(t, f.EndTime.IsZero(), "end_time RFC3339 must populate filter.EndTime") +} + +// TestTool_DocList_InvalidStartTime asserts malformed RFC3339 is rejected +// at the handler boundary (before the SDK is called). +func TestTool_DocList_InvalidStartTime(t *testing.T) { + c, _ := newTestServer(t, &fakeSvc{}) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + res, err := c.CallTool(ctx, &mcpsdk.CallToolParams{ + Name: "doc_list", + Arguments: map[string]any{"kb_id": "kb_x", "start_time": "tomorrow"}, + }) + require.NoError(t, err) + require.True(t, res.IsError, "expected IsError=true on malformed RFC3339 start_time") +} + +// TestTool_DocList_InvalidEndTime mirrors the start_time guard for end_time. +func TestTool_DocList_InvalidEndTime(t *testing.T) { + c, _ := newTestServer(t, &fakeSvc{}) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + res, err := c.CallTool(ctx, &mcpsdk.CallToolParams{ + Name: "doc_list", + Arguments: map[string]any{"kb_id": "kb_x", "end_time": "2026-05-01"}, // date-only, not RFC3339 + }) + require.NoError(t, err) + require.True(t, res.IsError, "expected IsError=true on malformed RFC3339 end_time") +} + func TestTool_DocView(t *testing.T) { svc := &fakeSvc{getDoc: &sdk.Knowledge{ID: "k1", FileName: "a.pdf"}} c, _ := newTestServer(t, svc)