Skip to content

Implement: Repo map: ranked, read-only symbol outline (tree-sitter/LSP - #49

Draft
BabyKoan wants to merge 4 commits into
DenizOkcu:mainfrom
BabyKoan:koan/implement-22
Draft

BabyKoan wants to merge 4 commits into
DenizOkcu:mainfrom
BabyKoan:koan/implement-22

Conversation

@BabyKoan

@BabyKoan BabyKoan commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements #22

  • feat(repoMap): add LSP-first extraction, ranking, and mtime/HEAD cache
  • feat(repoMap): add regex-based symbol extraction engine

Closes #22


Quality Report

Changes: 2 files changed, 505 insertions(+)

Code scan: clean

Tests: passed (0 test)

Branch hygiene: clean

Generated by Kōan

@atoomic

atoomic commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan rebase and view failures from ci

@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-22 was rebased onto main and review feedback was applied.

Changes applied

  • Rebased koan/implement-22 onto origin/main — already up-to-date.
  • Fixed CI typecheck failure in src/llm/repoMap.ts:129:
  • lspWorkspaceSymbols can return entries with path: undefined.
  • Skipped those rows before pushing into RepoMapSymbol[].
  • Verified npm run typecheck passes.
  • No other actionable review comments to apply.

Stats

2 files changed, 506 insertions(+)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=No repoMap.ts, ranked symbol outline, regex fallback engine, or mtime/HEAD cache exists on main; onl)
  • Rebased koan/implement-22 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run #28289314159 failed
  • Pre-push CI fix: no changes needed or Claude found nothing to fix
  • Force-pushed koan/implement-22 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

BabyKoan added a commit to BabyKoan/haze that referenced this pull request Jun 29, 2026
@BabyKoan

BabyKoan commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Previous review — superseded by a newer review below.

@atoomic

atoomic commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan rebase

@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-22 was rebased onto main and review feedback was applied.

Changes applied

  • Done. Applied review feedback in src/llm/repoMap.ts.
  • LSP failures: Log each server error in extractSymbolsViaLsp and corrected the comment to match behavior — failures fall through to the next server; regex is the final fallback.
  • File read errors: Replaced silent empty-string substitution with try/catch + console.error when reading files for regex extraction and when back-filling LSP symbol contents.
  • Cache corruption: loadCache now returns {} only for missing cache (ENOENT); parse/read errors are logged and re-thrown.
  • Git HEAD failures: gitHead now returns null on failure instead of an empty string, so a missing HEAD busts the cache instead of matching a stale empty key. Updated RepoMapCacheEntry.head to string | null.
  • Recent-files failures: recentlyTouchedFiles now logs the git error before returning an empty set.

Stats

2 files changed, 521 insertions(+)
Actions performed
  • Already-solved check: skipped (Claude call failed)
  • Rebased koan/implement-22 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-22 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@BabyKoan
BabyKoan force-pushed the koan/implement-22 branch from eaa25e8 to b5c3a54 Compare June 30, 2026 01:46
@BabyKoan

Copy link
Copy Markdown
Contributor Author

PR Review — Implement: Repo map: ranked, read-only symbol outline (tree-sitter/LSP

Solid extraction primitives, but the feature is still a disconnected library with several regex-fallback correctness gaps.

The module is cleanly structured: LSP-first dispatch, pure ranking/reference-count functions, and a simple mtime/HEAD cache are good architectural choices. Prior review feedback has been addressed — LSP failures, file-read errors, cache corruption, and git-HEAD handling now log or fail loudly rather than silently swallowing problems.

What still needs attention before merge:

  • repoMap.ts has no callers, so the PR does not yet deliver a usable feature.
  • The method regex produces false symbols for control-flow keywords (if, while, for, etc.).
  • Block comments are not stripped, so commented-out declarations become symbols.
  • The cache grows forever and lacks a format-version safeguard.
  • options.path causes reference counts to read the wrong files.
  • The regex pass walks directories and every file type, causing spurious read errors and wasted I/O.
  • Tests mutate process.env and process.cwd() globally, risking isolation problems.

🟡 Important

1. Repo map module has no product integration

buildRepoMap and its helpers are exported, but nothing in the repository imports or calls them. I searched the full tree for repoMap, buildRepoMap, RepoMap, and the exported function names and found only this file and its test.

As written, the feature is a library with no agent tool, system prompt, CLI command, or callback wiring. Unless issue #22 explicitly scoped this to extraction primitives, the PR does not deliver an end-to-end repo map capability. Add an integration point or document that wiring is intentionally deferred.

export async function buildRepoMap(options: RepoMapOptions = {}): Promise<RepoMapResult> {
  ...
}
2. Method regex matches control-flow keywords
src/llm/repoMap.ts:32-39

Every modifier in the method pattern is optional, so it matches statements such as if (x) {, while (true) {, for (const x of y) {, and switch (v) {. Those produce symbols named if, while, for, and switch.

Because computeReferenceCounts then counts every identifier occurrence, these keywords rank artificially high and pollute the repo map. Require at least one class-member modifier, or drop unmodified method detection from the regex fallback.

{kind: 'method', regex: /^\s*(?:private\s+|public\s+|protected\s+|static\s+|async\s+)*([A-Za-z_$][\w$]*)\s*\([^)]*\)\s*(?:\{|:)/}
3. Block comments are not stripped before matching
src/llm/repoMap.ts:66-68

isCommentLine only checks for // single-line comments. A line such as export class Commented {} that sits inside a /* ... */ block comment is treated as active code and Commented is extracted as a symbol.

This causes commented-out declarations to appear in the repo map. Strip block-comment regions before running the declaration regexes, or track an in-block state while scanning lines.

function isCommentLine(line: string): boolean {
  return /^\s*\/\//.test(line);
}
4. Cache never evicts deleted files and has no format version
src/llm/repoMap.ts:150-154

loadCache reads the whole cache object, buildRepoMap mutates entries for files it sees, and saveCache writes the object back. Two problems follow:

  • Stale entries accumulate forever. If a file is deleted or renamed, its cache entry is never removed, so the cache grows unbounded and may resurrect symbols for files that no longer exist.
  • No cache format version. If the extraction logic or RepoMapSymbol shape changes, old cache entries are reused verbatim without migration.

Add a version field and drop entries whose version mismatches; prune keys that are not present in the walked file set before saving.

async function loadCache(): Promise<Record<string, RepoMapCacheEntry>> {
  try {
    const raw = await fs.readFile(CACHE_FILE, 'utf8');
    return JSON.parse(raw) as Record<string, RepoMapCacheEntry>;
  } catch (error) {
    if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
      return {};
    }
    console.error('Repo map cache is corrupt or unreadable:', error);
    throw error;
  }
}
5. options.path breaks reference-count file resolution
src/llm/repoMap.ts:246-292

When options.path is provided, scopeRoot becomes that subdirectory and walkDir returns paths relative to scopeRoot. Later, reference counting joins those relative paths against workspaceRoot():

const absolutePath = path.join(workspaceRoot(), symbol.path);

If options.path is packages/foo, a symbol for packages/foo/src/bar.ts is stored as src/bar.ts, then read as <workspaceRoot>/src/bar.ts instead of <workspaceRoot>/packages/foo/src/bar.ts. The reference counts will be wrong or zero for any scoped repo map.

Fix by storing workspace-relative paths consistently, or by remembering scopeRoot and joining against it when back-filling file contents.

const scopeRoot = options.path ? resolveWorkspacePath(options.path) : workspaceRoot();
...
const absolutePath = path.join(workspaceRoot(), symbol.path);
6. Regex fallback walks directories and every file type
src/llm/repoMap.ts:262-285

walkDir returns both files and directories, but the loop does not skip directories before calling fs.readFile. That causes unnecessary Failed to read ... for repo map errors for every directory in the tree.

It also reads every non-ignored file as UTF-8. Binary files, JSON, YAML, Markdown, etc. are scanned, wasting I/O and risking accidental "symbols" from matching text. Filter to entry.isFile and restrict the regex pass to source extensions matching the configured LSP servers (e.g. .ts, .tsx, .js, .jsx, .py, .go, .rs).

const entries = await walkDir(scopeRoot, {
  recursive: true,
  filter: async entry => !await isGitIgnored(entry.absolutePath),
});

for (const entry of entries) {
  const cached = cache[entry.path];
  const stat = await fs.stat(entry.absolutePath);
  ...
  const content = await fs.readFile(entry.absolutePath, 'utf8');
7. Tests mutate global env and current working directory
tests/llm/repoMap.test.ts:6-179

The test file sets process.env.HAZE_REPO_MAP_CACHE at module load time and mutates process.cwd() via process.chdir(tmp) in beforeEach. Vitest runs files concurrently by default, so changing the process-wide current working directory and environment can leak to other tests and cause flaky failures.

Move cache-path setup into beforeAll/afterAll and reset the env var in afterAll. Better yet, refactor buildRepoMap to accept an explicit root parameter (or inject workspaceRoot()) so scoped behavior can be validated without process.chdir.

const testCacheDir = await fs.mkdtemp(path.join(os.tmpdir(), 'haze-repomap-cache-'));
process.env.HAZE_REPO_MAP_CACHE = path.join(testCacheDir, 'repo-map-cache.json');
...
beforeEach(async () => {
  tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'haze-repomap-'));
  originalCwd = process.cwd();
  process.chdir(tmp);
  ...
})

🟢 Suggestions

1. LSP results from multiple servers may contain duplicates
src/llm/repoMap.ts:112-144

extractSymbolsViaLsp concatenates workspace-symbol results from every installed server. If a workspace has overlapping LSP configurations, the same symbol can appear multiple times and receive inflated reference counts. Consider deduplicating by (name, kind, path, line) before returning.

for (const server of servers) {
  try {
    const values = await lspWorkspaceSymbols(server, query, limit);
    for (const value of values) {
      if (!value.path) continue;
      results.push({...});
    }
  } catch (error) {
    console.warn(`LSP workspace-symbol query failed for ${server.name}:`, error);
  }
}

Checklist

  • No hardcoded secrets or injection vectors
  • Input validation at system boundaries
  • Error handling: failures logged or rethrown rather than swallowed
  • Performance: no unbounded I/O or unbounded collections — warning #4, warning #6
  • Testing: good coverage and isolated tests — warning #7
  • Production readiness: integration, migration, and caller impact — warning #1, warning #4, warning #5
  • Architecture: appropriate coupling and abstraction — warning #1

To rebase specific severity levels, mention me: @BabyKoan rebase critical (fixes 🔴 only), @BabyKoan rebase important (fixes 🔴 + 🟡), or just @BabyKoan rebase for all.


Automated review by Kōan (Claude) HEAD=b5c3a54 9 min 55s

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.

Repo map: ranked, read-only symbol outline (tree-sitter/LSP) for whole-codebase awareness

2 participants