Skip to content

Commit 2051693

Browse files
authored
engine: rename pageindex→treewalk + auto strategy default (HAL-106) (#35)
* engine: rename pageindex strategy to treewalk (HAL-106) Strip the borrowed PageIndex name — treewalk is now the native identifier for Vectorless's tree-navigation strategy across the strategy id, Go types, config keys, the /v1/answer/treewalk endpoint, and all tests. Pure rename; go build, go vet, and package tests all green. Remaining HAL-106 sub-tasks: promote treewalk to default strategy, dedup the agentic overlap, and fold the dedicated endpoint into /v1/answer. * engine: add auto strategy as the default (HAL-106) Smart default selection: route small documents (total leaf content within the call budget) to single-pass for one fast cheap call, and larger or more complex documents to treewalk for agentic navigation. New AutoStrategy in pkg/retrieval with unit tests; wired into config (default value + validation) and both binaries' strategy builders + the selectable-strategy set. go build, go vet, and package tests all green. * engine: address PR #35 review (auto page-fallback, nil-safety, finish rename) Addresses the Copilot + CodeRabbit review on #35: - auto: non-paged docs (no PageStart/PageEnd — Markdown/HTML/DOCX/TXT) now route to single-pass, never treewalk (which navigates by page range) + nil-safe pick/Select/SelectWithCost so a misconfigured Auto can't panic. - complete the rename in non-Go files: openapi.yaml + config.example*.yaml (incl. the uppercase env-var refs) — zero pageindex remains anywhere. - query.go: list 'auto' in the strategy-override doc comment. - config_test: exercise 'auto' as a valid Validate() strategy. - tests: non-paged fallback + nil-safety. Out of scope (filed separately): treewalk SSE streams reasoning regardless of IncludeReasoning (pre-existing behaviour, not introduced here). * engine: green the CI baseline (HAL-116) — race fix, dep + toolchain bump, lint - toc_builder: fix the data race in verifyTitlesConcurrent — each goroutine now accumulates a local Usage and folds it under the mutex, instead of sharing &localUse across errgroup goroutines (concurrent usage.add writes). - deps: golang.org/x/net v0.53.0 -> v0.55.0 (GO-2026-5030); add toolchain go1.25.11 + pin CI go-version (stdlib GO-2026-5039 / GO-2026-5037). - lint: //lint:ignore U1000 the four staged helpers with reasons; drop the genuinely-unused idempotencyCache.mu field. These CI failures were pre-existing (surfaced on PR #35, not caused by the rename). go build + staticcheck green locally; -race verified in CI. Closes HAL-116. * engine: fix the pdftable parser data race + golangci //nolint (HAL-116) - pkg/parser: serialize pdftable.OpenBytes behind a package mutex (openPDFBytes). pdftable mutates package-level state on open and is not concurrency-safe; the -race detector flagged concurrent opens (real in prod — ingest workers parse in parallel). Stopgap; the proper fix belongs in pdftable (Foundational Libs). - convert the unused-helper suppressions from staticcheck //lint:ignore to golangci-lint //nolint:unused,staticcheck (CI lint uses golangci-lint). go build + go vet + parser tests green. * engine: green the CI lint baseline — errcheck / ineffassign / staticcheck (HAL-117) Completes the errcheck/ineffassign cleanup across the repo, including the renamed files (treewalk.go / answer_treewalk.go) the off-main pass couldn't reach: - errcheck: _, _ = fmt.Fprintf on the SSE writers; unchecked returns handled across cmd/benchmark, handlers, connecthandler, parser, storage, queue, db, ingest. - ineffassign: drop the ineffectual 'written += remaining' before break in treewalk_strategy. - staticcheck: real TLS-disabled assertion (was an empty branch) in config_test; De Morgan's rewrite in retrieval_test; drop embedded-field selector in decompose_test. - misspell: ignore-rule for 'strat' (our 'strategy' abbreviation) in .golangci.yml. - formatting normalized via the golangci formatters. golangci-lint run: 0 issues. go build + unit tests green. Closes HAL-117. * engine: remove dead funcs + guard pdftable ExtractTables race (HAL-116/117) - lint: the CI 'lint' job runs BOTH golangci-lint and standalone staticcheck (honnef), and the latter reads //lint:ignore, not golangci's //nolint. Rather than juggle two directive formats, delete the 4 genuinely-unused funcs (treeWalkTraceTokenFromCitations, originMatches, isTimeout, isEncryptedPDFError) — both linters agree they're dead. (Re-add with callers when HAL-73/HAL-112 land.) - race: pdftable.ExtractTables also mutates package-level state — a second pdftable race distinct from OpenBytes; serialize it on the same mutex. Stopgap; the root fix is HAL-118 (make pdftable concurrency-safe). golangci-lint + standalone staticcheck: 0 issues. go build + unit tests green. * engine: go mod tidy — blank line before toolchain directive (HAL-116) The ubuntu-only 'Verify go.mod/go.sum are tidy' CI step requires the canonical format go mod tidy emits: a blank line between the 'go' and 'toolchain' directives. My earlier manual insertion of the toolchain line omitted it.
1 parent f150e5b commit 2051693

59 files changed

Lines changed: 1256 additions & 970 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ jobs:
1717
fail-fast: false
1818
matrix:
1919
os: [ubuntu-latest, macos-latest, windows-latest]
20-
go: ["1.25"]
20+
go: ["1.25.11"]
2121
steps:
2222
- uses: actions/checkout@v6
2323

@@ -70,7 +70,7 @@ jobs:
7070
- name: Set up Go
7171
uses: actions/setup-go@v6
7272
with:
73-
go-version: "1.25"
73+
go-version: "1.25.11"
7474
cache: true
7575

7676
# MinIO can't run as a `services:` container because GitHub Actions
@@ -115,7 +115,7 @@ jobs:
115115
- name: Set up Go
116116
uses: actions/setup-go@v6
117117
with:
118-
go-version: "1.25"
118+
go-version: "1.25.11"
119119
cache: true
120120

121121
- name: staticcheck
@@ -137,7 +137,7 @@ jobs:
137137
- uses: actions/checkout@v6
138138
- uses: actions/setup-go@v6
139139
with:
140-
go-version: "1.25"
140+
go-version: "1.25.11"
141141
cache: true
142142
- name: Install govulncheck
143143
run: go install golang.org/x/vuln/cmd/govulncheck@latest

.golangci.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ linters:
3030
- (net/http.ResponseWriter).Write
3131
- (github.com/jackc/pgx/v5.Tx).Rollback
3232
- (github.com/hallelx2/vectorless-engine/pkg/queue.Queue).Close
33+
misspell:
34+
# "strat" is our abbreviation for "strategy" (variable + key names),
35+
# not a misspelling of "start".
36+
ignore-rules:
37+
- strat
3338
exclusions:
3439
rules:
3540
- path: _test\.go

cmd/benchmark/main.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func main() {
7777
ID string `json:"id"`
7878
} `json:"sections"`
7979
}
80-
json.Unmarshal(treeResp, &treeData)
80+
_ = json.Unmarshal(treeResp, &treeData) // benchmark best-effort parse
8181
sectionID := ""
8282
if len(treeData.Sections) > 1 {
8383
sectionID = treeData.Sections[1].ID // pick a child section
@@ -87,9 +87,9 @@ func main() {
8787
fmt.Printf("Using section ID: %s\n\n", sectionID)
8888

8989
type result struct {
90-
name string
91-
rest []time.Duration
92-
grpc []time.Duration
90+
name string
91+
rest []time.Duration
92+
grpc []time.Duration
9393
}
9494

9595
var results []result
@@ -101,12 +101,12 @@ func main() {
101101
fmt.Print(".")
102102
// REST
103103
start := time.Now()
104-
restGET(base + "/v1/health")
104+
_, _ = restGET(base + "/v1/health")
105105
r.rest = append(r.rest, time.Since(start))
106106

107107
// gRPC (Connect)
108108
start = time.Now()
109-
healthClient.Check(ctx, connect.NewRequest(&v1.HealthCheckRequest{}))
109+
_, _ = healthClient.Check(ctx, connect.NewRequest(&v1.HealthCheckRequest{}))
110110
r.grpc = append(r.grpc, time.Since(start))
111111
}
112112
fmt.Println(" done")
@@ -118,11 +118,11 @@ func main() {
118118
for i := 0; i < n; i++ {
119119
fmt.Print(".")
120120
start := time.Now()
121-
restGET(base + "/v1/documents")
121+
_, _ = restGET(base + "/v1/documents")
122122
r.rest = append(r.rest, time.Since(start))
123123

124124
start = time.Now()
125-
docsClient.ListDocuments(ctx, connect.NewRequest(&v1.ListDocumentsRequest{Limit: 10}))
125+
_, _ = docsClient.ListDocuments(ctx, connect.NewRequest(&v1.ListDocumentsRequest{Limit: 10}))
126126
r.grpc = append(r.grpc, time.Since(start))
127127
}
128128
fmt.Println(" done")
@@ -134,11 +134,11 @@ func main() {
134134
for i := 0; i < n; i++ {
135135
fmt.Print(".")
136136
start := time.Now()
137-
restGET(base + "/v1/documents/" + *docID + "/tree")
137+
_, _ = restGET(base + "/v1/documents/" + *docID + "/tree")
138138
r.rest = append(r.rest, time.Since(start))
139139

140140
start = time.Now()
141-
docsClient.GetDocumentTree(ctx, connect.NewRequest(&v1.GetDocumentTreeRequest{
141+
_, _ = docsClient.GetDocumentTree(ctx, connect.NewRequest(&v1.GetDocumentTreeRequest{
142142
DocumentId: *docID,
143143
}))
144144
r.grpc = append(r.grpc, time.Since(start))
@@ -153,11 +153,11 @@ func main() {
153153
for i := 0; i < n; i++ {
154154
fmt.Print(".")
155155
start := time.Now()
156-
restGET(base + "/v1/sections/" + sectionID)
156+
_, _ = restGET(base + "/v1/sections/" + sectionID)
157157
r.rest = append(r.rest, time.Since(start))
158158

159159
start = time.Now()
160-
docsClient.GetSection(ctx, connect.NewRequest(&v1.GetSectionRequest{
160+
_, _ = docsClient.GetSection(ctx, connect.NewRequest(&v1.GetSectionRequest{
161161
SectionId: sectionID,
162162
}))
163163
r.grpc = append(r.grpc, time.Since(start))
@@ -182,12 +182,12 @@ func main() {
182182
"query": q,
183183
})
184184
start := time.Now()
185-
restPOST(base+"/v1/query", body)
185+
_, _ = restPOST(base+"/v1/query", body)
186186
r.rest = append(r.rest, time.Since(start))
187187

188188
// gRPC (Connect)
189189
start = time.Now()
190-
queryClient.Query(ctx, connect.NewRequest(&v1.QueryRequest{
190+
_, _ = queryClient.Query(ctx, connect.NewRequest(&v1.QueryRequest{
191191
DocumentId: *docID,
192192
Query: q,
193193
}))
@@ -246,7 +246,7 @@ func restGET(url string) ([]byte, error) {
246246
if err != nil {
247247
return nil, err
248248
}
249-
defer resp.Body.Close()
249+
defer func() { _ = resp.Body.Close() }() // best-effort close
250250
return io.ReadAll(resp.Body)
251251
}
252252

@@ -255,7 +255,7 @@ func restPOST(url string, body []byte) ([]byte, error) {
255255
if err != nil {
256256
return nil, err
257257
}
258-
defer resp.Body.Close()
258+
defer func() { _ = resp.Body.Close() }() // best-effort close
259259
return io.ReadAll(resp.Body)
260260
}
261261

cmd/engine/main.go

Lines changed: 48 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func run() error {
9090
if err != nil {
9191
return fmt.Errorf("init queue: %w", err)
9292
}
93-
defer q.Close()
93+
defer func() { _ = q.Close() }() // best-effort close
9494

9595
llmClient, err := buildLLM(cfg.LLM)
9696
if err != nil {
@@ -206,41 +206,41 @@ func run() error {
206206
}
207207
q.Register(queue.KindIngestDocument, pipeline.Handler())
208208

209-
// /v1/answer/pageindex gets its OWN PageIndexStrategy instance,
209+
// /v1/answer/treewalk gets its OWN TreeWalkStrategy instance,
210210
// independent of whatever selection strategy is configured in
211211
// retrieval.strategy. This way the endpoint is always available
212-
// (gated by retrieval.pageindex.enabled), even on a deployment
212+
// (gated by retrieval.treewalk.enabled), even on a deployment
213213
// using chunked-tree as its default selection path.
214-
var pageIndexStrategy *retrieval.PageIndexStrategy
215-
if cfg.Retrieval.PageIndex.Enabled && llmClient != nil {
216-
pageIndexStrategy = buildPageIndexStrategy(cfg.Retrieval, llmClient, store)
217-
logger.Info("retrieval: pageindex answer endpoint enabled",
218-
"max_hops", pageIndexStrategy.MaxHops,
219-
"page_content_limit", pageIndexStrategy.PageContentLimit,
220-
"model_override", cfg.Retrieval.PageIndex.Model,
214+
var treeWalkStrategy *retrieval.TreeWalkStrategy
215+
if cfg.Retrieval.TreeWalk.Enabled && llmClient != nil {
216+
treeWalkStrategy = buildTreeWalkStrategy(cfg.Retrieval, llmClient, store)
217+
logger.Info("retrieval: treewalk answer endpoint enabled",
218+
"max_hops", treeWalkStrategy.MaxHops,
219+
"page_content_limit", treeWalkStrategy.PageContentLimit,
220+
"model_override", cfg.Retrieval.TreeWalk.Model,
221221
)
222222
}
223223

224224
deps := api.Deps{
225-
Logger: logger,
226-
DB: pool,
227-
Storage: store,
228-
Queue: q,
229-
Strategy: strategy,
230-
Version: version,
231-
MultiDoc: multiDoc,
232-
LLM: llmClient,
233-
LLMModel: modelFor(cfg.LLM),
234-
AnswerSpan: cfg.Retrieval.AnswerSpan,
235-
Answer: cfg.Retrieval.Answer,
236-
Planner: planner,
237-
Planning: cfg.Retrieval.Planning,
238-
ReRanker: reRanker,
239-
ReRank: cfg.Retrieval.ReRank,
240-
Replay: replayStore,
241-
Abstain: cfg.Retrieval.Abstain,
242-
PageIndexStrategy: pageIndexStrategy,
243-
PageIndex: cfg.Retrieval.PageIndex,
225+
Logger: logger,
226+
DB: pool,
227+
Storage: store,
228+
Queue: q,
229+
Strategy: strategy,
230+
Version: version,
231+
MultiDoc: multiDoc,
232+
LLM: llmClient,
233+
LLMModel: modelFor(cfg.LLM),
234+
AnswerSpan: cfg.Retrieval.AnswerSpan,
235+
Answer: cfg.Retrieval.Answer,
236+
Planner: planner,
237+
Planning: cfg.Retrieval.Planning,
238+
ReRanker: reRanker,
239+
ReRank: cfg.Retrieval.ReRank,
240+
Replay: replayStore,
241+
Abstain: cfg.Retrieval.Abstain,
242+
TreeWalkStrategy: treeWalkStrategy,
243+
TreeWalk: cfg.Retrieval.TreeWalk,
244244
}
245245

246246
srv := &http.Server{
@@ -393,36 +393,38 @@ func buildStrategy(c config.RetrievalConfig, client llmgate.Client, store storag
393393
}
394394
a.ModelOverride = c.Agentic.Model
395395
return a
396-
case "pageindex":
397-
return buildPageIndexStrategy(c, client, store)
396+
case "treewalk":
397+
return buildTreeWalkStrategy(c, client, store)
398+
case "auto":
399+
return retrieval.NewAuto(retrieval.NewSinglePass(client), buildTreeWalkStrategy(c, client, store))
398400
default:
399401
return retrieval.NewChunkedTree(client)
400402
}
401403
}
402404

403-
// buildPageIndexStrategy constructs the page-based agentic
405+
// buildTreeWalkStrategy constructs the page-based agentic
404406
// strategy with the storage-backed PageLoader and the configured
405-
// caps. Used by buildStrategy when retrieval.strategy=pageindex AND
406-
// by the /v1/answer/pageindex endpoint setup (which wires its own
407+
// caps. Used by buildStrategy when retrieval.strategy=treewalk AND
408+
// by the /v1/answer/treewalk endpoint setup (which wires its own
407409
// instance regardless of the selection strategy).
408410
//
409411
// The TOCProvider is left nil here. PR-A (toc-tree-builder) adds
410412
// documents.toc_tree + a DB-backed provider; until it lands the
411413
// strategy degrades to its synthesised view, which is the
412414
// documented fallback path.
413-
func buildPageIndexStrategy(c config.RetrievalConfig, client llmgate.Client, store storage.Storage) *retrieval.PageIndexStrategy {
414-
p := retrieval.NewPageIndexStrategy(client)
415+
func buildTreeWalkStrategy(c config.RetrievalConfig, client llmgate.Client, store storage.Storage) *retrieval.TreeWalkStrategy {
416+
p := retrieval.NewTreeWalkStrategy(client)
415417
p.PageLoader = storagePageLoader{s: store}
416-
if c.PageIndex.MaxHops > 0 {
417-
p.MaxHops = c.PageIndex.MaxHops
418+
if c.TreeWalk.MaxHops > 0 {
419+
p.MaxHops = c.TreeWalk.MaxHops
418420
}
419-
if c.PageIndex.PageContentLimit > 0 {
420-
p.PageContentLimit = c.PageIndex.PageContentLimit
421+
if c.TreeWalk.PageContentLimit > 0 {
422+
p.PageContentLimit = c.TreeWalk.PageContentLimit
421423
}
422-
if c.PageIndex.MaxCitations > 0 {
423-
p.MaxCitations = c.PageIndex.MaxCitations
424+
if c.TreeWalk.MaxCitations > 0 {
425+
p.MaxCitations = c.TreeWalk.MaxCitations
424426
}
425-
p.ModelOverride = c.PageIndex.Model
427+
p.ModelOverride = c.TreeWalk.Model
426428
return p
427429
}
428430

@@ -437,14 +439,14 @@ func (sf storageFetcher) Get(ctx context.Context, ref string) ([]byte, error) {
437439
if err != nil {
438440
return nil, err
439441
}
440-
defer rc.Close()
442+
defer func() { _ = rc.Close() }() // best-effort close
441443
return io.ReadAll(rc)
442444
}
443445

444446
// storagePageLoader adapts a storage.Storage to
445447
// retrieval.PageContentLoader. Mirrors storageFetcher but lives
446448
// behind a separate interface so the two callers (agentic /
447-
// pageindex) can be wired independently. The PageIndex strategy
449+
// treewalk) can be wired independently. The TreeWalk strategy
448450
// materialises section bodies once per get_pages observation, so
449451
// reading the full reader into a []byte is the right shape.
450452
type storagePageLoader struct{ s storage.Storage }
@@ -454,7 +456,7 @@ func (l storagePageLoader) Load(ctx context.Context, ref string) ([]byte, error)
454456
if err != nil {
455457
return nil, err
456458
}
457-
defer rc.Close()
459+
defer func() { _ = rc.Close() }() // best-effort close
458460
return io.ReadAll(rc)
459461
}
460462

0 commit comments

Comments
 (0)