Skip to content

Trim peripheral features, improve search, and fix review findings - #1

Merged
tae2089 merged 18 commits into
mainfrom
refactor/trim-peripheral-features
Jul 10, 2026
Merged

Trim peripheral features, improve search, and fix review findings#1
tae2089 merged 18 commits into
mainfrom
refactor/trim-peripheral-features

Conversation

@tae2089

@tae2089 tae2089 commented Jul 10, 2026

Copy link
Copy Markdown
Owner

개요

"서비스가 무겁다"에서 출발해 (1) 주변부 기능 트림, (2) retrieval/검색 대폭 개선(검색엔진 기반, 임베딩 없음), (3) safepath 추출, (4) 전체 코드베이스 품질 리뷰 기반 수정을 담았습니다. 18 커밋, +1043/-6089.

전체 테스트 그린: CGO_ENABLED=1 go test -tags "fts5" ./... -count=1 → 36 패키지 통과, 회귀 0. Postgres 태그 테스트는 격리 DB에서 검증(사용자 데이터 무손상).

1. 트림 & 인프라

  • 98456ad benchmark/eval 하네스 + 고아 langfuse compose 삭제 (~5.7k LOC)
  • 0c2e9a9 CI에 go test 스텝 추가 (여태 vet+build만 돌렸음)

2. retrieval/검색 개선 (검색엔진 기반)

  • 444d0d2 retrieve_docs가 매 쿼리마다 네임스페이스 전체 스캔하던 폴백 게이트 수정 → O(코드베이스)→O(매치)
  • 8390f07 FTS 엔진 랭크를 버리고 수제 점수로만 정렬하던 것 → 검색엔진 히트 우선
  • dcf4c54+61a01fe pg_trgm 오타 퍼지 (인덱스 사용 <% 연산자, 0건일 때만 발동, search_documents 코퍼스 한정) — live PG EXPLAIN으로 Bitmap Index Scan 확인
  • 72bee1e 폴백 스캔 truncation 로그 + 데드코드 정리

3. safepath 추출

  • a610422 경로안전 메커니즘(심링크 walk, canonical, containment)을 internal/safepath로 단일화 + 유닛테스트 신설(기존 0). 3개 보안 정책과 에러 문자열은 호출부 유지 — 통합이 보안 동작을 바꾸지 않도록.

4. 코드 품질 리뷰 수정 (4개 서브시스템 적대적 리뷰)

보안:

  • 448485d detect_changes 등의 base 파라미터 git 옵션 인젝션 차단 + 비ASCII 경로(core.quotePath=false)
  • b332f51 /status에 bearer 인증 (repo명/에러문자열 무인증 노출 차단)
  • 2b2d3ae wiki doc API가 CWD 임의 파일 읽던 것 → docs/ 서브트리 한정

버그/계약:

  • 3ab9d09 get_doc_content의 namespace:"default" → shared docs 루트 매핑
  • a2782ef dangling fallback edge가 페이지 전체를 무음 드롭하던 것 → skip
  • 54ef7c5 UpsertAnnotation create 경로 namespace 소유 검증
  • cb7bcd8 비트랜잭션 incremental update(MCP 빌드 경로) 삭제 프로토콜 3중 결함 수리
  • 836b701 test prefix만으로 프로덕션 심볼(TestConfig, testimonialCard) 오분류하던 것 수정

테스트/정리:

  • 8f1ef58 live-PG tsv 트리거로 깨진 RebuildNodes 2건 + 병렬 dedup 플레이크 + postgres 태그 컴파일 깨짐 수리
  • b05b3af 데드코드 6종 삭제 + gofmt 13파일

미포함 / 후속

  • webhook insecure 모드 SSRF(서명생략+임의 clone URL): insecure의 로컬경로 clone이 의도된 기능(e2e 의존)이라 코드 변경 보류. loopback 개발 전용임을 문서화 권장.
  • indexer 스풀 메모리 바운딩, community N+1, edgeresolve 에러 무음 등은 백로그.
  • 주의: .ccg.yaml의 앱 DB가 postgres 테스트 기본 DSN(ccg_test)과 동일 — 테스트가 DROP함. 앱/테스트 DB 분리 권장.

🤖 Generated with Claude Code

tae2089 and others added 18 commits July 10, 2026 21:10
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>
@tae2089
tae2089 merged commit 10008ff into main Jul 10, 2026
1 check failed
@tae2089
tae2089 deleted the refactor/trim-peripheral-features branch July 10, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant