-
Notifications
You must be signed in to change notification settings - Fork 0
Document default data locations in Getting Started #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document default data locations in Getting Started #51
Conversation
Summary of ChangesHello @doronpers, 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 enhances the Highlights
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
|
🔄 Feedback Loop AnalysisMetrics Analysis Report — AllGenerated: 2026-01-16T22:41:40.516202 Summary
High Frequency Patterns (0)Patterns Ranked by Severity (0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a helpful 'Data Locations' section to the Getting Started guide to document the default paths for learning artifacts. This is a great addition for user clarity. My review focuses on ensuring the accuracy of this new documentation and improving its clarity. I've identified a potential inconsistency in the default file paths between what's documented and what's defined in some core modules. I've also suggested a minor wording improvement to make the instructions about overriding paths clearer for users.
| - **Metrics** (test runs): `data/metrics_data.json` | ||
| - **Patterns** (learned library): `data/patterns.json` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default paths data/metrics_data.json and data/patterns.json appear to be inconsistent with the defaults defined in some core components. For instance, metrics.pattern_manager.PatternManager defaults to patterns.json and metrics.sync_client.LocalSyncClient defaults to metrics_data.json, both without the data/ prefix. This discrepancy could cause files to be created in the project root instead of the data/ directory, leading to user confusion. To ensure consistency, it would be best to either align the documentation with the actual behavior or update the code to match these documented paths.
| - **Patterns** (learned library): `data/patterns.json` | ||
| - **Optional markdown sync** (pattern guide): `docs/AI_PATTERNS_GUIDE.md` | ||
|
|
||
| If you change paths via CLI flags, keep these files together so the metrics and pattern sync stay aligned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrase 'keep these files together' is a bit ambiguous. To improve clarity for the user, I suggest rephrasing it to be more specific about the location.
| If you change paths via CLI flags, keep these files together so the metrics and pattern sync stay aligned. | |
| If you change paths via CLI flags, it is recommended to keep these files in the same directory so the metrics and pattern sync stay aligned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 712fbf6ee8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - **Metrics** (test runs): `data/metrics_data.json` | ||
| - **Patterns** (learned library): `data/patterns.json` | ||
| - **Optional markdown sync** (pattern guide): `docs/AI_PATTERNS_GUIDE.md` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Point readers at the real AI patterns guide path
This line says the default markdown sync target is docs/AI_PATTERNS_GUIDE.md, but the repo’s canonical guide lives under documentation/AI_PATTERNS_GUIDE.md (README.md 94-99). A new user following Getting Started will look for or sync to a different path than the rest of the documentation uses, which risks creating a separate, out‑of‑sync guide. Consider aligning this path with the documented guide location or clarifying that it’s a separate sync output.
Useful? React with 👍 / 👎.
There was a problem hiding this 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 adds a "Data Locations" section to the Getting Started guide to document the default paths where feedback-loop stores learning artifacts, helping users understand where their data lives and how to maintain alignment when overriding paths.
Changes:
- Added "Data Locations" section documenting default paths for metrics, patterns, and optional markdown sync files
- Included guidance about keeping files aligned when using custom CLI flag paths
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - **Metrics** (test runs): `data/metrics_data.json` | ||
| - **Patterns** (learned library): `data/patterns.json` | ||
| - **Optional markdown sync** (pattern guide): `docs/AI_PATTERNS_GUIDE.md` |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documented path for the optional markdown sync is docs/AI_PATTERNS_GUIDE.md, which matches the default path used in the code (e.g., in metrics/integrate.py line 69). However, the actual file currently exists at documentation/AI_PATTERNS_GUIDE.md. This creates a discrepancy where the code's default path points to a location where the file doesn't exist. Consider either updating the code's default path to match the actual file location, or clarifying in the documentation that users may need to adjust this path or create a symlink.
| - **Optional markdown sync** (pattern guide): `docs/AI_PATTERNS_GUIDE.md` | |
| - **Optional markdown sync** (pattern guide): `docs/AI_PATTERNS_GUIDE.md` (default code path; the file currently lives at `documentation/AI_PATTERNS_GUIDE.md`, so either adjust the path via CLI flags or create a symlink) |
Motivation
Description
Data Locationssection todocumentation/GETTING_STARTED.mdthat lists the defaults:data/metrics_data.json,data/patterns.json, anddocs/AI_PATTERNS_GUIDE.md.Testing
Codex Task