Skip to content

fix(analysis): repair host discovery and evidence coverage - #213

Open
Dzuhaib wants to merge 1 commit into
QoderAI:mainfrom
Dzuhaib:fix/analysis-host-discovery-evidence-coverage
Open

Dzuhaib wants to merge 1 commit into
QoderAI:mainfrom
Dzuhaib:fix/analysis-host-discovery-evidence-coverage

Conversation

@Dzuhaib

@Dzuhaib Dzuhaib commented Oct 24, 2026

Copy link
Copy Markdown

Summary

  • Consolidate Qoder asset and session home resolution, document --qoder-home, and prefer an existing ~/.qoder-cn before ~/.qoder.
  • Fetch 30/90/180-day history windows independently, expose commit limits, truncation, and coverage, and downgrade the review matrix when any window is incomplete.
  • Recognize root and nested AGENTS.md and CLAUDE.md carriers, exclude scratch directories from suggested scopes, discover tracked npm and Just validation gates, and include root AGENTS.md in Claude Rules.
  • Add a real lead/session binding regression while preserving the conflict path.
  • Make Windows fixtures portable with directory junctions, isolated DSH home state, and clean child-process shutdown.

Spec and stories

docs/specs/2026-10-24-harness-analysis-defects.md (Stories #212, #195, #164).

Validation

  • npm test: 1765 passed, 30 skipped, 0 failed
  • npm run harness-studio-apps:test: 3 files passed

Maintainers are allowed to edit this PR branch.

Consolidate Qoder asset and session home resolution, document --qoder-home, and prefer an existing ~/.qoder-cn before ~/.qoder. Fetch 30/90/180-day history windows independently, expose commit limits, truncation, and coverage, and downgrade the review matrix when any window is incomplete.

Recognize root and nested AGENTS.md and CLAUDE.md carriers, exclude scratch directories from suggested scopes, discover tracked npm and Just validation gates, and include root AGENTS.md in Claude Rules. Add a real lead/session binding regression while preserving the conflict path, and make Windows fixtures portable with directory junctions, isolated DSH home state, and clean child-process shutdown.

Spec: docs/specs/2026-10-24-harness-analysis-defects.md (Stories QoderAI#212, QoderAI#195, QoderAI#164). Test: npm test (1765 passed, 30 skipped); npm run harness-studio-apps:test (3 files passed).

Co-authored-by: opencode (DeepSeek V4.1 Flash) <noreply@opencode.ai>

@sorrycc sorrycc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Moving the Qoder home logic into one module and fetching each history window separately both fix real problems. I checked out the branch, read the diff and ran npm test (results at the end). Some changes are needed before merge, mainly items 1–3.

Must fix

1. resolveQoderSessionHome now uses QODER_HOME even when it points at the shared client cache (scripts/host-support/qoder-home.mjs)

Before this PR, the session analyzer ignored QODER_HOME and used options.home ?? options["qoder-home"] ?? "~/.qoder". Now it uses QODER_HOME without checking it. In the same file, resolveQoderSharedClientCacheRoot treats a QODER_HOME whose basename is not .qoder/qoder as the SharedClientCache root. A user with QODER_HOME=~/Library/Application Support/Qoder/SharedClientCache will have session analysis read transcripts from the cache directory and quietly find zero sessions. That is the same kind of failure #212 is about. Use the same check as the asset resolver:

if (environmentHome && looksLikeQoderAssetHome(environmentHome)) return path.resolve(expandHome(environmentHome));

and add a test with a non-asset QODER_HOME.

2. A root AGENTS.md and a root CLAUDE.md are counted twice (project-profile.mjs agentInstructionFiles)

Many repos ship both at the root, often with CLAUDE.md being @AGENTS.md or a symlink. Both now become separate instruction files with the same scope. count/rootCount double, and because status compares files.length to suggestedMinimum, a repo can move from thin to adequate when nothing new covers it. The same happens for every nested directory that has both files. Suggestion: dedupe by scope for count/rootCount/nestedCount/status, and keep the per-file list and carriers breakdown for reporting.

3. The consolidation is incomplete; three ~/.qoder fallbacks are still hardcoded

The PR says it consolidates Qoder home resolution, but these still default to ~/.qoder, so a CN-only install still misses them:

  • scripts/coding-agent-practices/checkup/sources.mjs:34
  • scripts/harness-component-snapshot/snapshot.mjs:113
  • scripts/harness-analysis/validate-canvas.mjs:633 (it also ignores the asset-home basename check on QODER_HOME)

Please route them through resolveQoderAssetHome, or say in the PR why they are excluded. Also, profiles/qoder.mjs still has fallbackLabel: "~/.qoder", so redacted evidence will say ~/.qoder even when ~/.qoder-cn was read.

Should fix

4. Each window now reruns git log --numstat

historyWindows now runs one full git log -N --numstat per window, so 4 log walks instead of 1, each with a 30s timeout. On this repo, analyzeGitHistoryProfile went from 483 ms to 1251 ms (about 2.6×), and the cost grows with repo size. Two cheaper options:

  • Reuse the main scan when it already covers the window. If the oldest commit in commits is older than sinceSeconds, the window is complete, so filter in memory as before and only query git for windows the main scan didn't reach.
  • Or run one git log --since=<largest window> and split the results in memory.

5. truncated is set when a window has exactly maxCommits commits

windowParsed.length >= maxCommits marks a window with exactly N commits as truncated. Request -${maxCommits + 1} and check > maxCommits (then drop the extra commit) to make it exact.

6. With the default limit, active repos always get review

With DEFAULT_MAX_COMMITS = 500, this repo's 90- and 180-day windows both come back truncated (618 commits in 90 days), so history-window-coverage will never be ready for a moderately active project. That is defensible, but the matrix note should say how to fix it (raise --max-commits). Otherwise users see a permanent review item with no next step. Also, the confidence !== "low" condition is new and not part of the spec's "downgrade when any window is incomplete". Please mention it in the spec or drop it.

7. PROJECT_GATE_PATTERN accepts watch and dev variants

(?::.+)? matches test:watch, build:watch, test:ui, e2e:headed and similar. These are now presented to agents as validation gate commands, and an agent that runs npm run test:watch will hang. Please exclude :watch|:dev|:ui|:headed|:open suffixes, or only allow suffixes known to finish.

Nits / questions

  • Claude Rules now includes root AGENTS.md as agents-md-compat. If Claude Code only reads it through an @AGENTS.md import in CLAUDE.md, showing it as a Claude rule overstates what Claude loads. Can you link the host behavior this relies on? Also, the visited set compares unresolved paths, so CLAUDE.md -> AGENTS.md symlinks produce two items. Consider realpath.
  • defaultQoderHome() picks ~/.qoder-cn whenever it exists. A user who has both dirs (e.g. tried CN once, mainly uses international) now silently loses ~/.qoder. Consider preferring the home with session data, or warning when both exist.
  • qoder-home.mjs has its own copy of expandHome instead of importing session-analysis/paths.mjs. If that's to avoid an import cycle, a one-line comment would help.
  • SCRATCH_DIRECTORIES stops scratch dirs from being suggested as scopes, but their files still count toward totals.sourceFiles and therefore suggestedMinimum. Is that intended?

Tests

npm test on 9319fce (macOS, Node 26): 119 files passed, 1 skipped; 1788 tests passed, 7 skipped, 0 failed. The new tests cover the happy paths well. Tests are still missing for items 1, 2 and 7 (non-asset QODER_HOME in session analysis, root AGENTS.md + CLAUDE.md together, test:watch script).

Bundle size: the shipped code shrinks slightly (about 50 duplicated lines in providers/qoder.mjs replace one 65-line shared module). No new dependencies.

Requesting changes for 1–3. 4–7 can be follow-ups if you'd rather keep this PR focused.

This branch has not been deployed

No deployments
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.

2 participants