diff --git a/internal/api/client.go b/internal/api/client.go index f375289..3012efb 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -130,65 +130,12 @@ func (c *Client) AnalyzeSidecars(ctx context.Context, zipPath, idempotencyKey st return &ir, nil } -// AnalyzeIncremental uploads a zip of changed files and requests an incremental -// graph update from the API. changedFiles is sent on every request (initial and -// retries) so the server always has the full context. -func (c *Client) AnalyzeIncremental(ctx context.Context, zipPath string, changedFiles []string, idempotencyKey string) (*SidecarIR, error) { - post := func() (*JobResponse, error) { - return c.postIncrementalZip(ctx, zipPath, changedFiles, idempotencyKey) - } - job, err := c.pollLoop(ctx, post) - if err != nil { - return nil, err - } - - var ir SidecarIR - if err := json.Unmarshal(job.Result, &ir); err != nil { - return nil, fmt.Errorf("decode incremental sidecar result: %w", err) - } - return &ir, nil -} - // postZip sends the repository ZIP to the analyze endpoint and returns the // raw job response (which may be pending, processing, or completed). func (c *Client) postZip(ctx context.Context, zipPath, idempotencyKey string) (*JobResponse, error) { return c.postZipTo(ctx, zipPath, idempotencyKey, analyzeEndpoint) } -// postIncrementalZip builds a multipart form with both the ZIP and the -// changedFiles JSON array, then submits it to the analyze endpoint. -func (c *Client) postIncrementalZip(ctx context.Context, zipPath string, changedFiles []string, idempotencyKey string) (*JobResponse, error) { - f, err := os.Open(zipPath) - if err != nil { - return nil, err - } - defer f.Close() - - var buf bytes.Buffer - mw := multipart.NewWriter(&buf) - fw, err := mw.CreateFormFile("file", filepath.Base(zipPath)) - if err != nil { - return nil, err - } - if _, err = io.Copy(fw, f); err != nil { - return nil, err - } - changedJSON, err := json.Marshal(changedFiles) - if err != nil { - return nil, err - } - if err := mw.WriteField("changedFiles", string(changedJSON)); err != nil { - return nil, err - } - mw.Close() - - var job JobResponse - if err := c.request(ctx, http.MethodPost, analyzeEndpoint, mw.FormDataContentType(), &buf, idempotencyKey, &job); err != nil { - return nil, err - } - return &job, nil -} - // deadCodeEndpoint is the API path for dead code analysis. const deadCodeEndpoint = "/v1/analysis/dead-code" diff --git a/internal/files/daemon.go b/internal/files/daemon.go index 1575a12..8cdfc3d 100644 --- a/internal/files/daemon.go +++ b/internal/files/daemon.go @@ -213,7 +213,7 @@ func (d *Daemon) incrementalUpdate(ctx context.Context, changedFiles []string) { } defer os.Remove(zipPath) - ir, err := d.client.AnalyzeIncremental(ctx, zipPath, changedFiles, idemKey) + ir, err := d.client.AnalyzeSidecars(ctx, zipPath, "incremental-"+idemKey[:8]) if err != nil { d.logf("Incremental API error: %v", err) return @@ -476,9 +476,9 @@ func (d *Daemon) mergeGraph(incremental *api.SidecarIR, changedFiles []string) { keptRels = append(keptRels, newRels...) d.ir.Graph.Relationships = keptRels - if len(incremental.Domains) > 0 { - d.ir.Domains = incremental.Domains - } + // Preserve domains from the last full generate. Incremental responses + // contain domains classified from only the changed files, which are + // incorrect for the repo as a whole. Domains only refresh on full generate. if len(extRemap) > 0 { d.logf("Resolved %d external references to internal nodes", len(extRemap)) diff --git a/internal/files/daemon_export_test.go b/internal/files/daemon_export_test.go new file mode 100644 index 0000000..3937a83 --- /dev/null +++ b/internal/files/daemon_export_test.go @@ -0,0 +1,21 @@ +package files + +import "github.com/supermodeltools/cli/internal/api" + +// NewTestDaemon creates a daemon preloaded with an existing SidecarIR for testing merge logic. +func NewTestDaemon(ir *api.SidecarIR) *Daemon { + return &Daemon{ + ir: ir, + logf: func(string, ...interface{}) {}, + } +} + +// MergeGraph exposes mergeGraph for testing. +func (d *Daemon) MergeGraph(incremental *api.SidecarIR, changedFiles []string) { + d.mergeGraph(incremental, changedFiles) +} + +// GetIR returns the current merged SidecarIR. +func (d *Daemon) GetIR() *api.SidecarIR { + return d.ir +} diff --git a/internal/files/daemon_test.go b/internal/files/daemon_test.go new file mode 100644 index 0000000..7706b04 --- /dev/null +++ b/internal/files/daemon_test.go @@ -0,0 +1,322 @@ +package files + +import ( + "testing" + + "github.com/supermodeltools/cli/internal/api" +) + +// ── helpers ───────────────────────────────────────────────────────────────── + +func buildIR(nodes []api.Node, rels []api.Relationship) *api.SidecarIR { + return &api.SidecarIR{ + Graph: api.SidecarGraph{ + Nodes: nodes, + Relationships: rels, + }, + } +} + +func newNode(id string, labels []string, props ...string) api.Node { + p := make(map[string]any) + for i := 0; i+1 < len(props); i += 2 { + p[props[i]] = props[i+1] + } + return api.Node{ID: id, Labels: labels, Properties: p} +} + +func newRel(id, typ, start, end string) api.Relationship { + return api.Relationship{ID: id, Type: typ, StartNode: start, EndNode: end} +} + +func nodeIDSet(ir *api.SidecarIR) map[string]bool { + m := make(map[string]bool, len(ir.Graph.Nodes)) + for _, n := range ir.Graph.Nodes { + m[n.ID] = true + } + return m +} + +func hasRelEdge(ir *api.SidecarIR, start, end string) bool { + for _, r := range ir.Graph.Relationships { + if r.StartNode == start && r.EndNode == end { + return true + } + } + return false +} + +// ── merge tests ───────────────────────────────────────────────────────────── + +func TestMergeGraph_BasicMerge(t *testing.T) { + existing := buildIR( + []api.Node{ + newNode("file-a", []string{"File"}, "filePath", "/repo/a.go"), + newNode("fn-a1", []string{"Function"}, "filePath", "/repo/a.go", "name", "Foo"), + }, + []api.Relationship{newRel("r1", "DEFINES", "file-a", "fn-a1")}, + ) + incremental := buildIR( + []api.Node{ + newNode("file-b", []string{"File"}, "filePath", "/repo/b.go"), + newNode("fn-b1", []string{"Function"}, "filePath", "/repo/b.go", "name", "Bar"), + }, + []api.Relationship{newRel("r2", "DEFINES", "file-b", "fn-b1")}, + ) + + d := NewTestDaemon(existing) + d.MergeGraph(incremental, []string{"/repo/b.go"}) + + result := d.GetIR() + ids := nodeIDSet(result) + for _, want := range []string{"file-a", "fn-a1", "file-b", "fn-b1"} { + if !ids[want] { + t.Errorf("expected node %q after basic merge", want) + } + } + if len(result.Graph.Nodes) != 4 { + t.Errorf("expected 4 nodes, got %d", len(result.Graph.Nodes)) + } +} + +func TestMergeGraph_FileReplacement(t *testing.T) { + existing := buildIR( + []api.Node{ + newNode("file-a-old", []string{"File"}, "filePath", "/repo/a.go"), + newNode("fn-old", []string{"Function"}, "filePath", "/repo/a.go", "name", "Foo"), + newNode("file-b", []string{"File"}, "filePath", "/repo/b.go"), + }, + []api.Relationship{newRel("r1", "DEFINES", "file-a-old", "fn-old")}, + ) + incremental := buildIR( + []api.Node{ + newNode("file-a-new", []string{"File"}, "filePath", "/repo/a.go"), + newNode("fn-new", []string{"Function"}, "filePath", "/repo/a.go", "name", "Foo"), + }, + []api.Relationship{newRel("r2", "DEFINES", "file-a-new", "fn-new")}, + ) + + d := NewTestDaemon(existing) + d.MergeGraph(incremental, []string{"/repo/a.go"}) + + result := d.GetIR() + ids := nodeIDSet(result) + + if ids["file-a-old"] { + t.Error("old File node for a.go should have been removed") + } + if ids["fn-old"] { + t.Error("old Function node for a.go should have been removed") + } + if !ids["file-a-new"] { + t.Error("new File node for a.go should be present") + } + if !ids["fn-new"] { + t.Error("new Function node for a.go should be present") + } + if !ids["file-b"] { + t.Error("unrelated file-b node should still be present") + } +} + +func TestMergeGraph_UUIDRemapping(t *testing.T) { + existing := buildIR( + []api.Node{ + newNode("file-a-old", []string{"File"}, "filePath", "/repo/a.go"), + newNode("fn-foo-old", []string{"Function"}, "filePath", "/repo/a.go", "name", "Foo"), + newNode("file-b", []string{"File"}, "filePath", "/repo/b.go"), + newNode("fn-bar", []string{"Function"}, "filePath", "/repo/b.go", "name", "Bar"), + }, + []api.Relationship{ + newRel("r1", "DEFINES", "file-a-old", "fn-foo-old"), + newRel("r2", "DEFINES", "file-b", "fn-bar"), + newRel("r3", "CALLS", "fn-bar", "fn-foo-old"), + }, + ) + incremental := buildIR( + []api.Node{ + newNode("file-a-new", []string{"File"}, "filePath", "/repo/a.go"), + newNode("fn-foo-new", []string{"Function"}, "filePath", "/repo/a.go", "name", "Foo"), + }, + []api.Relationship{newRel("r4", "DEFINES", "file-a-new", "fn-foo-new")}, + ) + + d := NewTestDaemon(existing) + d.MergeGraph(incremental, []string{"/repo/a.go"}) + + result := d.GetIR() + if !hasRelEdge(result, "fn-bar", "fn-foo-new") { + t.Error("CALLS relationship should be remapped: fn-bar → fn-foo-new") + } + if hasRelEdge(result, "fn-bar", "fn-foo-old") { + t.Error("CALLS relationship should not still reference stale fn-foo-old") + } +} + +func TestMergeGraph_ExternalDependencyResolution(t *testing.T) { + existing := buildIR( + []api.Node{ + newNode("file-button", []string{"File"}, "filePath", "/repo/web/src/components/ui/button.tsx"), + newNode("file-main", []string{"File"}, "filePath", "/repo/main.go"), + }, + nil, + ) + incremental := buildIR( + []api.Node{ + newNode("file-main-new", []string{"File"}, "filePath", "/repo/main.go"), + newNode("ext-button", []string{"LocalDependency"}, "importPath", "@/components/ui/button"), + }, + []api.Relationship{newRel("r1", "IMPORTS", "file-main-new", "ext-button")}, + ) + + d := NewTestDaemon(existing) + d.MergeGraph(incremental, []string{"/repo/main.go"}) + + result := d.GetIR() + ids := nodeIDSet(result) + + if ids["ext-button"] { + t.Error("resolved LocalDependency placeholder should not appear in merged graph") + } + if !ids["file-button"] { + t.Error("real file-button node must remain in merged graph") + } + if !hasRelEdge(result, "file-main-new", "file-button") { + t.Error("IMPORTS relationship should be remapped to the real file-button node") + } +} + +func TestMergeGraph_EmptyIncremental(t *testing.T) { + existing := buildIR( + []api.Node{ + newNode("file-a", []string{"File"}, "filePath", "/repo/a.go"), + newNode("fn-a1", []string{"Function"}, "filePath", "/repo/a.go", "name", "Alpha"), + }, + []api.Relationship{newRel("r1", "DEFINES", "file-a", "fn-a1")}, + ) + + d := NewTestDaemon(existing) + d.MergeGraph(buildIR(nil, nil), []string{"/repo/a.go"}) + + if result := d.GetIR(); result == nil { + t.Fatal("GetIR() returned nil after empty incremental merge") + } +} + +func TestMergeGraph_IncrementalNoMatchingExisting(t *testing.T) { + existing := buildIR( + []api.Node{newNode("file-a", []string{"File"}, "filePath", "/repo/a.go")}, + nil, + ) + incremental := buildIR( + []api.Node{ + newNode("file-c", []string{"File"}, "filePath", "/repo/c.go"), + newNode("fn-c1", []string{"Function"}, "filePath", "/repo/c.go", "name", "NewFunc"), + }, + []api.Relationship{newRel("r1", "DEFINES", "file-c", "fn-c1")}, + ) + + d := NewTestDaemon(existing) + d.MergeGraph(incremental, []string{"/repo/c.go"}) + + result := d.GetIR() + ids := nodeIDSet(result) + + if !ids["file-c"] { + t.Error("file-c should be added to the graph") + } + if !ids["fn-c1"] { + t.Error("fn-c1 should be added to the graph") + } + if !ids["file-a"] { + t.Error("file-a should remain untouched") + } + if !hasRelEdge(result, "file-c", "fn-c1") { + t.Error("DEFINES relationship for new file should be present") + } +} + +func TestMergeGraph_NilExisting(t *testing.T) { + d := NewTestDaemon(nil) + incremental := buildIR( + []api.Node{newNode("file-a", []string{"File"}, "filePath", "/repo/a.go")}, + nil, + ) + + d.MergeGraph(incremental, []string{"/repo/a.go"}) + + result := d.GetIR() + if result == nil { + t.Fatal("expected non-nil SidecarIR after merging into nil daemon") + } + if !nodeIDSet(result)["file-a"] { + t.Error("file-a should be present when merging into nil existing graph") + } +} + +// ── domain preservation tests ─────────────────────────────────────────────── + +func TestMergeGraph_DomainsPreservedOnIncremental(t *testing.T) { + existing := &api.SidecarIR{ + Graph: api.SidecarGraph{ + Nodes: []api.Node{newNode("file-a", []string{"File"}, "filePath", "/repo/a.go")}, + }, + Domains: []api.SidecarDomain{ + {Name: "CommandCLI"}, + {Name: "ApiClient"}, + {Name: "SharedKernel"}, + }, + } + incremental := &api.SidecarIR{ + Graph: api.SidecarGraph{ + Nodes: []api.Node{newNode("file-b", []string{"File"}, "filePath", "/repo/b.go")}, + }, + Domains: []api.SidecarDomain{ + {Name: "LocalCache"}, // garbage domain from 1-file classification + }, + } + + d := NewTestDaemon(existing) + d.MergeGraph(incremental, []string{"/repo/b.go"}) + + result := d.GetIR() + if len(result.Domains) != 3 { + t.Fatalf("expected 3 domains preserved, got %d", len(result.Domains)) + } + names := make(map[string]bool) + for _, dom := range result.Domains { + names[dom.Name] = true + } + for _, want := range []string{"CommandCLI", "ApiClient", "SharedKernel"} { + if !names[want] { + t.Errorf("domain %q should be preserved, got domains: %v", want, result.Domains) + } + } +} + +func TestMergeGraph_DomainsPreservedEvenWhenIncrementalHasMore(t *testing.T) { + existing := &api.SidecarIR{ + Graph: api.SidecarGraph{ + Nodes: []api.Node{newNode("file-a", []string{"File"}, "filePath", "/repo/a.go")}, + }, + Domains: []api.SidecarDomain{{Name: "Original"}}, + } + incremental := &api.SidecarIR{ + Graph: api.SidecarGraph{ + Nodes: []api.Node{newNode("file-b", []string{"File"}, "filePath", "/repo/b.go")}, + }, + Domains: []api.SidecarDomain{ + {Name: "New1"}, + {Name: "New2"}, + {Name: "New3"}, + }, + } + + d := NewTestDaemon(existing) + d.MergeGraph(incremental, []string{"/repo/b.go"}) + + result := d.GetIR() + if len(result.Domains) != 1 || result.Domains[0].Name != "Original" { + t.Errorf("expected original domain preserved, got %v", result.Domains) + } +}