Trim peripheral features, improve search, and fix review findings - #1
Merged
Conversation
Trim dev-only feature surface to lighten the tool. The benchmark and eval packages are self-contained harnesses (benchmark shells out to the external claude CLI; eval compares golden corpora) reachable only from their own CLI commands. docker-compose.langfuse.yaml was orphaned observability infra referenced by no build target or code. Drops the two cobra registrations and the test helpers orphaned by the deleted tests. No core build/query path touched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI previously ran only vet + builds; the Go test suite was never exercised. Add a fts5-tagged test step so regressions are caught. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The DB-scan fallback fired whenever FTS returned fewer distinct files than the wide candidate ceiling (50-500), which is nearly always, making retrieval O(namespace size) instead of O(matches). Gate the scan on the caller's actual result limit so it only supplements genuinely sparse-FTS queries, and cap the fallback load at scanRowCap to bound worst cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FTS/ts_rank candidate order was discarded and results were sorted purely by the hand-tuned annotation score, so a high-scoring scan supplement could bury a genuine search-engine hit. Capture each file's search-engine rank before the scan merge and sort engine-first: backend hits keep their relevance order and outrank scan-only supplements, with the annotation score as the refining tie-break. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Exact tsquery cannot match misspelled queries. When tsquery underfills the requested limit, supplement with pg_trgm trigram similarity on nodes.name / nodes.qualified_name so typos still return candidates. The extension and its GIN indexes are created best-effort in Migrate: if pg_trgm is unavailable (insufficient privilege) it logs a warning and search degrades to exact-only. The fuzzy query swallows errors so a missing extension never breaks exact search. Behaviorally validated only against a live Postgres via the postgres build tag (TEST_POSTGRES_DSN); not exercised by the default/CI suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The symlink-walk, canonicalization, and Rel-based containment mechanics were duplicated across namespacefs and the docs/analysis/parse MCP handlers. Extract the three genuinely-shared primitives — EnsureNoSymlinkInPath, Canonical(path, allowMissingLeaf) (merging the former canonicalPath/canonicalExistingPath pair), and IsWithinRoot — into a single internal/safepath package with direct unit tests, closing a gap where namespacefs had none. The three distinct containment *policies* (prefix-reject-symlink in docs, Rel-follow-symlink in analysis, walk-reject-symlink in namespacefs) and all error strings stay at their call sites: merging them would change security behavior. Structural change only; all handler path-traversal and symlink-escape tests unchanged and green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address code-review findings on the fuzzy fallback: - Use the index-accelerated `<%` operator (rewritten to `%>`, which gin_trgm_ops supports) instead of `word_similarity(?, col) >= const`, which can never use the trigram indexes. Confirmed via EXPLAIN that the operator form yields a Bitmap Index Scan. - Fire fuzzy only on a total exact-FTS miss, not whenever exact underfills the limit, so precise queries are no longer diluted with loose matches. - Scope fuzzy to nodes that have a search_documents row (JOIN), keeping it within the same corpus and kind mix as the exact path. - Set the word_similarity threshold via transaction-local set_config so the operator cutoff matches the tuned value; log real failures at Warn while still degrading gracefully, and do not log on context cancellation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- scanDBCandidates now warns when the fallback scan hits scanRowCap, so a silently truncated result set is observable instead of looking complete. - Remove the pointless `var ( _ = dbCandidateFloor; _ = dbCandidateCap )` block: both constants are used by DBCandidateLimit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The caller-supplied base ref of detect_changes/get_affected_flows sat before "--" with no validation, so values like --output=<path> were parsed as git diff options. Reject refs starting with '-' and add the "--" separator to ChangedFiles. Also run diff with core.quotePath=false: quoted octal output for non-ASCII paths broke hunk-to-file attribution and file matching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
/status serializes sync-queue repo names, branches, and raw error strings; it was registered without the auth middleware that protects /mcp and /wiki/api/. /health and /ready stay open for probes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
resolveDocPath listed "." among its roots, so /wiki/api/doc and /wiki/api/context could read any file under the process CWD (.env, go.mod, source) as a "doc". Resolve shared docs against the docs/ directory itself for both relative and absolute paths. Also drop the orphaned retrieveResult type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An explicit namespace:"default" resolved files under namespaces/default/ while resolvedRagIndexPath and retrieve_docs treat default as the shared docs root, so the same document was reachable with namespace omitted but not with it spelled out. Route default through the shared branch. Also drop the redundant retrieveDocsFromDB keep-alive var. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
find_suspect_fallback_edges returned an empty zero-value Result with a nil error whenever any edge endpoint node was missing, silently hiding every other suspect on the page. Skip edges with missing endpoints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
UpsertAnnotation's lookup was namespace-scoped but its create path was not, so a caller could attach an annotation to another namespace's node or to a nonexistent node id. Verify the node belongs to the caller's namespace inside the create transaction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- RebuildNodes tests staged a stale tsv via UPDATE, but the BEFORE UPDATE trigger recomputed tsv from content and erased the stale state, failing both tests against a real Postgres. Disable the trigger during staging so RebuildNodes' own SQL is what gets verified. - The rapid-push dedup test raced three Adds against an already-running worker and flaked under parallel load. Hold the single worker with a gated blocker repo so all pushes deterministically arrive while queued. - main_postgres_test referenced migrateSchemaVersion, a type renamed to migration.MigrationSchemaVersion; the postgres-tagged file no longer compiled (unnoticed because CI never builds that tag). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Delete verified-unused symbols: existingFilesMissingFrom, importPackageNames, rebuildSearch, newParsedBuildEdgeBatch (service); countNonIgnored, lintRuleDoc (cli/lint). Run gofmt -w over the 13 drifted files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mapDefTypeToNodeKind marked any declaration whose name started with the language test prefix as a test node, so a production type TestConfig or a function testimonialCard (prefix "test" + lowercase continuation) was miscategorized, which then excluded it from dead-code analysis and changed its search kind. Restrict the test check to function/method declarations and require a word boundary after a bare-word prefix (separator-terminated prefixes like "test_" already encode the boundary). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
updateGraphWithoutTx (the path taken by the MCP build_or_update_graph / parse_project tools, whose injected syncer is not transactional) passed the full existing-file set to the first normal batch. Because SyncWithExisting deletes existing files absent from the batch, a multi-record spool deleted files belonging to later batches and re-added them (churning node IDs, annotations, and stats); it also double-counted deletions against the explicit delete pass, and skipped deletions entirely when no normal batch ran. Mirror the transactional path: normal batches carry no existing files and deletions run once, unconditionally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
개요
"서비스가 무겁다"에서 출발해 (1) 주변부 기능 트림, (2) retrieval/검색 대폭 개선(검색엔진 기반, 임베딩 없음), (3) safepath 추출, (4) 전체 코드베이스 품질 리뷰 기반 수정을 담았습니다. 18 커밋, +1043/-6089.
전체 테스트 그린:
CGO_ENABLED=1 go test -tags "fts5" ./... -count=1→ 36 패키지 통과, 회귀 0. Postgres 태그 테스트는 격리 DB에서 검증(사용자 데이터 무손상).1. 트림 & 인프라
98456adbenchmark/eval 하네스 + 고아 langfuse compose 삭제 (~5.7k LOC)0c2e9a9CI에go test스텝 추가 (여태 vet+build만 돌렸음)2. retrieval/검색 개선 (검색엔진 기반)
444d0d2retrieve_docs가 매 쿼리마다 네임스페이스 전체 스캔하던 폴백 게이트 수정 → O(코드베이스)→O(매치)8390f07FTS 엔진 랭크를 버리고 수제 점수로만 정렬하던 것 → 검색엔진 히트 우선dcf4c54+61a01fepg_trgm 오타 퍼지 (인덱스 사용<%연산자, 0건일 때만 발동, search_documents 코퍼스 한정) — live PG EXPLAIN으로 Bitmap Index Scan 확인72bee1e폴백 스캔 truncation 로그 + 데드코드 정리3. safepath 추출
a610422경로안전 메커니즘(심링크 walk, canonical, containment)을internal/safepath로 단일화 + 유닛테스트 신설(기존 0). 3개 보안 정책과 에러 문자열은 호출부 유지 — 통합이 보안 동작을 바꾸지 않도록.4. 코드 품질 리뷰 수정 (4개 서브시스템 적대적 리뷰)
보안:
448485ddetect_changes등의base파라미터 git 옵션 인젝션 차단 + 비ASCII 경로(core.quotePath=false)b332f51/status에 bearer 인증 (repo명/에러문자열 무인증 노출 차단)2b2d3aewiki doc API가 CWD 임의 파일 읽던 것 → docs/ 서브트리 한정버그/계약:
3ab9d09get_doc_content의namespace:"default"→ shared docs 루트 매핑a2782efdangling fallback edge가 페이지 전체를 무음 드롭하던 것 → skip54ef7c5UpsertAnnotation create 경로 namespace 소유 검증cb7bcd8비트랜잭션 incremental update(MCP 빌드 경로) 삭제 프로토콜 3중 결함 수리836b701test prefix만으로 프로덕션 심볼(TestConfig, testimonialCard) 오분류하던 것 수정테스트/정리:
8f1ef58live-PG tsv 트리거로 깨진 RebuildNodes 2건 + 병렬 dedup 플레이크 + postgres 태그 컴파일 깨짐 수리b05b3af데드코드 6종 삭제 + gofmt 13파일미포함 / 후속
.ccg.yaml의 앱 DB가 postgres 테스트 기본 DSN(ccg_test)과 동일 — 테스트가 DROP함. 앱/테스트 DB 분리 권장.🤖 Generated with Claude Code