[DAS-Dashboard#1194] Command-Router not working on dashboard. - #307
Open
levisingularity wants to merge 6 commits into
Open
[DAS-Dashboard#1194] Command-Router not working on dashboard.#307levisingularity wants to merge 6 commits into
levisingularity wants to merge 6 commits into
Conversation
… Web UI API calls
- removed redundant classes and services - removed 'command translation' layer making code overly complex - moved dashboard initial state generation to the back-end.
- Removed constants.js and serviceinventory.js - Reorganized API calls, removed unused api call methods. - Reorganized data rendering to use new keys provided by the cli.
WalkthroughThe change removes peer host and port CLI options, centralizes endpoint configuration, adds a dashboard initial-state API, replaces enum-based service handling with command strings, and updates dashboard state, runtime metrics, service actions, and integration coverage. ChangesCLI service endpoint simplification
Dashboard initial state and service orchestration
Dashboard frontend runtime state
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@das-cli/src/commands/link_creation_agent/lca_docs.py`:
- Line 30: Update the link-creation-agent start command synopsis in lca_docs.py
to show the port-range placeholder with the colon delimiter, matching the parser
and existing examples: start:end rather than start_port-end_port.
In `@das-cli/src/common/bus_node/busnode_command_registry.py`:
- Around line 51-60: The _get_bus_endpoint method’s missing-endpoint ValueError
is being masked by the broad ValueError handling in
BusNodeContainerManager.start_container(). Use a distinct configuration
exception for the agents.query.endpoint validation, or narrow start_container()
to catch only unknown-service errors, so the CLI preserves and displays the
missing query endpoint message.
- Around line 73-107: Add Bats lifecycle coverage for cmd_evolution_agent,
cmd_link_creation_agent, cmd_inference_agent, and cmd_context_broker: verify
start --help and restart --help omit the legacy --peer-hostname and --peer-port
options, and startup commands use the configured agents.query.endpoint. Add
coverage for a missing agents.query.endpoint that asserts an actionable
configuration error.
In `@das-dashboard/backend/controllers/container_controllers.py`:
- Around line 80-124: Add regression tests covering start_service, stop_service,
and restart_service with catalog commands such as query-engine, verifying
configured local and remote host resolution and the generated das-cli argument
order (<service_command>, <action>) with JSON output. Cover the related
manage_container flow in container_services.py, including both command
construction and host selection, while leaving production behavior unchanged.
In `@das-dashboard/backend/services/dashboard_services.py`:
- Around line 12-14: Add integration tests for the GET /initial-state endpoint
through fetch_initial_state, covering both local and remote service
configurations. Assert that hosts rows and serviceServerMap use the expected
service-command keys, and verify missing or invalid configuration returns the
shared handler’s expected error response.
In
`@das-dashboard/src/components/dashboard/MainContent/sidebar/ArchitectureActionControl.jsx`:
- Around line 91-93: Update the useEffect in ArchitectureActionControl so it
initializes selected service IDs only on the first discovery, then preserves the
existing selection across orchestrationServices updates while removing IDs no
longer available. Add a test that deselects a service, refreshes with equivalent
machine data, and verifies that service remains excluded.
In `@das-dashboard/src/hooks/useArchitectureTabMetrics.js`:
- Around line 19-43: Update the host synchronization logic around the fleet
stream setup so every existing host refreshes its stored base services from the
current machines entry instead of returning before applying changes; ensure
runtime patching and fleetMergedServices use that current per-host base list.
During inactive-host cleanup, also remove the host from fleetHostStatsByHost
alongside services, history, and link state so re-added hosts cannot display
stale statistics.
In `@das-dashboard/src/pages/setup_das/SetupDas.jsx`:
- Around line 88-95: Update refreshDashboardState so getInitialState failures
propagate or produce an explicit failure result instead of being swallowed. In
both configuration flows, distinguish a successful write from a failed dashboard
refresh and show a warning when refresh fails, preventing stale machines or
service metadata from being reported as success. Add save and load tests
covering this failure path.
In `@das-dashboard/src/utils/serviceRows.js`:
- Around line 3-63: Add focused tests for patchServicesWithRuntime and
rollupMetricsHistory covering exact service_command_label matches, AtomDB marker
fallback behavior, one-runtime-to-one-row assignment, and preservation of
unmatched base rows. Verify rollupMetricsHistory produces per-container CPU and
memory series limited to the dashboard’s 20-snapshot history window, using the
module’s existing marker/configuration symbols and test conventions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e5d4b647-340c-4d02-a60e-9ca746d1760d
📒 Files selected for processing (48)
das-cli/src/commands/context_broker/context_broker_cli.pydas-cli/src/commands/context_broker/context_broker_docs.pydas-cli/src/commands/evolution_agent/evolution_agent_cli.pydas-cli/src/commands/evolution_agent/evolution_agent_docs.pydas-cli/src/commands/inference_agent/inference_agent_cli.pydas-cli/src/commands/inference_agent/inference_agent_docs.pydas-cli/src/commands/link_creation_agent/lca_docs.pydas-cli/src/commands/link_creation_agent/link_creation_agent_cli.pydas-cli/src/commands/query_agent/query_agent_cli.pydas-cli/src/common/bus_node/busnode_command_registry.pydas-cli/src/common/container_manager/busnode_container_manager.pydas-cli/src/common/factory/busnode_manager_factory.pydas-cli/tests/integration/test_context_broker.batsdas-cli/tests/integration/test_evolution_agent.batsdas-cli/tests/integration/test_inference_agent.batsdas-cli/tests/integration/test_link_creation_agent.batsdas-cli/tests/integration/test_logs.batsdas-dashboard/backend/controllers/config_controllers.pydas-dashboard/backend/controllers/container_controllers.pydas-dashboard/backend/controllers/dashboard_controllers.pydas-dashboard/backend/main.pydas-dashboard/backend/services/config_services.pydas-dashboard/backend/services/container_services.pydas-dashboard/backend/services/dashboard_services.pydas-dashboard/backend/services_init.pydas-dashboard/backend/shared/enums/das_services.pydas-dashboard/backend/shared/exceptions/exception_handlers.pydas-dashboard/backend/shared/internal/web_configuration.pydas-dashboard/backend/shared/utils/service_inventory.pydas-dashboard/src/api/APIUtils.jsdas-dashboard/src/api/ConfigAPI.jsdas-dashboard/src/api/DashboardAPI.jsdas-dashboard/src/api/ServicesAPI.jsdas-dashboard/src/components/dashboard/ArchitectureView/ArchitectureView.jsxdas-dashboard/src/components/dashboard/ArchitectureView/utils/constants.jsdas-dashboard/src/components/dashboard/MainContent/servicestable/AgentRow.jsxdas-dashboard/src/components/dashboard/MainContent/servicestable/ServicesTable.jsxdas-dashboard/src/components/dashboard/MainContent/sidebar/ArchitectureActionControl.jsxdas-dashboard/src/components/dashboard/MainContent/sidebar/AtomDBActionControl.jsxdas-dashboard/src/components/dashboard/MainContent/sidebar/SideBar.jsxdas-dashboard/src/components/global_providers/DashboardContextProvider.jsxdas-dashboard/src/components/global_providers/ServerTabMetricsProvider.jsxdas-dashboard/src/hooks/useArchitectureTabMetrics.jsdas-dashboard/src/hooks/useServerTabMetrics.jsdas-dashboard/src/pages/setup_das/SetupDas.jsxdas-dashboard/src/utils/infraStatus.jsdas-dashboard/src/utils/serviceInventory.jsdas-dashboard/src/utils/serviceRows.js
💤 Files with no reviewable changes (10)
- das-dashboard/src/components/dashboard/ArchitectureView/utils/constants.js
- das-dashboard/backend/services/config_services.py
- das-dashboard/backend/shared/enums/das_services.py
- das-dashboard/src/utils/serviceInventory.js
- das-dashboard/src/api/ConfigAPI.js
- das-cli/tests/integration/test_link_creation_agent.bats
- das-cli/tests/integration/test_evolution_agent.bats
- das-cli/tests/integration/test_inference_agent.bats
- das-cli/tests/integration/test_logs.bats
- das-cli/tests/integration/test_context_broker.bats
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
das-dashboard/src/pages/setup_das/SetupDas.jsx (1)
102-116: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not replace the refresh-failure warning with a success toast.
ToastProviderstores one toast. Each flow shows a warning, then immediately shows a success toast. The success toast replaces the warning. The user can see a successful configuration operation while dashboard state remains stale.
das-dashboard/src/pages/setup_das/SetupDas.jsx#L102-L116: Store the refresh failure and emit one warning toast instead of the subsequent save-success toast.das-dashboard/src/pages/setup_das/SetupDas.jsx#L178-L189: Store the refresh failure and emit one warning toast instead of the subsequent load-success toast.Add save and load tests that reject
getInitialState()and assert that the final visible toast is the warning.#!/bin/bash set -euo pipefail ast-grep outline das-dashboard/src/components/global_providers/ToastProvider.jsx --items all rg -n -C 5 'showToast\s*=|setToast' \ das-dashboard/src/components/global_providers/ToastProvider.jsx rg -n -C 8 \ 'Configuration saved, but dashboard state could not be refreshed|Configuration saved successfully|Configuration loaded, but dashboard state could not be refreshed|Configuration loaded successfully' \ das-dashboard/src/pages/setup_das/SetupDas.jsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@das-dashboard/src/pages/setup_das/SetupDas.jsx` around lines 102 - 116, Update both save and load flows in SetupDas.jsx (lines 102-116 and 178-189) to track whether refreshDashboardState failed and show only the corresponding warning toast when it does, rather than subsequently displaying the save/load success toast; otherwise preserve the success behavior. Add save and load tests that reject getInitialState() and assert the final visible toast is the warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@das-dashboard/src/components/dashboard/MainContent/sidebar/ArchitectureActionControl.jsx`:
- Line 1: Update the selection-initialization effect around
hasInitializedSelection and availableIds so it does not mark initialization
complete when orchestrationServices is empty; return early while availableIds
has no entries, then initialize the selection from all discovered service IDs on
the first nonempty discovery. Add a test that starts with no machines, supplies
orchestration services, and verifies every discovered service is selected.
---
Outside diff comments:
In `@das-dashboard/src/pages/setup_das/SetupDas.jsx`:
- Around line 102-116: Update both save and load flows in SetupDas.jsx (lines
102-116 and 178-189) to track whether refreshDashboardState failed and show only
the corresponding warning toast when it does, rather than subsequently
displaying the save/load success toast; otherwise preserve the success behavior.
Add save and load tests that reject getInitialState() and assert the final
visible toast is the warning.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bdcd0a68-d96a-46e5-b26c-dd4acfb9f00e
📒 Files selected for processing (6)
das-cli/src/commands/link_creation_agent/lca_docs.pydas-cli/src/common/bus_node/busnode_command_registry.pydas-cli/src/common/exceptions.pydas-dashboard/src/components/dashboard/MainContent/sidebar/ArchitectureActionControl.jsxdas-dashboard/src/hooks/useArchitectureTabMetrics.jsdas-dashboard/src/pages/setup_das/SetupDas.jsx
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
1 participant
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.
das-cli, eliminating the command translation layers and slightly simplifying the code.Screencast.from.2026-08-06.15-48-59.webm