Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the configuration system and restructures documentation for better maintainability and clarity. The changes include consolidating testing guides, moving CLI documentation to a more prominent location, updating diagram syntax to plain ASCII (from Unicode box characters), and aligning development workflow documentation with the current Makefile targets and jan-cli commands.
Key Changes
- Configuration System: Created new centralized configuration documentation in
docs/configuration/with comprehensive guides for precedence, environment variable mapping, Docker Compose integration, and Kubernetes Helm values generation - Documentation Consolidation: Merged multiple testing documents into single comprehensive guides, moved CLI documentation to main docs directory, and updated IDE integration guides to reference current tooling
- Command Reference Updates: Replaced deprecated commands with current
jan-clicommands andmaketargets throughout all documentation - Convention Guides: Added comprehensive convention documents covering architecture patterns, design patterns, workflow, and quick reference conventions for developers
Reviewed Changes
Copilot reviewed 83 out of 172 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/guides/services-template.md | Updated scaffold command from PowerShell script to jan-cli dev scaffold |
| docs/guides/monitoring.md | Changed ASCII art box diagrams from Unicode to plain ASCII, updated make monitor-down-v to make monitor-clean |
| docs/guides/mcp-testing.md | Major refactor to reflect Kong integration, replaced direct service URLs with Kong-proxied paths, updated health check procedures |
| docs/guides/kong-plugins.md | Changed tree diagram from Unicode to ASCII format |
| docs/guides/jan-cli.md | New comprehensive CLI guide covering installation, commands, configuration management, service operations, development tools |
| docs/guides/ide/vscode.md | Updated task/launch references from hybrid-mode commands to dev-full commands, replaced emoji with text labels |
| docs/guides/ide/tasks.json | Simplified VS Code tasks to use Makefile targets instead of raw Docker commands |
| docs/guides/ide/launch.json | Updated preLaunchTask from "Start All Services" to "Start Dev-Full Stack" |
| docs/guides/ide/README.md | Updated quick setup to create .vscode directory and copy files properly |
| docs/guides/hybrid-mode.md | Complete rewrite focusing on dev-full workflow with jan-cli dev run commands |
| docs/guides/documentation-cleanup.md | New document summarizing documentation consolidation effort |
| docs/guides/development.md | Streamlined to focus on current workflow, removed outdated hybrid-mode references, updated to reflect dev-full mode |
| docs/guides/dev-full-mode.md | New focused guide on dev-full hybrid debugging workflow |
| docs/guides/deployment.md | Updated Docker Compose commands to reflect new profile-based targets |
| docs/guides/authentication.md | Changed arrow symbols from Unicode to ASCII |
| docs/guides/README.md | Updated common tasks to reflect dev-full and jan-cli workflows |
| docs/getting-started/README.md | Added make quickstart wizard documentation, updated service table, revised troubleshooting |
| docs/conventions/workflow.md | New workflow conventions document for daily development practices |
| docs/conventions/design-patterns.md | New comprehensive code patterns guide (moved from conventions-patterns.md) |
| docs/conventions/conventions.md | New quick reference conventions guide |
| docs/conventions/conventions-*.md | Removed old convention files (consolidated into new structure) |
| docs/conventions/architecture-patterns.md | New architecture and structure conventions (moved from conventions-architecture.md) |
| docs/configuration/service-migration.md | New service migration strategy document |
| docs/configuration/precedence.md | New configuration precedence documentation |
| docs/configuration/kubernetes.md | New Kubernetes Helm values generation guide |
| docs/configuration/env-var-mapping.md | New environment variable mapping reference |
| docs/configuration/docker-compose.md | New Docker Compose generation documentation |
| docs/configuration/README.md | New configuration system overview and user guide |
| docs/architecture/test-flows.md | Updated diagram syntax from Unicode to ASCII, added test-all.postman.json reference |
| docs/architecture/data-flow.md | Minor update about streaming support |
| docs/api/mcp-tools/integration.md | Removed outdated mcp-go integration notes |
| config/schema/monitoring.schema.json | Generated monitoring schema JSON |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### Stop Monitoring Stack | ||
| ```bash | ||
| # Stop but keep data | ||
| make monitor-down | ||
|
|
||
| # Stop and remove all data volumes (fresh start) | ||
| make monitor-down-v | ||
| make monitor-clean |
There was a problem hiding this comment.
The command reference changed from make monitor-down-v to make monitor-clean. Ensure that the Makefile contains this target and that it performs the expected cleanup (removing volumes).
| make newman-mcp | ||
| make health-check # full stack health summary | ||
| curl http://localhost:8091/healthz | ||
| curl http://localhost:3015/healthz || true # returns 404 because the vector store uses custom routes |
There was a problem hiding this comment.
The comment explains why the health check returns 404. Consider documenting the actual health check endpoint for the vector store if one exists, or note that health checks should be performed via a different method.
Suggested change
| curl http://localhost:3015/healthz || true # returns 404 because the vector store uses custom routes | |
| curl http://localhost:3015/healthz || true # returns 404: the vector store does not expose a /healthz endpoint. To check if the vector store is running, verify the service is up via Docker or check its logs. (No HTTP health check endpoint is available.) |
|
|
||
| **Disadvantages:** | ||
| - Must be run from project root | ||
| - Requires file extension (.sh or.ps1) |
There was a problem hiding this comment.
Missing space between 'or' and '.ps1'. Should be 'or .ps1'.
Suggested change
| - Requires file extension (.sh or.ps1) | |
| - Requires file extension (.sh or .ps1) |
| jan-cli config export --format env | ||
|
|
||
| # Export as Docker env file | ||
| jan-cli config export --format docker-env --output.env |
There was a problem hiding this comment.
Missing space between '--output' and '.env'. Should be '--output .env'.
Suggested change
| jan-cli config export --format docker-env --output.env | |
| jan-cli config export --format docker-env --output .env |
| ### Prerequisites | ||
| - **Docker Desktop 24+** with Docker Compose V2 | ||
| - **GNU Make** (built in on macOS/Linux, install via Chocolatey/Brew on Windows) | ||
| - **Go 1.21+** � only required when editing Go code or using `jan-cli` |
There was a problem hiding this comment.
Invalid character '�' appears before 'only'. Should be a dash or appropriate separator.
Suggested change
| - **Go 1.21+** � only required when editing Go code or using `jan-cli` | |
| - **Go 1.21+** — only required when editing Go code or using `jan-cli` |
| export POSTGRES_PORT=5432 | ||
|
|
||
| # Run service | ||
| go run./cmd/server |
There was a problem hiding this comment.
Missing space between 'run' and './cmd/server'. Should be 'go run ./cmd/server'.
Suggested change
| go run./cmd/server | |
| go run ./cmd/server |
- Improved drift_test.go to properly detect actual changes vs whitespace - Added detailed diff output when configuration drift is detected - Fixed redundant newline in fmt.Println (linting error) - Updated audience field from 'account' to 'jan-client' in defaults.yaml - Fixed vllm port from 8001 to 8101 in cmd/jan-cli/config/defaults.yaml - Removed feat/v2-config-refactor from workflow push branches Changes ensure CI tests pass by: 1. Properly detecting configuration drift with better diagnostics 2. Eliminating false positives from whitespace-only changes 3. Fixing Go linting issues that caused build failures
- Fixed ensure-docker-env target to use Windows commands (mkdir/copy) - Fixed clean-build target with Windows-compatible rmdir commands - Fixed db-reset target to suppress errors on Windows - Replaced Unix 'true' command with Windows-compatible error suppression Resolves 'true is not recognized' error in make quickstart on Windows. All Docker operations now work correctly on Windows PowerShell.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.