Skip to content

Commit a1e4fce

Browse files
tae2089claude
andcommitted
refactor: adopt std-lib declaration ordering (cohesion + interface assertions)
Group each type with its constructor and methods as a contiguous block instead of scattering methods across the file, and place interface- satisfaction assertions (var _ Iface = (*T)(nil)) directly above the methods they assert rather than at the bottom of the file. Pure structural change: no behavior change. Verified that the non-blank line multiset of every touched .go file is identical to HEAD (reorder only), build is green, and the full fts5 test suite passes. Documents the convention in guide/development.md (Declaration order) with a pointer from CLAUDE.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent cf81348 commit a1e4fce

45 files changed

Lines changed: 1758 additions & 1722 deletions

Some content is hidden

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

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ Graceful shutdown: SIGINT/SIGTERM 시 진행 중인 clone/build에 context cance
5757
- TDD: Red → Green → Refactor
5858
- Tidy First: 구조적 변경과 행위 변경 분리
5959
- GORM 쿼리만 사용 (raw SQL 금지)
60+
- 코드 정렬: 종류별 그룹화가 아니라 "타입 + 그 타입의 생성자·메소드"를 붙여 두는 응집 관례를 따른다 (상세: `guide/development.md` §Declaration order)
6061
- 테스트: `CGO_ENABLED=1 go test -tags "fts5" ./... -count=1`
6162
- Integration test: `./scripts/integration-test.sh` (Gitea + PostgreSQL + ccg Docker 전체 파이프라인)
6263

guide/development.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,37 @@ go test ./internal/adapters/inbound/cli -run TestProjectSkills -count=1
152152
- Logging: `slog`
153153
- CLI: `cobra` framework
154154
- Build flags: `CGO_ENABLED=1 -tags "fts5"`
155+
156+
### Declaration order within a file
157+
158+
Follow the standard-library convention of **cohesion over kind-grouping**: keep a
159+
type together with everything that operates on it, rather than sorting the file
160+
into "all types, then all functions". Go does not care about declaration order at
161+
compile time, so this rule exists purely for the reader.
162+
163+
Within a file, order top-level declarations as:
164+
165+
1. Package-level `const` / `var` blocks that configure the whole file, near the top
166+
(after imports).
167+
2. For each type, a contiguous block: the `type` declaration → its interface-
168+
satisfaction assertion(s) → its `New*` constructor(s) → its methods. Do not let a
169+
free function or an unrelated type split a type's method set.
170+
3. Free helper functions after the type they support, or grouped at the end of the
171+
file if they are shared.
172+
173+
Interface-satisfaction assertions go **above** the methods, not at the bottom of the
174+
file, so the implemented contract is visible upfront:
175+
176+
- `var _ Iface = (*T)(nil)` sits immediately after the `type T` declaration.
177+
- When `T` is declared in another file of the same package (e.g. the split
178+
`graphgorm.Store`), put the assertion at the top of the file — after imports,
179+
before that file's methods on `T`.
180+
181+
One deliberate exception stays next to what it describes (this *is* the cohesion
182+
rule, not a violation of it):
183+
184+
- A package-level `var` (e.g. a compiled `regexp`) placed immediately above the
185+
single function that uses it.
186+
187+
There is no standard tool that enforces this ordering; `gofmt`/`gofumpt` handle
188+
formatting only. It is a review-time convention.

internal/adapters/inbound/mcp/evidence.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,6 @@ import (
1212
requestctx "github.com/tae2089/code-context-graph/internal/ctx"
1313
)
1414

15-
// namespaceEvidenceFromContext builds evidence metadata for namespace-scoped graph queries.
16-
// @intent include namespace path and git state when available so LLM has traceable provenance.
17-
func (h *handlers) namespaceEvidenceFromContext(ctx context.Context) namespaceEvidenceBlock {
18-
ns := requestctx.FromContext(ctx)
19-
return h.namespaceEvidence(ns)
20-
}
21-
2215
// namespaceEvidenceBlock captures stable namespace provenance fields shared across MCP responses.
2316
// @intent keep evidence payloads typed while exposing namespace and git provenance.
2417
type namespaceEvidenceBlock struct {
@@ -36,6 +29,13 @@ type namespaceGitEvidenceBlock struct {
3629
Remote string `json:"remote,omitempty"`
3730
}
3831

32+
// namespaceEvidenceFromContext builds evidence metadata for namespace-scoped graph queries.
33+
// @intent include namespace path and git state when available so LLM has traceable provenance.
34+
func (h *handlers) namespaceEvidenceFromContext(ctx context.Context) namespaceEvidenceBlock {
35+
ns := requestctx.FromContext(ctx)
36+
return h.namespaceEvidence(ns)
37+
}
38+
3939
// @intent collect namespace-scoped path and git provenance so MCP responses can explain where graph evidence came from.
4040
func (h *handlers) namespaceEvidence(namespace string) namespaceEvidenceBlock {
4141
ns := requestctx.Normalize(namespace)

internal/adapters/inbound/mcp/handler_docs.go

Lines changed: 52 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -23,58 +23,6 @@ func (h *handlers) ragIndexRoot() string {
2323
return dir
2424
}
2525

26-
// @intent normalize a docs/index root to an absolute, symlink-evaluated path before path checks.
27-
// @requires root must be a filesystem path that can be resolved or created as needed.
28-
// @ensures returned path is absolute, cleaned, and symlink-resolved when it exists.
29-
// @domainRule safe-root containment checks must happen after symlink evaluation.
30-
// @sideEffect may create the root directory on disk when create is true.
31-
func resolveSafeRoot(root string, create bool) (string, error) {
32-
absRoot, err := filepath.Abs(root)
33-
if err != nil {
34-
return "", fmt.Errorf("resolve safe root: %w", err)
35-
}
36-
if create {
37-
if err := os.MkdirAll(absRoot, 0o755); err != nil {
38-
return "", fmt.Errorf("create safe root: %w", err)
39-
}
40-
}
41-
if _, err := os.Stat(absRoot); err == nil {
42-
realRoot, err := filepath.EvalSymlinks(absRoot)
43-
if err != nil {
44-
return "", fmt.Errorf("resolve safe root symlinks: %w", err)
45-
}
46-
return filepath.Clean(realRoot), nil
47-
} else if !os.IsNotExist(err) {
48-
return "", fmt.Errorf("stat safe root: %w", err)
49-
}
50-
return filepath.Clean(absRoot), nil
51-
}
52-
53-
// @intent reject relative paths that would resolve outside the resolved docs root.
54-
// @requires relPath must be a relative, traversal-free path fragment.
55-
// @ensures returned path stays within the resolved safe root and has no symlink escape.
56-
// @domainRule traversal checks happen before symlink evaluation, and containment checks happen after it.
57-
// @sideEffect may create the configured root directory indirectly through resolveSafeRoot when createRoot is true.
58-
func safePathUnderRoot(root, relPath, field string, createRoot bool, allowMissingLeaf bool) (string, error) {
59-
clean := filepath.Clean(relPath)
60-
if filepath.IsAbs(clean) || strings.HasPrefix(clean, "..") {
61-
return "", fmt.Errorf("invalid %s: path traversal not allowed", field)
62-
}
63-
base, err := resolveSafeRoot(root, createRoot)
64-
if err != nil {
65-
return "", err
66-
}
67-
target, err := ensureNoSymlinkInPath(base, clean, allowMissingLeaf)
68-
if err != nil {
69-
return "", fmt.Errorf("resolve %s: %w", field, err)
70-
}
71-
target = filepath.Clean(target)
72-
if target != base && !strings.HasPrefix(target, base+string(os.PathSeparator)) {
73-
return "", fmt.Errorf("%s %q is outside configured safe root", field, relPath)
74-
}
75-
return target, nil
76-
}
77-
7826
// getDocContent reads a generated documentation file by relative path.
7927
// @intent Returns the content of a documentation file directly so agents can read detailed descriptions.
8028
// @param request file_path is the relative documentation path based on the working directory.
@@ -200,3 +148,55 @@ func (h *handlers) searchDocsFromDB(ctx context.Context, namespace, query string
200148
}
201149
return results, nil
202150
}
151+
152+
// @intent normalize a docs/index root to an absolute, symlink-evaluated path before path checks.
153+
// @requires root must be a filesystem path that can be resolved or created as needed.
154+
// @ensures returned path is absolute, cleaned, and symlink-resolved when it exists.
155+
// @domainRule safe-root containment checks must happen after symlink evaluation.
156+
// @sideEffect may create the root directory on disk when create is true.
157+
func resolveSafeRoot(root string, create bool) (string, error) {
158+
absRoot, err := filepath.Abs(root)
159+
if err != nil {
160+
return "", fmt.Errorf("resolve safe root: %w", err)
161+
}
162+
if create {
163+
if err := os.MkdirAll(absRoot, 0o755); err != nil {
164+
return "", fmt.Errorf("create safe root: %w", err)
165+
}
166+
}
167+
if _, err := os.Stat(absRoot); err == nil {
168+
realRoot, err := filepath.EvalSymlinks(absRoot)
169+
if err != nil {
170+
return "", fmt.Errorf("resolve safe root symlinks: %w", err)
171+
}
172+
return filepath.Clean(realRoot), nil
173+
} else if !os.IsNotExist(err) {
174+
return "", fmt.Errorf("stat safe root: %w", err)
175+
}
176+
return filepath.Clean(absRoot), nil
177+
}
178+
179+
// @intent reject relative paths that would resolve outside the resolved docs root.
180+
// @requires relPath must be a relative, traversal-free path fragment.
181+
// @ensures returned path stays within the resolved safe root and has no symlink escape.
182+
// @domainRule traversal checks happen before symlink evaluation, and containment checks happen after it.
183+
// @sideEffect may create the configured root directory indirectly through resolveSafeRoot when createRoot is true.
184+
func safePathUnderRoot(root, relPath, field string, createRoot bool, allowMissingLeaf bool) (string, error) {
185+
clean := filepath.Clean(relPath)
186+
if filepath.IsAbs(clean) || strings.HasPrefix(clean, "..") {
187+
return "", fmt.Errorf("invalid %s: path traversal not allowed", field)
188+
}
189+
base, err := resolveSafeRoot(root, createRoot)
190+
if err != nil {
191+
return "", err
192+
}
193+
target, err := ensureNoSymlinkInPath(base, clean, allowMissingLeaf)
194+
if err != nil {
195+
return "", fmt.Errorf("resolve %s: %w", field, err)
196+
}
197+
target = filepath.Clean(target)
198+
if target != base && !strings.HasPrefix(target, base+string(os.PathSeparator)) {
199+
return "", fmt.Errorf("%s %q is outside configured safe root", field, relPath)
200+
}
201+
return target, nil
202+
}

internal/adapters/inbound/mcp/handler_parse.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,6 @@ import (
1717
"github.com/tae2089/code-context-graph/internal/safepath"
1818
)
1919

20-
// @intent refresh search documents through the injected override, defaulting to the service impl.
21-
func (h *handlers) refreshSearchDocuments(ctx context.Context) (int, error) {
22-
if h.deps.Build.Maintenance == nil {
23-
return 0, nil
24-
}
25-
return h.deps.Build.Maintenance.RefreshDocuments(ctx)
26-
}
27-
2820
// @intent serialize build_or_update_graph results with a fixed JSON schema without changing the wire format.
2921
type buildOrUpdateGraphResponse struct {
3022
Status string `json:"status"`
@@ -46,6 +38,14 @@ type runPostprocessResponse struct {
4638
SkippedSteps []string `json:"skipped_steps"`
4739
}
4840

41+
// @intent refresh search documents through the injected override, defaulting to the service impl.
42+
func (h *handlers) refreshSearchDocuments(ctx context.Context) (int, error) {
43+
if h.deps.Build.Maintenance == nil {
44+
return 0, nil
45+
}
46+
return h.deps.Build.Maintenance.RefreshDocuments(ctx)
47+
}
48+
4949
// @intent apply per-request parse limits without mutating the shared handler dependency configuration.
5050
func (h *handlers) withParseLimitsFromRequest(request mcp.CallToolRequest) *handlers {
5151
maxFileBytes := int64(request.GetInt("max_file_bytes", int(h.deps.Runtime.MaxFileBytes)))
@@ -342,16 +342,6 @@ func (h *handlers) runPostprocess(ctx context.Context, request mcp.CallToolReque
342342
return mcp.NewToolResultText(jsonStr), nil
343343
}
344344

345-
// @intent append values to a slice while preserving uniqueness for skipped-step reporting.
346-
func appendUniqueStrings(dst []string, values ...string) []string {
347-
for _, value := range values {
348-
if !slices.Contains(dst, value) {
349-
dst = append(dst, value)
350-
}
351-
}
352-
return dst
353-
}
354-
355345
// @intent restrict parse and build requests to configured analysis roots before filesystem traversal begins.
356346
// @domainRule only paths contained in configured analysis roots may be parsed or rebuilt.
357347
// @ensures returned path is canonical, existing, and contained within an allowed analysis root.
@@ -376,3 +366,13 @@ func (h *handlers) validateAnalysisPath(path string) (string, error) {
376366
}
377367
return target, nil
378368
}
369+
370+
// @intent append values to a slice while preserving uniqueness for skipped-step reporting.
371+
func appendUniqueStrings(dst []string, values ...string) []string {
372+
for _, value := range values {
373+
if !slices.Contains(dst, value) {
374+
dst = append(dst, value)
375+
}
376+
}
377+
return dst
378+
}

internal/adapters/inbound/mcp/handler_query.go

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ import (
1616
"github.com/tae2089/trace"
1717
)
1818

19-
var strictFalse = false
20-
2119
const (
2220
defaultQueryGraphLimit = 50
2321
maxQueryGraphLimit = 500
@@ -119,6 +117,17 @@ type nodeResponse struct {
119117
Evidence namespaceEvidenceBlock `json:"evidence"`
120118
}
121119

120+
// listGraphStatsResponse is the serialized payload for graph statistics.
121+
// @intent preserve a stable typed JSON response for graph statistics without changing the wire format.
122+
type listGraphStatsResponse struct {
123+
TotalNodes int64 `json:"total_nodes"`
124+
TotalEdges int64 `json:"total_edges"`
125+
NodesByKind map[string]int64 `json:"nodes_by_kind"`
126+
NodesByLanguage map[string]int64 `json:"nodes_by_language"`
127+
EdgesByKind map[string]int64 `json:"edges_by_kind"`
128+
Evidence namespaceEvidenceBlock `json:"evidence"`
129+
}
130+
122131
// getNode returns detailed metadata for a graph node by qualified name.
123132
// @intent look up a node by qualified name so callers can retrieve its core identity and location metadata.
124133
// @param request qualified_name is the fully qualified node name to resolve.
@@ -229,18 +238,6 @@ func (h *handlers) search(ctx context.Context, request mcp.CallToolRequest) (*mc
229238
}))
230239
}
231240

232-
// validateQueryGraphLimit checks that the limit parameter for queryGraph is within acceptable bounds.
233-
// @intent enforce reasonable limits on queryGraph results to prevent excessive load and encourage pagination.
234-
func validateQueryGraphLimit(limit int) error {
235-
if err := validatePositiveLimit(limit); err != nil {
236-
return err
237-
}
238-
if limit > maxQueryGraphLimit {
239-
return newToolResultErr(fmt.Sprintf("limit must be <= %d, got %d", maxQueryGraphLimit, limit))
240-
}
241-
return nil
242-
}
243-
244241
// getAnnotation returns stored annotation metadata for a graph node.
245242
// @intent fetch stored annotation tags and summary data so semantic search results can show business context.
246243
// @param request qualified_name is the fully qualified node name whose annotations should be loaded.
@@ -298,6 +295,8 @@ func (h *handlers) getAnnotation(ctx context.Context, request mcp.CallToolReques
298295
}))
299296
}
300297

298+
var strictFalse = false
299+
301300
// queryGraph runs one of the predefined graph traversal patterns.
302301
// @intent expose repeated graph traversals through one pattern-driven tool entry point.
303302
// @param request pattern must be one of the allowlisted query kinds and target is a node name or file path.
@@ -545,27 +544,6 @@ func (h *handlers) callQueryPatternEdges(ctx context.Context, anchorID uint, pat
545544
return h.deps.Graph.Reader.CallEdges(ctx, anchorID, peerIDs, direction)
546545
}
547546

548-
// compactQueryTargetAmbiguity formats ambiguous query_graph matches into one compact error string.
549-
// @intent compress ambiguous short-symbol matches into one line so callers can choose the intended node.
550-
func compactQueryTargetAmbiguity(target string, matches []querypkg.CandidateMatch) string {
551-
parts := make([]string, 0, len(matches))
552-
for _, match := range matches {
553-
parts = append(parts, fmt.Sprintf("%s (%s, %s:%d)", match.QualifiedName, match.Kind, match.FilePath, match.StartLine))
554-
}
555-
return fmt.Sprintf("query_graph target %q is ambiguous: %s", target, strings.Join(parts, "; "))
556-
}
557-
558-
// listGraphStatsResponse is the serialized payload for graph statistics.
559-
// @intent preserve a stable typed JSON response for graph statistics without changing the wire format.
560-
type listGraphStatsResponse struct {
561-
TotalNodes int64 `json:"total_nodes"`
562-
TotalEdges int64 `json:"total_edges"`
563-
NodesByKind map[string]int64 `json:"nodes_by_kind"`
564-
NodesByLanguage map[string]int64 `json:"nodes_by_language"`
565-
EdgesByKind map[string]int64 `json:"edges_by_kind"`
566-
Evidence namespaceEvidenceBlock `json:"evidence"`
567-
}
568-
569547
// listGraphStats returns aggregate node and edge statistics for the graph.
570548
// @intent summarize the current graph load state with kind and language distributions.
571549
// @ensures returns total node and edge counts plus kind and language aggregates when the query succeeds.
@@ -595,3 +573,25 @@ func (h *handlers) listGraphStats(ctx context.Context, request mcp.CallToolReque
595573
return result, nil
596574
}))
597575
}
576+
577+
// validateQueryGraphLimit checks that the limit parameter for queryGraph is within acceptable bounds.
578+
// @intent enforce reasonable limits on queryGraph results to prevent excessive load and encourage pagination.
579+
func validateQueryGraphLimit(limit int) error {
580+
if err := validatePositiveLimit(limit); err != nil {
581+
return err
582+
}
583+
if limit > maxQueryGraphLimit {
584+
return newToolResultErr(fmt.Sprintf("limit must be <= %d, got %d", maxQueryGraphLimit, limit))
585+
}
586+
return nil
587+
}
588+
589+
// compactQueryTargetAmbiguity formats ambiguous query_graph matches into one compact error string.
590+
// @intent compress ambiguous short-symbol matches into one line so callers can choose the intended node.
591+
func compactQueryTargetAmbiguity(target string, matches []querypkg.CandidateMatch) string {
592+
parts := make([]string, 0, len(matches))
593+
for _, match := range matches {
594+
parts = append(parts, fmt.Sprintf("%s (%s, %s:%d)", match.QualifiedName, match.Kind, match.FilePath, match.StartLine))
595+
}
596+
return fmt.Sprintf("query_graph target %q is ambiguous: %s", target, strings.Join(parts, "; "))
597+
}

0 commit comments

Comments
 (0)