Skip to content

Fix review feedback: test isolation, dead code, default path alignment, doc tool name, symbol coords#128

Merged
Looted merged 2 commits intodevelopfrom
copilot/sub-pr-127
Mar 30, 2026
Merged

Fix review feedback: test isolation, dead code, default path alignment, doc tool name, symbol coords#128
Looted merged 2 commits intodevelopfrom
copilot/sub-pr-127

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 30, 2026

Addresses five review findings from the bootstrap path fix PR.

Changes

  • workspace-health.test.ts: "detects missing .kb/config.json" was only creating 5 of 7 doc dirs, so missingDocDirs was non-empty and the test wasn't strictly isolating missing-config behavior. Now creates all KIBI_DOC_DIRS entries before asserting.

  • file-filter.test.ts: Remove unused origDir variable.

  • file-filter.ts: DEFAULT_SYNC_PATHS used root-level globs (requirements/**/*.md) instead of the documentation/-prefixed paths that DEFAULT_CONFIG.paths in the CLI defines. Partial user overrides caused unoverridden keys to silently fall back to wrong root-level directories, producing false bootstrap warnings.

    - requirements: "requirements/**/*.md",
    + requirements: "documentation/requirements/**/*.md",
  • documentation/adr/ADR-008.md: Update stale `kb.symbols.refresh` reference → `kb_symbols_refresh`.

  • documentation/symbols.yaml: Correct SYM-getKbExistenceTargets coordinates: sourceLine 71→72 (actual declaration), sourceEndLine 86→102 (actual closing brace).


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

… paths, doc tool name, symbol coords

Agent-Logs-Url: https://github.com/Looted/kibi/sessions/c816f8d7-4203-4132-9532-c6d246a496bd

Co-authored-by: Looted <6255880+Looted@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix bootstrap path handling and enhance workspace health tests Fix review feedback: test isolation, dead code, default path alignment, doc tool name, symbol coords Mar 30, 2026
Copilot AI requested a review from Looted March 30, 2026 09:31
@Looted Looted marked this pull request as ready for review March 30, 2026 09:32
Copilot AI review requested due to automatic review settings March 30, 2026 09:32
@Looted Looted merged commit bb051bf into develop Mar 30, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses prior review feedback across the OpenCode plugin tests, path defaults, and Kibi documentation/manifest metadata to improve correctness and alignment with the CLI’s expected documentation layout.

Changes:

  • Fix test isolation in workspace-health.test.ts by creating all standard documentation dirs before asserting missing-config behavior.
  • Align OpenCode’s default sync/include paths in file-filter.ts to documentation/...-prefixed locations to avoid incorrect fallbacks on partial overrides.
  • Update docs/manifest metadata: correct the ADR tool name reference and refresh symbols.yaml coordinates for getKbExistenceTargets.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/opencode/tests/workspace-health.test.ts Ensures missing-config test no longer fails due to unrelated missing doc dirs.
packages/opencode/tests/file-filter.test.ts Removes unused variable to reduce dead code.
packages/opencode/src/file-filter.ts Updates default path globs to documentation/-prefixed patterns to match expected layout.
documentation/adr/ADR-008.md Updates tool name reference to kb_symbols_refresh.
documentation/symbols.yaml Corrects generated coordinate ranges for SYM-getKbExistenceTargets.

Comment on lines +26 to +27
// Local copy of DEFAULT_CONFIG.paths to avoid cross-package TS rootDir issues.
// Must stay in sync with DEFAULT_CONFIG.paths in packages/cli/src/utils/config.ts.
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

The comment says this is a local copy of DEFAULT_CONFIG.paths, but the values here are glob patterns (documentation/.../**/*.md) rather than the directory roots used by DEFAULT_CONFIG.paths in packages/cli/src/utils/config.ts. This is misleading for future maintainers and may cause the constant to drift from the intended source of truth; please clarify the comment (or reference the exact CLI constant you’re mirroring, e.g. DEFAULT_CONFIG.paths + glob normalization).

Suggested change
// Local copy of DEFAULT_CONFIG.paths to avoid cross-package TS rootDir issues.
// Must stay in sync with DEFAULT_CONFIG.paths in packages/cli/src/utils/config.ts.
// Mirror of DEFAULT_CONFIG.paths (packages/cli/src/utils/config.ts),
// with each directory root expanded to the markdown glob used for matching here.

Copilot uses AI. Check for mistakes.
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.

3 participants