Conversation
Round 2 follow-up to bb63643. CodeRabbit re-reviewed; r2 closes the residual gaps. Code: - AX-2 docstrings added across helper / internal / public surfaces (including exported struct fields on stats/config types). Local doc coverage now 82.26%. - specs/forge.lthn.ai/core/go-rag.md: remaining UK spelling fix - internal/compat/cli (new) — local shim + go.mod replacements so GOWORK=off verification resolves cleanly without external dep - go.mod / go.sum: cli shim require/replace entries Disposition replies (RESOLVED-COMMENT): - Testify findings on chunk_test.go, cmd_commands_test.go, keyword_test.go, ingest_test.go, test_helpers_test.go: AX-6 bans testify; stdlib helpers convention preserved. - Writer routing, env port validation, endpoint parsing, bounded fan-out, safe type assertions, chunk index, chunkless dedupe, local chunk validation: already addressed in bb63643, verified still in place. - PR title metadata: not a code finding. Verification: gofmt clean, GOWORK=off go vet + go test -count=1 ./... pass with explicit GOPATH/GOMODCACHE/GOCACHE. Closes residual findings on #4 Co-authored-by: Codex <noreply@openai.com>
bash /tmp/v090/audit.sh . → verdict: COMPLIANT (all 7 dimensions zero). go test -count=1 ./... → all green. Co-authored-by: Codex <noreply@openai.com> Co-Authored-By: Virgil <virgil@lethean.io>
📝 WalkthroughWalkthroughComprehensive test suite additions for RAG system components with validation of chunking, ingestion, keyword queries, Qdrant, and Ollama integration. Import pathway refactored from Changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
qdrant.go (1)
474-488:⚠️ Potential issue | 🟠 MajorPreserve the legacy
indexpayload key when decoding chunk metadata.Line 479 only inspects
chunk_index, but this struct now documentsIndexas a compatibility alias. Any collection populated with the olderindexfield will lose chunk metadata inSearch(), which in turn changes chunk-aware ranking and deduplication for existing data.Suggested change
func payloadChunkIndex(payload map[string]any) (int, bool) { if payload == nil { return 0, false } - switch v := payload["chunk_index"].(type) { - case int: - return v, true - case int64: - return int(v), true - case float64: - return int(v), true - default: - return 0, false + for _, key := range []string{"chunk_index", "index"} { + switch v := payload[key].(type) { + case int: + return v, true + case int64: + return int(v), true + case float64: + return int(v), true + } } + return 0, false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@qdrant.go` around lines 474 - 488, The payloadChunkIndex function only reads "chunk_index" and drops legacy "index" values; update payloadChunkIndex to also check payload["index"] (after or before payload["chunk_index"] as desired for precedence) and convert int/int64/float64 the same way so collections using the older "index" key retain their chunk metadata; ensure you still return (int, bool) and reuse the same type-switch logic (int, int64, float64) for the "index" key and prefer the explicit "chunk_index" if you want newer data to win.
🧹 Nitpick comments (3)
rfc_features_test.go (1)
91-104: Optional: strengthen parsing robustness with negative/edge-case tests.
TestEndpointConfigParsingonly covers two valid endpoint formats. Consider adding a few additional unit tests for likely failure modes (e.g., missing port, invalid scheme, malformed host/URL, unsupported scheme) to lock in error handling and prevent regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rfc_features_test.go` around lines 91 - 104, Add negative and edge-case assertions to TestEndpointConfigParsing to exercise qdrantConfigFromEndpoint and ollamaConfigFromEndpoint failure paths: add subtests for missing port (e.g., "https://example.com"), invalid scheme (e.g., "ftp://host:1234"), malformed URL/host (e.g., "http://:1234" or "http:///"), and unsupported/empty scheme, and assert that err is non-nil and configuration values are not set or defaulted as expected; use the same helper assertions (assertNoError/assertTrue/assertEqual) but invert them for expected failures to lock in error handling and prevent regressions.ax7_keyword_query_test.go (1)
424-437: Checkerrbefore consumingseq.Line 429 returns both
seqanderr, but the loop starts at Line 432 before the test asserts success at Line 435. IfQuerySeq()regresses and returns an error, this test can fail with a noisier iterator problem instead of the underlying assertion.Suggested change
seq, err := QuerySeq(core.Background(), store, newMockEmbedder(2), "query", QueryConfig{Collection: "docs", Limit: 5, Threshold: 0.1}) + core.AssertNoError(t, err) var results []QueryResult for result := range seq { results = append(results, result) } - core.AssertNoError(t, err) core.AssertLen(t, results, 1) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ax7_keyword_query_test.go` around lines 424 - 437, Test checks consume the seq channel before verifying err; move the error check so QuerySeq's err is asserted before iterating. In TestAX7_QuerySeq_Good check the returned err from QuerySeq (variable err) with core.AssertNoError(t, err) immediately after the call to QuerySeq (or return/fail on non-nil) and only then range over seq; reference TestAX7_QuerySeq_Good, QuerySeq, seq, and err to locate and update the test.ax7_ingest_collections_test.go (1)
87-96: Assert the returned error before ranging the iterator.Line 89 captures
(seq, err), but Line 91 consumesseqbefore Line 95 checkserr. IfListCollectionsSeq()starts failing here, the test can mask the real regression with a less useful iterator failure. Movecore.AssertNoError(t, err)above the loop.Suggested change
func TestAX7_ListCollectionsSeq_Ugly(t *core.T) { store := newMockVectorStore() seq, err := ListCollectionsSeq(core.Background(), store) + core.AssertNoError(t, err) count := 0 for range seq { count++ } - core.AssertNoError(t, err) core.AssertEqual(t, 0, count) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ax7_ingest_collections_test.go` around lines 87 - 96, The test TestAX7_ListCollectionsSeq_Ugly consumes the iterator seq before verifying the call to ListCollectionsSeq succeeded; move the assertion core.AssertNoError(t, err) immediately after the call to ListCollectionsSeq(core.Background(), store) (before iterating over seq) so the test fails on the actual error from ListCollectionsSeq rather than on iterator behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/rag/ax7_commands_test.go`:
- Around line 3-34: Replace usage of the external test harness (core.T and
core.Assert* calls) with stdlib testing.T and the local helpers from
test_helpers_test.go: change the test signatures to use (t *testing.T), import
"testing" instead of core, and replace
core.AssertLen/core.AssertEqual/core.AssertPanics/core.AssertTrue/core.AssertNotNil
with the corresponding local helpers (e.g., assertLen, assertEqual, assertPanics
or use assertTrue/assertNotNil equivalents provided). Keep calls to
AddRAGSubcommands, ragCmd and cli.NewGroup unchanged so the tests still verify
the same behavior.
In `@go.mod`:
- Around line 15-37: The go.mod indirect dependencies
github.com/buger/jsonparser and google.golang.org/grpc are pinned to vulnerable
versions (jsonparser v1.1.1 and grpc v1.79.2); update these modules to patched
releases (upgrade github.com/buger/jsonparser to a version that addresses
GHSA-6g7g-w4f8-9c9x / GO-2026-4514 and google.golang.org/grpc to a version that
fixes GHSA-p77j-4mvh-x3m3 / GO-2026-4762), then run your module update/cleanup
(go get to the patched versions and go mod tidy) and verify tests/builds to
ensure no regressions; locate occurrences by the module paths
github.com/buger/jsonparser and google.golang.org/grpc in the diff.
---
Outside diff comments:
In `@qdrant.go`:
- Around line 474-488: The payloadChunkIndex function only reads "chunk_index"
and drops legacy "index" values; update payloadChunkIndex to also check
payload["index"] (after or before payload["chunk_index"] as desired for
precedence) and convert int/int64/float64 the same way so collections using the
older "index" key retain their chunk metadata; ensure you still return (int,
bool) and reuse the same type-switch logic (int, int64, float64) for the "index"
key and prefer the explicit "chunk_index" if you want newer data to win.
---
Nitpick comments:
In `@ax7_ingest_collections_test.go`:
- Around line 87-96: The test TestAX7_ListCollectionsSeq_Ugly consumes the
iterator seq before verifying the call to ListCollectionsSeq succeeded; move the
assertion core.AssertNoError(t, err) immediately after the call to
ListCollectionsSeq(core.Background(), store) (before iterating over seq) so the
test fails on the actual error from ListCollectionsSeq rather than on iterator
behavior.
In `@ax7_keyword_query_test.go`:
- Around line 424-437: Test checks consume the seq channel before verifying err;
move the error check so QuerySeq's err is asserted before iterating. In
TestAX7_QuerySeq_Good check the returned err from QuerySeq (variable err) with
core.AssertNoError(t, err) immediately after the call to QuerySeq (or
return/fail on non-nil) and only then range over seq; reference
TestAX7_QuerySeq_Good, QuerySeq, seq, and err to locate and update the test.
In `@rfc_features_test.go`:
- Around line 91-104: Add negative and edge-case assertions to
TestEndpointConfigParsing to exercise qdrantConfigFromEndpoint and
ollamaConfigFromEndpoint failure paths: add subtests for missing port (e.g.,
"https://example.com"), invalid scheme (e.g., "ftp://host:1234"), malformed
URL/host (e.g., "http://:1234" or "http:///"), and unsupported/empty scheme, and
assert that err is non-nil and configuration values are not set or defaulted as
expected; use the same helper assertions (assertNoError/assertTrue/assertEqual)
but invert them for expected failures to lock in error handling and prevent
regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89b9d71f-81cc-4a6b-971a-359287e60861
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.suminternal/compat/cli/go.sumis excluded by!**/*.sum
📒 Files selected for processing (36)
ax7_chunk_core_test.goax7_ingest_collections_test.goax7_keyword_query_test.goax7_ollama_test.goax7_qdrant_test.gobenchmark_gpu_test.gobenchmark_test.gochunk.gochunk_test.gocmd/rag/ax7_commands_test.gocmd/rag/cmd_collections.gocmd/rag/cmd_ingest.gocmd/rag/cmd_query.gocmd/rag/cmd_rag.gocollections_test.goendpoint.gogo.modhelpers.gohelpers_test.goingest.goingest_test.gointegration_test.gointernal/compat/cli/go.modinternal/compat/cli/pkg/cli/ax7_cli_test.gointernal/compat/cli/pkg/cli/cli.gokeyword.gomock_test.goollama.goqdrant.goqdrant_integration_test.goquery.goquery_test.gorfc_features_test.gospecs/forge.lthn.ai/core/go-rag.mdtest_helpers_test.govectorstore.go
Comment on lines
+3
to
+34
| import ( | ||
| core "dappco.re/go" | ||
| "dappco.re/go/cli/pkg/cli" | ||
| ) | ||
|
|
||
| func TestAX7_AddRAGSubcommands_Good(t *core.T) { | ||
| parent := cli.NewGroup("root", "", "") | ||
| AddRAGSubcommands(parent) | ||
|
|
||
| core.AssertLen(t, parent.Commands(), 1) | ||
| core.AssertEqual(t, "rag", parent.Commands()[0].Name()) | ||
| } | ||
|
|
||
| func TestAX7_AddRAGSubcommands_Bad(t *core.T) { | ||
| called := false | ||
| core.AssertPanics(t, func() { | ||
| called = true | ||
| AddRAGSubcommands(nil) | ||
| }) | ||
|
|
||
| core.AssertTrue(t, called) | ||
| core.AssertNotNil(t, ragCmd) | ||
| } | ||
|
|
||
| func TestAX7_AddRAGSubcommands_Ugly(t *core.T) { | ||
| parent := cli.NewGroup("root", "", "") | ||
| AddRAGSubcommands(parent) | ||
| AddRAGSubcommands(parent) | ||
|
|
||
| core.AssertLen(t, parent.Commands(), 1) | ||
| core.AssertLen(t, parent.Commands()[0].Commands(), 3) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify non-stdlib test signatures/usages in test files.
rg -nP --type=go '^func\s+Test\w+\(t\s+\*core\.T\)' -g '*_test.go'
rg -nP --type=go '\bcore\.(Assert|Require)\w+\(' -g '*_test.go'Repository: dAppCore/go-rag
Length of output: 45105
Switch all AX7 tests to stdlib testing with local assertion helpers from test_helpers_test.go.
The test functions at lines 8, 16, and 27 use *core.T and core.Assert* calls from the dappco.re/go package, violating the project guideline requiring stdlib testing only. This pattern appears across all AX7 test files (e.g., ax7_ollama_test.go, ax7_qdrant_test.go, ax7_chunk_core_test.go, ax7_keyword_query_test.go, ax7_ingest_collections_test.go). Refactor to use *testing.T with stdlib checks or the local helpers (assertNoError, assertEqual, assertLen, assertContains, etc.) defined in test_helpers_test.go.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/rag/ax7_commands_test.go` around lines 3 - 34, Replace usage of the
external test harness (core.T and core.Assert* calls) with stdlib testing.T and
the local helpers from test_helpers_test.go: change the test signatures to use
(t *testing.T), import "testing" instead of core, and replace
core.AssertLen/core.AssertEqual/core.AssertPanics/core.AssertTrue/core.AssertNotNil
with the corresponding local helpers (e.g., assertLen, assertEqual, assertPanics
or use assertTrue/assertNotNil equivalents provided). Keep calls to
AddRAGSubcommands, ragCmd and cli.NewGroup unchanged so the tests still verify
the same behavior.
Comment on lines
+15
to
37
| dappco.re/go/core v0.8.0-alpha.1 // indirect | ||
| dappco.re/go/inference v0.8.0-alpha.1 // indirect | ||
| dappco.re/go/log v0.8.0-alpha.1 // indirect | ||
| github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect | ||
| github.com/bahlo/generic-list-go v0.2.0 // indirect | ||
| github.com/buger/jsonparser v1.1.1 // indirect | ||
| github.com/charmbracelet/bubbletea v1.3.10 // indirect | ||
| github.com/charmbracelet/colorprofile v0.4.3 // indirect | ||
| github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834 // indirect | ||
| github.com/charmbracelet/x/ansi v0.11.6 // indirect | ||
| github.com/charmbracelet/x/cellbuf v0.0.15 // indirect | ||
| github.com/charmbracelet/x/term v0.2.2 // indirect | ||
| github.com/clipperhouse/displaywidth v0.11.0 // indirect | ||
| github.com/clipperhouse/uax29/v2 v2.7.0 // indirect | ||
| github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect | ||
| github.com/google/uuid v1.6.0 // indirect | ||
| github.com/inconshreveable/mousetrap v1.1.0 // indirect | ||
| github.com/lucasb-eyer/go-colorful v1.3.0 // indirect | ||
| github.com/mailru/easyjson v0.9.2 // indirect | ||
| github.com/mattn/go-isatty v0.0.20 // indirect | ||
| github.com/mattn/go-localereader v0.0.1 // indirect | ||
| github.com/mattn/go-runewidth v0.0.21 // indirect | ||
| github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect | ||
| github.com/muesli/cancelreader v0.2.2 // indirect | ||
| github.com/muesli/termenv v0.16.0 // indirect | ||
| github.com/rivo/uniseg v0.4.7 // indirect | ||
| github.com/spf13/cobra v1.10.2 // indirect | ||
| github.com/spf13/pflag v1.0.10 // indirect | ||
| github.com/wk8/go-ordered-map/v2 v2.1.8 // indirect | ||
| github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect | ||
| go.opentelemetry.io/otel v1.42.0 // indirect | ||
| go.opentelemetry.io/otel/sdk/metric v1.42.0 // indirect | ||
| golang.org/x/crypto v0.49.0 // indirect | ||
| golang.org/x/net v0.52.0 // indirect | ||
| golang.org/x/sys v0.42.0 // indirect | ||
| golang.org/x/term v0.41.0 // indirect | ||
| golang.org/x/text v0.35.0 // indirect | ||
| golang.org/x/sys v0.43.0 // indirect | ||
| golang.org/x/term v0.42.0 // indirect | ||
| golang.org/x/text v0.36.0 // indirect | ||
| gonum.org/v1/gonum v0.17.0 // indirect | ||
| google.golang.org/genproto/googleapis/rpc v0.0.0-20260316180232-0b37fe3546d5 // indirect | ||
| google.golang.org/grpc v1.79.2 // indirect | ||
| google.golang.org/protobuf v1.36.11 // indirect | ||
| gopkg.in/yaml.v3 v3.0.1 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
query_osv () {
local pkg="$1"
local ver="$2"
curl -sS https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d "{\"package\":{\"name\":\"${pkg}\",\"ecosystem\":\"Go\"},\"version\":\"${ver}\"}" |
jq -r --arg p "$pkg" --arg v "$ver" '
"package=\($p) version=\($v)",
(if (.vulns|length)==0 then " no vulns returned"
else (.vulns[] | " " + .id + " | " + (.summary // "no summary"))
end)
'
}
query_osv "github.com/buger/jsonparser" "1.1.1"
query_osv "google.golang.org/grpc" "1.79.2"Repository: dAppCore/go-rag
Length of output: 499
Upgrade vulnerable indirect dependencies before release.
Line 19 and line 35 currently pin versions with known security vulnerabilities (github.com/buger/jsonparser v1.1.1 and google.golang.org/grpc v1.79.2). Both have active advisories: jsonparser has denial-of-service findings (GHSA-6g7g-w4f8-9c9x, GO-2026-4514), and gRPC-Go has an authorisation bypass via missing leading slash in :path (GHSA-p77j-4mvh-x3m3, GO-2026-4762). These should be remediated as a blocker before release.
🧰 Tools
🪛 OSV Scanner (2.3.5)
[HIGH] 19-19: github.com/buger/jsonparser 1.1.1: Denial of service in github.com/buger/jsonparser
(GO-2026-4514)
[HIGH] 19-19: github.com/buger/jsonparser 1.1.1: Denial of service in github.com/buger/jsonparser
[CRITICAL] 35-35: google.golang.org/grpc 1.79.2: Authorization bypass in gRPC-Go via missing leading slash in :path in google.golang.org/grpc
(GO-2026-4762)
[CRITICAL] 35-35: google.golang.org/grpc 1.79.2: gRPC-Go has an authorization bypass via missing leading slash in :path
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 15 - 37, The go.mod indirect dependencies
github.com/buger/jsonparser and google.golang.org/grpc are pinned to vulnerable
versions (jsonparser v1.1.1 and grpc v1.79.2); update these modules to patched
releases (upgrade github.com/buger/jsonparser to a version that addresses
GHSA-6g7g-w4f8-9c9x / GO-2026-4514 and google.golang.org/grpc to a version that
fixes GHSA-p77j-4mvh-x3m3 / GO-2026-4762), then run your module update/cleanup
(go get to the patched versions and go mod tidy) and verify tests/builds to
ensure no regressions; locate occurrences by the module paths
github.com/buger/jsonparser and google.golang.org/grpc in the diff.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings this repo to verdict: COMPLIANT against the v0.9.0 audit.
🤖 Generated with Claude Code + Codex
Co-Authored-By: Codex noreply@openai.com
Co-Authored-By: Virgil virgil@lethean.io
Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests