feat: pre-fetch doc-drift in sync-docs command#10
Conversation
Prompt user when repo-intel.json is missing instead of silently skipping doc-drift detection.
Queries doc-drift before spawning sync-docs-agent, passes stale doc paths directly in the agent prompt.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the documentation synchronization process by integrating Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a pre-fetch mechanism for 'doc-drift' data, which is a great enhancement for the sync-docs command. The changes look good overall. I've added a few comments with suggestions to improve maintainability by reducing code duplication and magic numbers, and to enhance debuggability by adding logging to error-handling blocks that currently fail silently. These changes should make the code more robust and easier to manage in the long run.
| const fs = require('fs'); | ||
| const path = require('path'); | ||
| const cwd = process.cwd(); | ||
| const stateDir = ['.claude', '.opencode', '.codex'].find(d => fs.existsSync(path.join(cwd, d))) || '.claude'; |
There was a problem hiding this comment.
This logic for determining stateDir is duplicated in skills/sync-docs/SKILL.md. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider centralizing this logic. The @agentsys/lib library appears to provide xplat.getStateDir() which could potentially be used here and in other places where this logic is repeated.
| const mapFile = path.join(cwd, stateDir, 'repo-intel.json'); | ||
|
|
||
| if (fs.existsSync(mapFile)) { | ||
| const docDrift = JSON.parse(binary.runAnalyzer(['repo-intel', 'query', 'doc-drift', '--top', '20', '--map-file', mapFile, cwd])); |
There was a problem hiding this comment.
The value '20' is used here as a magic number. For better readability and maintainability, it's recommended to define it as a constant at the top of the script block, e.g., const TOP_N_DOC_DRIFT = '20';, and use this constant here.
| repoIntelContext += '\nFull doc-drift data: ' + JSON.stringify(docDrift); | ||
| } | ||
| } | ||
| } catch (e) { /* unavailable */ } |
There was a problem hiding this comment.
Silently catching and ignoring errors can hide issues and make debugging difficult. While this feature might be opportunistic, logging the error (e.g., using console.warn) would provide valuable diagnostic information without interrupting the application's flow.
| } catch (e) { /* unavailable */ } | |
| } catch (e) { console.warn('Failed to pre-fetch repo-intel doc-drift data:', e); } |
| const stateDir = ['.claude', '.opencode', '.codex'] | ||
| .find(d => fs.existsSync(path.join(cwd, d))) || '.claude'; |
There was a problem hiding this comment.
This logic for determining stateDir is duplicated in commands/sync-docs.md. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this logic should be centralized. The @agentsys/lib library appears to provide xplat.getStateDir() which could potentially be used. Using a shared utility would require some refactoring here, as the require for @agentsys/lib is currently inside a conditional block.
| } catch (e) { | ||
| // Binary not available - proceed without | ||
| } |
There was a problem hiding this comment.
Silently catching and ignoring errors can hide issues and make debugging difficult. Even if the binary is unavailable, logging the error (e.g., using console.warn) would provide valuable diagnostic information for developers without interrupting the user's flow.
| } catch (e) { | |
| // Binary not available - proceed without | |
| } | |
| } catch (e) { | |
| console.warn('Failed to generate repo-intel map:', e); | |
| // Binary not available or failed to run - proceed without | |
| } |
1 participant
Pre-fetch doc-drift before spawning sync-docs-agent.