feat: workspace graph redesign (CG0004) — inventory, scopes, FTS5 search, related - #24
Conversation
…ayer to scanner pipeline Add explicit repo-boundary discovery and live-worktree inventory as a shared foundation for later scope, extraction, inference, search, and refresh passes. Boundaries are detected via real .git markers (directory or file) and classified against the campaign root's .gitmodules, not inferred from path names. The inventory walker records per-file git state (tracked/untracked/ignored), path depth, extension, and owning repo root for every eligible entry, stopping descent at nested boundaries so git classification stays per-repo. Thread BuildInventory into scanner.Scan so the shared inventory is available to existing artifact scanners and all later sequences. Keep artifact discovery backward-compatible; ignored content is excluded by default while authored untracked files stay visible. Add a containerized fixture harness under tests/integration so repo-state mutation scenarios (nested repos, submodules, tracked vs untracked vs ignored) run in isolation per festival rules, and add regression tests asserting build still succeeds across those scenarios.
…rtifact-to-scope bridges Introduce NodeFolder nodes and scope metadata (scope_kind, repo_root, path_depth, is_submodule, boundary_rel) so folders, repo roots, and submodules become first-class graph entities. Campaign root always emits a folder:. anchor; repo and submodule boundaries emit repo_root/submodule_root nodes; well-known campaign buckets (projects, festivals, workflow, .campaign/intents, etc.) emit campaign_bucket nodes; user-authored directories with authored content emit user_scope nodes. Connect folders into a scope hierarchy with structural contains edges and bridge existing artifact nodes (project, festival, intent, design_doc, explore_doc, chain) into their nearest owning folder scope so artifact discovery and workspace hierarchy share a single spine without rewriting artifact IDs. Add unit tests for campaign-root, known-bucket, artifact-scope, user-scope, repo/submodule, artifact bridge, and structural/inferred edge distinction. Add containerized integration coverage for customized and obsidian-style layouts to prove build stability across non-strict directory shapes.
…es, tags, and attachments Add NodeNote, NodeCanvas, NodeTag, NodeAttachment types so workspace content becomes first-class graph entities with path-stable IDs (note:<rel>, canvas:<rel>, attachment:<rel>, tag:<name>). Scan notes from the inventory layer instead of re-walking the filesystem. Parse YAML frontmatter into stable metadata keys (title, aliases, tags, type, status) and fall back to filename-derived titles when frontmatter is missing or malformed. Skip artifact-owned markdown paths (intents, festival task files) so artifact IDs remain authoritative. Add explicit link extraction: markdown [text](target), wiki [[target]] (with alias and section-anchor stripping), image embeds, inline #tags (with URL-encoded target resolution), and Obsidian-style canvas JSON files. Distinguish subtypes (markdown_link, wiki_link, embed, attachment, inline_tag, canvas_link) on existing edge types instead of inventing a parallel taxonomy. External URLs and missing targets are ignored; malformed canvases fail gracefully without aborting the scan. Cover the new behavior with unit tests (path-stable IDs, frontmatter fields, filename fallback, structural scope bridging, artifact deduping, malformed frontmatter, markdown and wiki link resolution, URL-encoded targets, inline tags vs headings, canvas parsing) and containerized integration tests for realistic Obsidian-style vault and malformed-frontmatter layouts.
…nce with evidence aggregation Introduce a bounded candidate-generation pass that enumerates potential inferred pairs from posting lists (same folder, shared ancestor, shared tag, shared frontmatter key/value, shared repo root) instead of global O(n*n) all-pairs comparison. CandidateBudget caps both per-group membership and total pair count; exceeding either cap flips a Truncated flag on the result. Add an aggregation pass that merges every CandidatePair for the same node pair, deduplicates per-signal evidence, computes a saturated confidence score, and emits one inferred edge with evidence_reasons JSON stored in Edge.Note and the dominant reason kind echoed into Edge.Subtype. Aggregation-time signals (shared_tokens, artifact_owned) extend candidate evidence for already-shortlisted pairs without producing new posting lists. Select EdgeSimilarTo for pairs dominated by lexical signals (shared_tokens, alias_match) and EdgeRelatesTo otherwise, matching the design package. Weak pairs below minInferredConfidence are dropped so campaign-root-only affinity does not produce noise. Cover the new behavior with unit tests for posting-list grouping, budget enforcement, no-all-pairs behavior, weak-signal exclusion, artifact-owned signal, shared-tokens SimilarTo selection, and integration tests that prove multi-file campaigns build end-to-end without spurious cross-folder edges.
…S5-backed query command Add the four contract tables (graph_meta, indexed_files, search_docs, search_docs_fts) to createTablesSQL. graph_meta stores build and refresh bookkeeping plus schema/plugin versions; indexed_files is reserved for refresh flow; search_docs carries title, scope, body, aliases, and tags; search_docs_fts mirrors the content columns with an external-content FTS5 virtual table. Introduce the internal/search package with typed request/response shapes (graph-query/v1alpha1, graph-related/v1alpha1, graph-status/v1alpha1, graph-refresh/v1alpha1), an Indexer that upserts and deletes search docs while keeping the FTS mirror consistent using the indexed values as 'delete' args, and FTSAvailable to surface search_available=true/false per contract. Add graph.SaveFullBuild: one transaction rebuilds nodes, edges, search_docs, and graph_meta so full builds leave the DB coherent under concurrent readers. The build command now uses SaveFullBuild, emits BuildMeta with GraphSchemaVersion graphdb/v2alpha1, and populates search_docs from note nodes (title, scope, body, aliases, tags, tracked state). Implement FTS5-backed Querier (internal/search/query.go) with tokenized MATCH expressions, optional scope/path-prefix/tracked/type filters, snippet() output, and a reasons list (fts_match, exact_path_token, title_match, same_scope). Replace the old substring node scan in cmd/camp-graph/query.go with search.Querier and a --json emitter that produces the graph-query/v1alpha1 envelope. Move the query command into its own cmd file, leaving root.go for shared config and registration. Cover with unit tests for meta round-trip, search_docs table readiness, indexer upsert/overwrite/delete (with FTS mirror verification), FTS availability, query lexical/scope/path-prefix/ tracked/type filters, and containerized integration tests asserting the graph-query/v1alpha1 envelope and content-backed matches for buried notes.
…cope-first browse, render mode+scope filters Add search.Resolve that looks up node IDs in the order specified by IMPLEMENTATION_CONTRACTS: exact node ID -> exact relative path -> top lexical hit. Wire context and render --node to the resolver so users can pass rel paths or natural-language fragments instead of only exact artifact IDs. Legacy exact-name fallback preserved for project:<name> style CLI habits. TUI browse now opens on scope anchors (campaign root, repo/submodule roots, campaign buckets) instead of every node. Tab cycles relation mode hybrid -> structural -> explicit -> semantic, which filters the micrograph neighbor list by Edge.Source. Added 'a' to widen to all nodes and 's' to return to scope anchors; header shows the active relation mode. enter on a scope opens the scope's neighborhood; search (/) still filters within the current view. Render gains --scope, --mode, --tracked, --untracked. --node wins over --scope per contract. --scope slices the graph to nodes whose name/path live inside the scope subtree (plus edges that remain internal). --mode filters edges by source (structural/explicit/ semantic/hybrid). --tracked and --untracked keep nodes whose git_state matches while preserving scope scaffolding that has no git_state recorded. Cover with unit tests for slice-by-scope, relation-mode filter, tracked-state filter, plus Resolve exact-id/exact-rel-path/top-hit/ no-match, and containerized integration tests for render --scope end-to-end behavior.
…with graph_meta-backed bookkeeping Add internal/runtime with IndexState (indexed_files CRUD + SHA-256/mtime fingerprint helpers), Status (reads graph_meta plus node/edge/indexed counts; recomputes search_available from a live FTS5 probe), UpdateRefreshMeta, and the Refresh flow. Refresh rebuilds inventory, diffs against indexed_files by content_hash with mtime fallback, falls back to mode=rebuild when the DB is empty or schema_version is incompatible, and always reports accurate reindexed/deleted counts plus duration in the graph-refresh/v1alpha1 envelope. Add camp-graph refresh and camp-graph status commands emitting the contract JSON envelopes (graph-refresh/v1alpha1, graph-status/v1alpha1) with --db overrides and --json output. Status surfaces search_available live so an unhealthy FTS never reads as true. Cover with unit tests for indexed_files upsert/load/delete round-trip, status round-trip from graph_meta, UpdateRefreshMeta, refresh fresh-DB forces rebuild, content-mutation promotes second run to refresh mode, deletion is counted, and parity between rebuild and subsequent refresh (node/edge counts match).
…e-first workitem enrichment Add internal/search.Related plus cmd/camp-graph/related.go. The command reads --path as campaign-relative (callers pass primary_doc first, relative_path second) and returns the graph-related/v1alpha1 envelope. Ranking order follows IMPLEMENTATION_CONTRACTS: scope neighbors first (same_scope reason), then explicit link targets and sources (explicit_edge), then lexical top-up using the filename stem (lexical_match). Duplicate node IDs collapse with the first-seen reason winning so output stays explainable. --mode narrows to structural/explicit/semantic/hybrid; hybrid is the default. --limit caps at 10 by default. The command is additive: when a path is unknown or graph data is stale, Related returns an empty items slice and the envelope still surfaces the stale flag from runtime.Status so camp workitem integrations can ignore enrichment cleanly without crashing. Cover with unit tests for same-scope dominance, explicit-edge resolution, --path required, unknown-path empty result, and containerized integration tests for enrichment end-to-end, unknown paths, and the graph-status/v1alpha1 envelope shape after build.
…ed code-aware extraction Add NodeRepo with the contract ID 'repo:<relative-repo-root>' as an explicit slice anchor for nested git boundaries (standalone nested repos and submodules). The campaign root keeps folder:. as its anchor; nested boundaries now own both a folder:<rel> scope node and a repo:<rel> slice node bridged with a structural contains edge so navigation works from either side. Render --scope and query-time resolution accept repo:<rel> or folder:<rel> interchangeably so users can target a nested repo slice with 'render --scope projects/camp-graph' without flattening the campaign graph. Add code-aware slice extraction that emits NodeFile entries (and NodePackage groupings for Go) exclusively for inventory entries whose RepoRoot is a nested boundary. Campaign-root code is skipped so the graph does not flood with tooling files from every project. Go packages are detected without go/ast by scanning the first 32 lines for 'package foo', keeping the extractor dependency-free and bounded. Each file node records MetaRepoRoot, MetaPathDepth, and language so consumers can rejoin code slices with their anchor. Cover with unit tests for campaign-root code exclusion, nested-repo file/package emission, repo_root metadata, and non-code file skipping. Integration test proves build/query on a nested repo slice works end-to-end and cross-boundary leakage is prevented.
…ules, e2e tests, vault smoke, docs Add explicit CompatibilityVerdict (fresh/matching/incompatible) and KnownCompatibleSchemas registry so refresh's rebuild fallback is rooted in a typed compatibility check instead of ad-hoc string comparison. The current release anchors at graphdb/v2alpha1; older schemas or empty graph_meta force ModeRebuild. Add containerized end-to-end integration test that exercises the full release contract on one fixture: build, query --json with content-backed matches, refresh --json mode reporting, status --json stale/search_available, and related --json with scope locality. Run the fixed ObsidianVault smoke commands (cd to the live vault, build, query JobSearch scoped to Work/JobSearch, related against Action Plan.md, query and related for ShinySwap/DesignDocs, status) and record results under docs/release-smoke-results.md. All five contract pass conditions hold on a 13,143-node / 4,332-search-doc vault. Wall-clock: build 4.28s, warm query 21ms, related 30ms. Update README with the new command surface (query/related/refresh/ status, scope/mode/tracked/untracked flags, schema_version tags for each JSON envelope, graphdb/v2alpha1 schema identifier).
…efore phase-gate approval 1. Populate indexed_files atomically on full build. Add graph.SaveFullBuildWithIndex and graph.IndexedFileRecord so build and refresh persist fingerprint rows in the same transaction as nodes, edges, search_docs, and graph_meta. status --json now reports a real indexed_files count immediately after build (18894 on ObsidianVault). 2. Real incremental refresh fast path. When the inventory diff reports zero added/changed/deleted files, Refresh updates only last_refresh_at/mode via UpdateRefreshMeta and returns live counts from the DB. The heavy SaveFullBuildWithIndex path only runs when work actually needs doing. Added unit test TestRefresh_NoChangesSkipsFullRebuild. 3. Migrate all new cmd/camp-graph fmt.Errorf call sites to the project error package: query.go, refresh.go, status.go, related.go, render_filters.go, and the new build additions in root.go now use graphErrors.Wrap/Wrapf/New. Pre-existing fmt.Errorf in legacy root.go commands is untouched to avoid scope creep. 4. writeSearchDocsTx uses RETURNING rowid instead of INSERT plus a separate SELECT, halving the per-document round-trips on full rebuilds.
obey-agent
left a comment
obey-agent
left a comment
There was a problem hiding this comment.
Verdict: Request Changes
Overview
This is a substantial, well-designed piece of work. The inventory-first architecture, bounded inference with posting-list caps, FTS5 external-content pattern with proper delete-before-insert, scope graph hierarchy, and JSON envelope contracts all show strong engineering judgment. The integration test harness using containerized fixtures (rather than t.TempDir) is exactly right. The PR is very close to mergeable but has five concrete defects that must be fixed before merge.
Key Findings (see inline comments)
-
Error handling inconsistency (internal/graph/store.go, persist.go, internal/scanner/scanner.go): 50+ uses of fmt.Errorf in the persistence and scanner foundations while every other package in this PR correctly uses graphErrors.Wrap/Wrapf. These are the worst places for inconsistency.
-
GenerateCandidates has no context.Context (internal/scanner/inference.go): The CPU-intensive posting-list enumeration loop cannot be cancelled. aggregateInferredEdges correctly checks ctx.Err() at every pair — GenerateCandidates must receive the same treatment. Mandatory per project CLAUDE.md.
-
Refresh no-op fast path runs a full scan first (internal/runtime/refresh.go): sc.Scan(ctx) — which SHA-256 hashes 18,894 files per the smoke test — runs unconditionally before the inventory diff decides whether the fast path applies.
-
FTSAvailable uses ExecContext for a SELECT (internal/search/index.go): Semantically wrong API; should be QueryRowContext. Works by accident on the current driver.
-
os.ReadFile error discarded in buildSearchDocs (cmd/camp-graph/root.go): Documents silently indexed with empty bodies when file reads fail — a correctness failure with no diagnostic signal.
Bonus (not blocking but worth tracking)
- scanNodes in store.go silently ignores json.Unmarshal errors on metadata deserialization.
- cmd/camp-graph/root.go is 547 lines (limit is 500) with 17 package-level globals. The new commands (query, refresh, status, related) are correctly factored — buildCmd and render helpers should follow.
What's Done Well
- FTS5 external-content pattern with correct prior-values-on-delete is implemented exactly right.
- CandidateBudget caps and the Truncated flag are excellent defensive design.
- Containerized fixture harness for repo-state tests aligns perfectly with project policy.
- JSON envelope schema versioning and CompatibilityVerdict fallback-to-rebuild are production-grade.
- Context propagation in aggregateInferredEdges, Refresh, and all SQL helpers is thorough — the GenerateCandidates gap stands out because everywhere else is correct.
- inferenceWeights as named package-level vars rather than buried magic numbers is clean.
Staff Standard
Not quite yet. Items 1 and 2 are explicit project standards, not style preferences. Fix those five issues and this is strong work.
| // Missing or unreadable notes should not halt the whole scan; | ||
| // skip silently because the note node still exists. | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Refresh holds ctx-checked loops but not the total-work cap. Candidate generation produces up to 5000 pairs, aggregation then runs token-overlap and artifact-owned signals on every pair — that's OK — but the scanner also reads every note body twice (once in scanNotes for frontmatter, once in extractExplicitLinks for link/tag parsing, and once more in buildSearchDocs in cmd/root.go). On a 13k-note vault that's 39k file reads per build. Consider reading each note once in scanNotes and stashing body+frontmatter on the node (in memory, not metadata) so the link/body passes can reuse it. This is a real scaling concern for the "public launch with <5 minute first-run" goal — the measured 7.45s build will balloon roughly linearly with vault size.
There was a problem hiding this comment.
Not fixed in this commit — tracked as a follow-up. The three reads (frontmatter in scanNotes, links in extractLinksFromNote, search body in buildSearchDocs) are real; a per-scan content cache would collapse them to one read per note. Leaving for a dedicated perf pass so this review cycle stays focused on correctness.
obey-agent
left a comment
obey-agent
left a comment
There was a problem hiding this comment.
Verdict: Request Changes
Overview
This is an ambitious, well-structured redesign that ships a meaningful amount of new surface — scope graph, FTS5 search, inference aggregation, refresh/status, related enrichment, and containerized tests. The in-memory model, JSON envelopes, and test coverage are real improvements. However, several contract and correctness issues make this risky to merge as-is: a flag the CLI advertises doesn't actually do anything, declared FK constraints aren't enforced, schema has no migration story, the "incremental refresh" always reads every file, and the project's own error-handling convention is applied inconsistently across core files.
Key Findings
query --modeis a no-op inQuerier.Search— the field is plumbed through the CLI intoQueryOptions.Modebut never consulted when building the SQL. OnlyRelatedhonors mode. (see inline oninternal/search/query.go)search_docs.node_id REFERENCES nodes(id) ON DELETE CASCADEis declared butPRAGMA foreign_keys=ONis never issued, so the cascade is a silent no-op. (see inline oninternal/graph/store.go)- Schema evolution is unhandled — only
CREATE TABLE IF NOT EXISTS, no migrations, future column changes will silently not apply on existing DBs. (see inline oninternal/graph/store.go) diffInventoryhashes every file every refresh before checking mtime, so "incremental refresh" is effectively a full content scan. On an 18k-file vault that dominates runtime. (see inline oninternal/runtime/refresh.go)- Refresh heavy path reports
reindexed_files: Nwhen it actually rewrote every row insearch_docs/indexed_files. The contract field doesn't describe what the code did. (see inline oninternal/runtime/refresh.go) resolveWikiTargetIDscans all graph nodes per missing wiki-link target — O(N*M) at vault scale. Same cost pattern will hit as vaults grow. (see inline oninternal/scanner/links.go)- Inconsistent error-package usage —
graphErrorsis the project standard (used correctly insearch/,runtime/, andScan()itself), butinternal/graph/persist.go,internal/graph/store.go, and half ofinternal/scanner/scanner.gostill usefmt.Errorf. Pick one and enforce. (see inline oninternal/scanner/scanner.go) - Silent error drops on hot paths:
metaJSON, _ := json.Marshal(...),scanNodesdrops unmarshal errors,liveCountsdiscards three DB errors,buildSearchDocsdiscards read errors. (see inlines) probeGitMarkeraccepts any.gitentry — broken symlinks, zero-byte files, stale worktree pointers all register as boundaries and can break the scan when git fails inside them. (see inline oninternal/scanner/repo_inventory.go)- Root path not symlink-resolved —
filepath.AbswithoutEvalSymlinksmeans macOS/varvs/private/varinconsistencies propagate (this repo's own MEMORY.md calls this out for tests). (see inline oninternal/scanner/repo_inventory.go) - File-read amplification — notes are read three times per build (frontmatter, links, search body). Real driver of the 7.45s build cost. (see inline on
internal/scanner/links.go)
What's Done Well
- Inventory layer, boundary discovery, and git-state classification are cleanly separated and testable.
- FTS5 indexer correctly uses the external-content delete pattern — that's the subtle footgun that trips most FTS5 implementations, and it's handled right.
- Candidate generation with explicit budgets (
MaxMembersPerGroup=128,MaxPairs=5000) is a mature approach to keeping inference bounded on large vaults. - JSON contract envelopes with
v1alpha1schema versions give room to evolve without breaking consumers. - Inference evidence aggregation (saturated weight sum, stable reason ordering, round3 serialization) is thoughtful and will be easy to tune.
- Containerized integration test harness is the right call and aligns with the "filesystem safety" rule in CLAUDE.md.
Staff Standard
No, not yet. The ambition and structure are staff-level; the execution has enough gaps I wouldn't submit this as mine. The three blockers I'd want cleared before merge: (1) either implement query --mode or remove it, (2) enable PRAGMA foreign_keys=ON or drop the CASCADE declaration, (3) fix diffInventory to stat-first-hash-later so incremental refresh is actually incremental. Error-package consistency and the silent-error drops are fast cleanups that should ride along. The metadata/scaling issues (file-read amplification, O(N*M) wiki resolver) are pre-existing-scale concerns I'd want tracked even if not fixed in this PR.
… ctx propagation, refresh fast-path, FTS probe, read errors Addresses all five blocking findings from obey-agent's review: 1. internal/graph/persist.go & store.go: replace fmt.Errorf with graphErrors.Wrap/Wrapf throughout the persistence foundation and stop silently ignoring json.Unmarshal errors in scanNodes. 2. internal/scanner/scanner.go: replace fmt.Errorf with graphErrors. 3. internal/scanner/inference.go: GenerateCandidates now takes context.Context and checks ctx.Err() inside every emission loop so cancellation propagates through the CPU-bound candidate pass. 4. internal/runtime/refresh.go: Refresh builds inventory only, diffs against indexed_files, and skips the full scan on the no-op path. New Scanner.BuildInventoryOnly caches the inventory so the heavy path reuses it without re-walking the filesystem. 5. internal/search/index.go: FTSAvailable uses QueryRowContext for its SELECT probe and tolerates sql.ErrNoRows. 6. cmd/camp-graph/root.go: buildSearchDocs logs os.ReadFile failures to stderr and skips the doc instead of indexing an empty body.
obey-agent
left a comment
obey-agent
left a comment
There was a problem hiding this comment.
Verdict: Request Changes
Overview
The latest fixup commit clears a substantial amount of the previous review debt: error wrapping is consistent in the core paths, candidate generation now honors cancellation, the refresh no-op path is materially cheaper, foreign keys are enabled, and the obvious silent-error drops called out earlier are addressed. However, this head still has two blocking gaps: the query surface is broader than what the index actually contains, and the clean-checkout integration suite still fails.
Key Findings
camp-graph querystill indexes notes only.buildSearchDocsemitssearch_docsrows exclusively forNodeNote, but the command surface and docs now advertise artifact queries likecamp graph query "auth" --type project. On the current head, a minimal fixture withprojects/alphareproducesCAMP_ROOT=<root> camp-graph query alpha --type project --jsonreturningresults: []. This also matches the clean-checkout failures inTestQueryIntents,TestIntentLifecycleStatuses,TestInventoryRegression_NestedRepoBuild, andTestScopeGraph_CustomizedLayoutBuild.go test -tags=integration ./...still fails on a clean checkout. A large subset of the new integration cases build fixtures throughSetupRepoFixtureswithout creating a.campaignmarker, whileRunGraphInDiralways exportsCAMP_ROOT=<dir>and root detection only accepts that override when<dir>/.campaignexists. Those tests currently die before build withdetermining campaign root: not inside a campaign directory.
What's Done Well
- The latest pass genuinely fixed many of the earlier correctness concerns rather than papering over them.
- The store/runtime cleanup made the code easier to trust: foreign-key enforcement, wrapped DB errors, and the refresh fast-path restructuring are all meaningful improvements.
- Splitting the oversized command file into focused subcommands was the right direction and makes the CLI surface easier to review and maintain.
Validation
go test ./...go vet ./...go vet -tags=integration ./...go test -tags=integration ./...← fails on the current PR head with the issues above
Staff Standard
Not yet. I would be comfortable merging this once the query/index contract is aligned with the advertised artifact search surface and the integration suite passes cleanly from a fresh checkout.
Review response — 700e0e5Threaded replies posted on each inline comment with fix locations. Summary: Fixed in 700e0e5
Not fixed in 700e0e5 — deferred
Verification
|
…exing + integration harness 1. cmd/camp-graph/build.go: extend buildSearchDocs to emit DocumentRecord rows for project, festival, chain, phase, sequence, task, intent, design_doc, explore_doc, file, and package nodes in addition to notes. Artifact nodes synthesise a body from Name + Status + Metadata; file-backed artifacts (intents, tasks, design_docs, explore_docs whose Path points at a concrete file) also fold the on-disk content in. buildSearchDocs now takes campRoot so artifact nodes get stable campaign-relative rel paths; refresh.go wraps it in a closure that captures cfg.CampRoot. `camp-graph query --type project|intent|...` now returns matches instead of silently empty results. 2. internal/search/related.go: reorder the hybrid pipeline so explicit edges are consulted before scope neighbors. An author-authored link is a stronger signal than shared folder locality; reversing the order also lets an explicitly-linked target surface with the explicit_edge reason rather than being swallowed by the same_scope bucket via dedup. 3. tests/integration/helpers.go WriteFile: quote the destination path so filenames containing spaces (e.g. "Action Plan.md") round-trip through `sh -c`. Previous unquoted redirect split paths on whitespace, truncating every fixture file whose name had a space. This was the real root cause of most query/related integration failures flagged in the latest review. 4. tests/integration/inventory_fixtures.go: seed a minimal .campaign/campaign.yaml marker at every outermost root fixture so camputil.FindCampaignRoot accepts CAMP_ROOT=<fixture path>. Nested fixtures (paths contained by another spec, or explicit SubmodulePath/ParentPath) are left marker-free. Opt-out via a new RepoSpec.SkipCampaignMarker flag for tests that deliberately want a non-campaign CAMP_ROOT. Verification: go test ./... and go test -tags=integration ./... both pass on a clean tree; go vet ./... and go vet -tags=integration ./... clean.
Follow-up response — 492549cBoth blockers from the latest review are addressed. Artifact indexing (
Integration harness (
While tracing the failures I also found and fixed a second harness bug that was masquerading as the query gap: Related ranking ( Reordered the hybrid pipeline so explicit edges consult first, then scope neighbors, then lexical. An author-authored link is a stronger signal than shared folder locality; the prior order swallowed explicit edges via dedup when the target was also in-scope, which is what the Verification
No remaining deferred items from the prior response; the file-read amplification concern from review round 1 is the one remaining item, still tracked for a dedicated perf pass rather than bundled here. |
obey-agent
left a comment
obey-agent
left a comment
There was a problem hiding this comment.
Verdict: Approve
The latest head closes the two blocking issues from the prior review. Artifact indexing now matches the advertised query --type ... surface, the shared integration harness now produces valid campaign roots, and the related-ranking fix restores explicit-edge priority in the hybrid path.
What I verified
go test ./...-> passedgo test -tags=integration ./...-> passed on a clean checkoutgo vet ./...-> cleango vet -tags=integration ./...-> clean
Key Findings
- No new blocking findings on
492549c. - The previously blocked query/index contract and integration-root issues are fixed in the current head.
What's Done Well
buildSearchDocsnow aligns the CLI contract with the actual index surface instead of leaving artifact search half-wired.- The integration harness fix is the right abstraction boundary: the shared fixture helper now produces campaign-valid roots by default rather than pushing that burden onto every test.
- The path-quoting fix in the container helper closes a real test-fidelity bug that would have kept hiding content-extraction regressions for filenames with spaces.
Staff Standard
Yes. I would merge this head as-is.
obey-agent
left a comment
obey-agent
left a comment
There was a problem hiding this comment.
Verdict: Approve
The latest head closes the two blocking issues from the prior review. Artifact indexing now matches the advertised query --type ... surface, the shared integration harness now produces valid campaign roots, and the related-ranking fix restores explicit-edge priority in the hybrid path.
What I verified
go test ./...-> passedgo test -tags=integration ./...-> passed on a clean checkoutgo vet ./...-> cleango vet -tags=integration ./...-> clean
Key Findings
- No new blocking findings on
492549c. - The previously blocked query/index contract and integration-root issues are fixed in the current head.
What's Done Well
buildSearchDocsnow aligns the CLI contract with the actual index surface instead of leaving artifact search half-wired.- The integration harness fix is the right abstraction boundary: the shared fixture helper now produces campaign-valid roots by default rather than pushing that burden onto every test.
- The path-quoting fix in the container helper closes a real test-fidelity bug that would have kept hiding content-extraction regressions for filenames with spaces.
Staff Standard
Yes. I would merge this head as-is.
obey-agent
left a comment
obey-agent
left a comment
There was a problem hiding this comment.
Verdict: Approve
The latest head closes the two blocking issues from the prior review. Artifact indexing now matches the advertised query surface, the shared integration harness now produces valid campaign roots, and the related-ranking fix restores explicit-edge priority in the hybrid path.
What I verified
- go test ./... -> passed
- go test -tags=integration ./... -> passed on a clean checkout
- go vet ./... -> clean
- go vet -tags=integration ./... -> clean
Staff Standard
Yes. I would merge this head as-is.
2 participants
Summary
Ships the full
camp-graphworkspace-graph redesign per festivalcamp-graph-workspace-graph-CG0004. Turns the plugin into ahierarchy-first, evidence-weighted workspace graph with real lexical
retrieval, bounded inference, incremental refresh, and optional
camp workitemenrichment.As a side effect, the note + wikilink + frontmatter + canvas pipeline
gives the graph native Obsidian-vault ingestion — a vault dropped
anywhere inside the campaign is discovered, indexed, linked, and
searchable alongside campaign artifacts.
via
.gitmarkers; live worktree walk with git-state classification(tracked/untracked/ignored); containerized fixture harness for
repo-state tests.
NodeFolderwith scope_kind metadata(campaign_root, repo_root, submodule_root, campaign_bucket,
artifact_scope, user_scope); artifact-to-scope bridges keep the
existing artifact IDs unchanged.
(title/aliases/tags/type/status), markdown+wiki links, inline tags,
Obsidian-style canvas files, attachment embeds.
posting lists (same folder, shared ancestor, shared tag, shared
frontmatter, shared repo root) with
MaxMembersPerGroup=128/MaxPairs=5000caps. Evidence aggregation produces one inferrededge per pair with reasons JSON in
Edge.Note.graph_meta,indexed_files,search_docs,search_docs_ftsperIMPLEMENTATION_CONTRACTS; FTS5-backed lexical query with snippets,
scope/path-prefix/tracked/type filters, reasons list. Schema tag
graphdb/v2alpha1.Resolve(exact id →exact rel path → top lexical hit) replaces substring node scan;
scope-first TUI with tab-cycled relation modes
(hybrid → structural → explicit → semantic);
render --scope,--mode,--tracked,--untracked.internal/runtimewithIndexState/Status/compatibility rules.refreshhas a real no-opfast path when inventory diff is empty and falls back to
mode=rebuildon schema drift.status --jsonexposesgraph-status/v1alpha1.camp-graph relatedemitsgraph-related/v1alpha1with scope-first ranking (same_scope →explicit_edge → lexical_match); additive, no compile-time coupling
into
camp.NodeRepowithrepo:<rel>IDs asexplicit slice anchors; bounded code-aware
NodeFile/NodePackageextraction for nested boundaries only; campaign-root code skipped
to avoid global overreach.
CompatibilityVerdict(fresh/matching/incompatible), containerized end-to-end tests covering
build/query/refresh/status/related, live ObsidianVault smoke with
wall-clock timings recorded in
docs/release-smoke-results.md.JSON envelopes
query --json→graph-query/v1alpha1related --json→graph-related/v1alpha1refresh --json→graph-refresh/v1alpha1status --json→graph-status/v1alpha1Live vault smoke
All five contract pass conditions hold on a 13,143-node /
4,332-search-doc ObsidianVault snapshot. Wall-clock:
build7.45s(includes SHA-256 fingerprinting for 18,894 indexed files), warm
query \"JobSearch\"21ms,related Action Plan.md30ms.Test plan
go test ./...passes across all packagesgo vet ./...andgo vet -tags=integration ./...cleanignored, nested repos, submodules, customized layouts, FTS5 query
with scope filters, refresh mode reporting, related enrichment,
render scope slicing
nodes, note+frontmatter, links/tags/canvases, inference posting
lists + aggregation, FTS5 indexer round-trip (incl. overwrite +
delete), Querier filters, Related ranking, index_state
round-trip, compatibility verdicts, refresh no-op fast path,
rebuild↔refresh parity, render filters