Repository navigation
[DAS-Dashboard#1198] General revamp on error treatment. - #308
Closed
levisingularity wants to merge 16 commits into
Closed
levisingularity wants to merge 16 commits into
levisingularity wants to merge 16 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.
… json without double method calls.
- WEB API now uses all das-cli commands with the flag "-o json" and expects to read responses in JSON only. - Responses that fail to be in JSON format are filled with default messages so that the user knows at least something has happened in das-cli.
- Adaptations to use new responses coming from the back-end (WEB API)
Contributor
WalkthroughThe CLI now emits shared structured responses and centralizes database operations. Bus endpoints come from configuration. The dashboard adds initial-state loading, shared CLI response parsing, dynamic service inventories, runtime metric merging, and normalized API error display. ChangesCLI contracts and service lifecycle
Dashboard orchestration and state
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 36
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
das-cli/src/common/decorators.py (1)
122-137: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDuplicate line in plain output.
Line 123 sends a plain string and line 132 sends the same sentence inside
ServiceResponse. Indas-cli/src/common/command.py,stdoutprints plain strings whenoutput_format == "plain"(lines 432-435) and_handle_outputprintsmessageagain in the same mode (lines 412-413). The user sees the message twice.Use
self.log(...)for the human line, as the migrated command modules do.🐛 Proposed fix
else: - self.stdout( + self.log( f"{name} is not running on port {port}", severity=StdoutSeverity.ERROR, )🤖 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-cli/src/common/decorators.py` around lines 122 - 137, Replace the first plain-string self.stdout call in the service-not-running branch with self.log, while retaining the ServiceResponse self.stdout call and its error severity so plain output emits the human-readable message only once.das-cli/src/commands/config/config_cli.py (1)
193-208: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAn unknown key reports
status: errorbut exits 0.
_show_config_keyemitsStdoutStatus.ERRORand then returns.runcompletes normally, so the process exit status is 0. The exit code contradicts the payload, and shell callers cannot detect the failure.Raise after the response so
Command.safe_runmaps it to exit code 1.🐛 Proposed fix
severity=StdoutSeverity.ERROR, ) - return + raise KeyError(f"The key '{key}' does not exist in the configuration file.")Add a bats case under
das-cli/tests/integration/that runsdas-cli config list <unknown-key>and asserts a non-zero status.🤖 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-cli/src/commands/config/config_cli.py` around lines 193 - 208, Update _show_config_key so that after emitting the existing StdoutStatus.ERROR response for a missing key, it raises an exception for Command.safe_run to map to exit code 1 instead of returning normally. Add an integration bats case under tests/integration that invokes config list with an unknown key and asserts a non-zero exit status.Source: Path instructions
das-cli/src/common/container_manager/busnode_container_manager.py (1)
35-49: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRemove the redundant
**kwargsforwarding.
Command.safe_runremoves global parameters, and these commands expose onlyport_range. The normal CLI path therefore passes an emptykwargsmapping, so noTypeErroroccurs. Remove the unused**kwargsparameters and forwarding in the AtomDB Broker and Query Agent start paths.🤖 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-cli/src/common/container_manager/busnode_container_manager.py` around lines 35 - 49, Remove the unused **kwargs parameters and forwarding from the AtomDB Broker and Query Agent start paths. Update their start command methods and callers so they accept and pass only the supported port_range argument, while preserving the existing Command.safe_run behavior and CLI flow.das-dashboard/src/components/dashboard/MainContent/servicestable/AgentRow.jsx (1)
20-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard against a missing
service_keybefore the action call.If a row has no
service_key,AgentRowstill enables its action andServicesAPIbuilds a/services/undefined/{action}request. Return early whenagent.service_keyis missing.🤖 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/components/dashboard/MainContent/servicestable/AgentRow.jsx` around lines 20 - 35, Update executeAction in AgentRow to return early when agent.service_key is missing, before invoking onAction. Preserve the existing action-state guards and only call onAction for rows with a valid service_key.das-dashboard/src/components/dashboard/MainContent/sidebar/SideBar.jsx (1)
38-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIgnore stale infrastructure-status responses.
When configuration changes replace
machines, an earlierfetchInfraStatusForAllHosts()request can resolve after the new request. Its result then overwritesatomDbOnlineandarchitectureOnlinefor the current configuration. Track the latest request or configuration generation and discard older results before calling the state setters.Based on learnings: "Save and load operations can select different configuration files with different server sets."
🤖 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/components/dashboard/MainContent/sidebar/SideBar.jsx` around lines 38 - 48, Update loadInfraStatus to track the latest machines configuration or request generation, and after fetchInfraStatusForAllHosts resolves, ignore the result unless it still belongs to the current generation. Only call setAtomDbOnline and setArchitectureOnline for the latest request, preserving correct status when server sets change.Source: Learnings
🤖 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/atomdb_broker/atomdb_broker_cli.py`:
- Line 84: Remove the unused exception binding from the
DockerContainerDuplicateError handler in the duplicate-container branch,
matching the unbound catch used by the peer command router implementation.
- Around line 99-106: In the error branch of the service-start handling, update
the ServiceResponse construction to reuse the existing message variable instead
of repeating the literal string. Ensure the message assignment remains the
single source for the error text and remove any redundant or unused binding.
- Around line 198-200: Update the command definition’s help field to use
HELP_RESTART instead of SHORT_HELP_RESTART, while leaving short_help assigned to
SHORT_HELP_RESTART.
In `@das-cli/src/commands/attention_broker/attention_broker_cli.py`:
- Line 154: Correct the user-facing start-error message from “instanciate” to
“instantiate” in attention_broker_cli.py:154-154, context_broker_cli.py:164-164,
inference_agent_cli.py:167-167, and jupyter_notebook_cli.py:114-114; apply the
same correction to the identical message in command_router_cli.py and
link_creation_agent_cli.py, preferably by introducing and reusing a shared
constant.
- Around line 148-159: Ensure each migrated start command re-raises the caught
exception after reporting the failure: add raise e in the
DockerError/PortBindingError handlers in
das-cli/src/commands/attention_broker/attention_broker_cli.py lines 148-159,
context_broker/context_broker_cli.py lines 158-169,
inference_agent/inference_agent_cli.py lines 161-172, and
jupyter_notebook/jupyter_notebook_cli.py lines 108-120. Add bats integration
cases under das-cli/tests/integration/ that force a port conflict for each
service and assert a non-zero exit status.
- Around line 50-84: Update the stop method around
self._attention_broker_manager.stop() to capture self._get_container() once
before stopping, then reuse that container for both success and
DockerContainerNotFoundError responses instead of calling _get_container()
multiple times; retain the existing stop behavior and messages.
In `@das-cli/src/commands/command_router/command_router_cli.py`:
- Around line 88-110: Start and stop exception handlers must reuse the cached
container instead of calling _get_container() again. Update
command_router_cli.py lines 88-110 and the corresponding _stop_container sites
at lines 166 and 176, plus atomdb_broker_cli.py lines 87-109 and lines 78 and
175, replacing each handler’s container lookup with the local container variable
so the original error response is preserved.
In `@das-cli/src/commands/config/config_cli.py`:
- Around line 77-91: Update _finish_set so config set output is redacted before
ServiceResponse serialization; do not pass the raw self._settings.get_content(),
which may contain cluster_secret_key or credentials. Emit only the settings path
or reuse an established redaction/normalization mechanism such as
_normalize_config, while preserving the success response structure and
JSON-parseable stdout.
In `@das-cli/src/commands/db/db_services.py`:
- Around line 77-80: Replace the direct manager._options accesses in
DbOperations with public property accessors for redis_port, redis_nodes, and
redis_cluster. Add the corresponding properties to each affected container
manager, following the existing name and port property pattern in
ContainerManager, and update all five call sites to use those accessors instead
of the protected options dictionary.
- Around line 111-117: Update the service startup flow in start_redis and the
corresponding MongoDB startup method so each cluster bootstrap is gated by
errors from that service invocation only, rather than the shared self.errors
list. Track or capture per-service failures around container startup, then allow
Redis or MongoDB cluster initialization when its own containers succeeded even
if another service previously recorded errors.
- Around line 1-273: Add bats integration coverage under
das-cli/tests/integration for DbOperations orchestration paths: verify occupied
Redis ports return an ERROR response and nonzero exit status; repeated db start
reports “already running” with success; stopping a missing container reports
“already stopped” with success; db stop --prune followed by db start confirms
volumes are removed and startup succeeds; and cluster startup with an
unreachable node reports an error naming that node.
- Around line 41-57: The failure paths must return non-zero process status after
emitting their error responses. Update DbOperations.finish in
das-cli/src/commands/db/db_services.py:41-57, MettaLoad._finish_load in
das-cli/src/commands/metta/metta_cli.py:84-101, and MettaCheck._finish_check in
das-cli/src/commands/metta/metta_cli.py:216-233 to raise or otherwise propagate
failure after writing the response; also prevent DbRestart from starting
databases when its stop operation fails. Add bats coverage for each failure path
and verify successful paths remain unchanged.
In `@das-cli/src/commands/metta/metta_cli.py`:
- Around line 272-281: Update _validate_directory to catch IsADirectoryError and
FileNotFoundError from each _validate_file call, convert those failures into the
existing error-message format, and continue processing remaining entries. Ensure
all per-entry results are aggregated so _finish_check can always produce a
single ServiceResponse.
In `@das-cli/src/commands/system/system_cli.py`:
- Around line 256-261: Move the os.system("clear") call into the plain-output
branch alongside _format_info_for_display, so machine-readable formats pass
system_info directly to stdout without terminal control sequences.
In `@das-cli/src/common/bus_node/busnode_command_registry.py`:
- Around line 74-108: Extract the shared attention-broker and bus-endpoint
command suffix into a helper in the command registry, then update
cmd_evolution_agent, cmd_link_creation_agent, cmd_inference_agent, and
cmd_context_broker to reuse it while preserving flag order. Also reuse the
attention-broker portion in cmd_query_engine without changing its
command-specific behavior.
- Around line 52-61: Update the agent start command path to catch
ConfigurationError raised by _get_bus_endpoint and convert it into the
established ServiceResponse JSON error format. Preserve the existing endpoint
validation and ensure missing agents.query.endpoint configuration no longer
propagates as an unhandled exception.
In `@das-cli/src/common/command.py`:
- Line 5: Remove the unused Any symbol from the typing import in command.py,
leaving the remaining imports unchanged.
- Around line 404-421: Update _handle_output so the plain output branch includes
the serialized payload error alongside the existing message before calling
_print_colored. Preserve the current JSON and YAML serialization behavior, and
retain the static message while appending error details when present.
- Around line 439-440: Remove the no-op flush_stdout method and both safe_run
call sites that invoke it, unless the hook is intentionally required; in that
case, document it as an extension point and retain the calls.
In `@das-cli/src/common/container_manager/atomdb/mongodb_container_manager.py`:
- Around line 41-43: Update the connection setup in the container manager method
around cluster_node and ssh.open: resolve both host and username before entering
the try block, retrieve username safely, and raise ConfigurationError when
either resolved value is missing instead of calling ssh.open with invalid data.
Keep the exception handler limited to SSH failures and chain the original SSH
exception when converting it to the manager’s error type; apply the same
validation at the corresponding duplicate location.
In `@das-cli/src/common/container_manager/busnode_container_manager.py`:
- Line 35: Resolve the return-type mismatch in
BusNodeContainerManager.start_container by either removing the returned
container or changing the annotation to the container’s actual type; preserve
the current caller behavior, since callers ignore the result.
In `@das-cli/src/common/service_response.py`:
- Around line 59-60: Update the error-field condition in the response
serialization method to check whether self.error is not None rather than relying
on truthiness, ensuring explicitly supplied falsy values are passed to
_serialize_error and emitted in the response.
In `@das-cli/src/das_cli.py`:
- Line 15: Restore the InferenceAgentModule import and its registration entry in
MODULES so the inference-agent CLI and help text are reachable. Add a bats
integration test under das-cli/tests/integration/ verifying das-cli
inference-agent --help exits successfully and lists start, stop, and restart;
also cover relevant missing-config or container-failure error paths if
consistent with existing test patterns.
In `@das-dashboard/backend/controllers/container_controllers.py`:
- Around line 80-124: Update the manage_container calls in start_service,
stop_service, and restart_service to pass service_command through the command
keyword instead of container_name, matching the database routes and preserving
the service-command semantics.
In `@das-dashboard/backend/services/container_services.py`:
- Around line 180-204: Define a module-level DAS_CLI_COMMAND_TIMEOUT constant in
container_services.py and pass it as the timeout argument to
run_das_cli_json_command within run_das_cli_command. Choose a value sufficient
for the slowest lifecycle operation, including container image pulls, while
ensuring remote calls cannot block indefinitely.
- Around line 130-135: Update _run_service_command to catch
DasCliResponseDecodeException and return a failed per-service result containing
the service context and decode-error details, rather than re-raising. Preserve
aggregation so _orchestrate_local continues processing remaining services and
_orchestrate_remote retains completed results and errors; keep any required
single-service 422 behavior by handling the re-raise only in manage_container.
- Around line 206-207: Remove the unused _clean_cli_output method from the
relevant service class and delete its clean_cli_output import, ensuring no
remaining references depend on either symbol.
In `@das-dashboard/backend/services/metrics_services.py`:
- Around line 89-99: Update the parsing flow in the surrounding metrics service
method so JSON list payloads are inspected before calling the dictionary-only
parse_das_cli_stdout function. Convert a zero-exit payload shaped like ["error
detail"] into DasCliCommandException with DEFAULT_CLI_ERROR_MESSAGE and the
detail, while preserving dictionary success handling and existing stdout
fallback behavior. Add tests covering both zero-exit list errors and zero-exit
dictionary successes.
In `@das-dashboard/backend/shared/exceptions/custom_exceptions.py`:
- Around line 11-24: Update the constructor branch handling missing detail and
stderror so the legacy message-to-detail demotion applies only to a single
positional argument, while an explicit message= keyword remains self.message and
is not replaced with DEFAULT_MESSAGE. Remove the affected local reassignments
while preserving self.stderror, super().__init__, and __str__ behavior, and
verify positional raise sites retain their existing semantics.
In `@das-dashboard/backend/shared/utils/das_cli_response.py`:
- Around line 120-126: Update is_cli_success to return status in
SUCCESS_STATUSES after the existing None check, removing the redundant status
not in ERROR_STATUSES fallback so unknown statuses are rejected. Add tests
covering declared success statuses, error status, unknown status, and the None
case.
In
`@das-dashboard/src/components/dashboard/MainContent/sidebar/ArchitectureActionControl.jsx`:
- Around line 92-101: Update the selection initialization logic in the useEffect
for orchestrationServices so an empty availableIds array does not set
hasInitializedSelection.current or replace the selection. Mark initialization
complete and initialize selectedServices only when availableIds is non-empty,
while preserving filtering behavior for subsequent updates.
In `@das-dashboard/src/components/global_providers/DashboardContextProvider.jsx`:
- Around line 30-37: Update the setCurrentMachine callback in applyInitialState
so it always resets currentMachine to machineList[0] when the list is non-empty,
or null when empty; remove the logic that preserves the previous machine by
matching serverIp.
In `@das-dashboard/src/hooks/useArchitectureTabMetrics.js`:
- Around line 41-52: Update the fleetStreamsRef.current[hostIp] branch in the
hostList iteration to preserve existing fleetServicesByHost rows when machines
changes identity. Instead of deleting the live rows, retain them and patch or
reconcile them against the newly computed baseServices, so rendering continues
using live data until the next WebSocket payload.
In `@das-dashboard/src/hooks/useServerTabMetrics.js`:
- Around line 109-113: In das-dashboard/src/hooks/useServerTabMetrics.js (lines
109-113), derive a stable service-set key for the stream effect, replace the
baseServices dependency with that key, and read the current array through a ref
so renders do not restart the socket. In
das-dashboard/src/hooks/useArchitectureTabMetrics.js (lines 41-52), key the
effect by stable host and service values rather than machines identity, and
update existing rows in the stream-reuse branch instead of deleting the host
entry.
In `@das-dashboard/src/pages/query/QueryPage.jsx`:
- Line 14: Restore the missing useQueryParameters import in QueryPage.jsx so
QueryPageContent can resolve its hook call without a ReferenceError.
In `@das-dashboard/src/utils/serviceRows.js`:
- Around line 70-72: Update formatMemoryCell to convert agent.memory_mb from
megabytes to gigabytes by dividing the numeric value by 1024 before applying
toFixed(2), while preserving the null placeholder and GB suffix.
---
Outside diff comments:
In `@das-cli/src/commands/config/config_cli.py`:
- Around line 193-208: Update _show_config_key so that after emitting the
existing StdoutStatus.ERROR response for a missing key, it raises an exception
for Command.safe_run to map to exit code 1 instead of returning normally. Add an
integration bats case under tests/integration that invokes config list with an
unknown key and asserts a non-zero exit status.
In `@das-cli/src/common/container_manager/busnode_container_manager.py`:
- Around line 35-49: Remove the unused **kwargs parameters and forwarding from
the AtomDB Broker and Query Agent start paths. Update their start command
methods and callers so they accept and pass only the supported port_range
argument, while preserving the existing Command.safe_run behavior and CLI flow.
In `@das-cli/src/common/decorators.py`:
- Around line 122-137: Replace the first plain-string self.stdout call in the
service-not-running branch with self.log, while retaining the ServiceResponse
self.stdout call and its error severity so plain output emits the human-readable
message only once.
In
`@das-dashboard/src/components/dashboard/MainContent/servicestable/AgentRow.jsx`:
- Around line 20-35: Update executeAction in AgentRow to return early when
agent.service_key is missing, before invoking onAction. Preserve the existing
action-state guards and only call onAction for rows with a valid service_key.
In `@das-dashboard/src/components/dashboard/MainContent/sidebar/SideBar.jsx`:
- Around line 38-48: Update loadInfraStatus to track the latest machines
configuration or request generation, and after fetchInfraStatusForAllHosts
resolves, ignore the result unless it still belongs to the current generation.
Only call setAtomDbOnline and setArchitectureOnline for the latest request,
preserving correct status when server sets change.
🪄 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: b8493c19-0b78-4445-a4c6-8d6676ed3936
📒 Files selected for processing (89)
das-cli/src/commands/atomdb_broker/atomdb_broker_cli.pydas-cli/src/commands/atomdb_broker/atomdb_broker_service_response.pydas-cli/src/commands/attention_broker/attention_broker_cli.pydas-cli/src/commands/attention_broker/attention_broker_service_response.pydas-cli/src/commands/command_router/command_router_cli.pydas-cli/src/commands/command_router/command_router_service_response.pydas-cli/src/commands/config/config_cli.pydas-cli/src/commands/context_broker/context_broker_cli.pydas-cli/src/commands/context_broker/context_broker_container_service_response.pydas-cli/src/commands/context_broker/context_broker_docs.pydas-cli/src/commands/database_adapter/database_adapter_service_response.pydas-cli/src/commands/database_adapter/dbms_adapter_cli.pydas-cli/src/commands/db/db_cli.pydas-cli/src/commands/db/db_service_response.pydas-cli/src/commands/db/db_services.pydas-cli/src/commands/evolution_agent/evolution_agent_cli.pydas-cli/src/commands/evolution_agent/evolution_agent_docs.pydas-cli/src/commands/evolution_agent/evolution_agent_service_response.pydas-cli/src/commands/example/example_cli.pydas-cli/src/commands/inference_agent/inference_agent_cli.pydas-cli/src/commands/inference_agent/inference_agent_container_service_response.pydas-cli/src/commands/inference_agent/inference_agent_docs.pydas-cli/src/commands/inference_agent/inference_agent_module.pydas-cli/src/commands/jupyter_notebook/jupyter_notebook_agent_container_service_response.pydas-cli/src/commands/jupyter_notebook/jupyter_notebook_cli.pydas-cli/src/commands/link_creation_agent/lca_docs.pydas-cli/src/commands/link_creation_agent/link_creation_agent_cli.pydas-cli/src/commands/link_creation_agent/link_creation_agent_container_service_response.pydas-cli/src/commands/metta/metta_cli.pydas-cli/src/commands/query_agent/query_agent_cli.pydas-cli/src/commands/query_agent/query_agent_container_service_response.pydas-cli/src/commands/system/system_cli.pydas-cli/src/common/__init__.pydas-cli/src/common/bus_node/busnode_command_registry.pydas-cli/src/common/command.pydas-cli/src/common/container_manager/atomdb/mongodb_container_manager.pydas-cli/src/common/container_manager/busnode_container_manager.pydas-cli/src/common/decorators.pydas-cli/src/common/exceptions.pydas-cli/src/common/factory/busnode_manager_factory.pydas-cli/src/common/service_response.pydas-cli/src/das_cli.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/database_services.pydas-dashboard/backend/services/metrics_services.pydas-dashboard/backend/services_init.pydas-dashboard/backend/shared/enums/das_services.pydas-dashboard/backend/shared/exceptions/custom_exceptions.pydas-dashboard/backend/shared/exceptions/exception_handlers.pydas-dashboard/backend/shared/internal/web_configuration.pydas-dashboard/backend/shared/utils/das_cli_config.pydas-dashboard/backend/shared/utils/das_cli_response.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/common/ApiErrorNotice.jsxdas-dashboard/src/components/configuration_page/AtomDB/AdapterDB/AdapterDB.jsxdas-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/MettaLoadActionControl.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/components/query_page/QueryAllAnswersModal.jsxdas-dashboard/src/hooks/useArchitectureTabMetrics.jsdas-dashboard/src/hooks/useQueryExecution.jsdas-dashboard/src/hooks/useServerTabMetrics.jsdas-dashboard/src/pages/query/QueryPage.jsxdas-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 (21)
- das-cli/src/commands/context_broker/context_broker_container_service_response.py
- das-cli/src/commands/attention_broker/attention_broker_service_response.py
- das-cli/src/commands/command_router/command_router_service_response.py
- das-cli/src/commands/database_adapter/database_adapter_service_response.py
- das-cli/src/commands/query_agent/query_agent_container_service_response.py
- das-cli/src/commands/link_creation_agent/link_creation_agent_container_service_response.py
- das-dashboard/backend/shared/enums/das_services.py
- das-cli/src/commands/db/db_service_response.py
- das-cli/src/commands/inference_agent/inference_agent_container_service_response.py
- das-cli/src/commands/jupyter_notebook/jupyter_notebook_agent_container_service_response.py
- das-dashboard/src/utils/serviceInventory.js
- das-cli/src/commands/evolution_agent/evolution_agent_service_response.py
- das-dashboard/src/api/ConfigAPI.js
- das-cli/tests/integration/test_link_creation_agent.bats
- das-cli/tests/integration/test_inference_agent.bats
- das-cli/tests/integration/test_evolution_agent.bats
- das-cli/tests/integration/test_logs.bats
- das-dashboard/src/components/dashboard/ArchitectureView/utils/constants.js
- das-cli/tests/integration/test_context_broker.bats
- das-cli/src/commands/atomdb_broker/atomdb_broker_service_response.py
- das-dashboard/backend/services/config_services.py
| ) | ||
|
|
||
| except DockerContainerDuplicateError: | ||
| except DockerContainerDuplicateError as e: |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove the unused exception binding.
except DockerContainerDuplicateError as e: binds e, but the duplicate-container branch never uses it. Pylint reports W0612 here. The peer file das-cli/src/commands/command_router/command_router_cli.py line 85 catches the same exception without a binding.
♻️ Proposed cleanup
- except DockerContainerDuplicateError as e:
+ except DockerContainerDuplicateError:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| except DockerContainerDuplicateError as e: | |
| except DockerContainerDuplicateError: |
🧰 Tools
🪛 Pylint (4.0.6)
[warning] 84-96: Unused variable 'e'
(W0612)
🤖 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-cli/src/commands/atomdb_broker/atomdb_broker_cli.py` at line 84, Remove
the unused exception binding from the DockerContainerDuplicateError handler in
the duplicate-container branch, matching the unbound catch used by the peer
command router implementation.
Source: Linters/SAST tools
Comment on lines
+99
to
+106
| message = "DAS-CLI failed to instanciate a container of this service." | ||
|
|
||
| self.stdout( | ||
| ServiceResponse( | ||
| service=CLI_SERVICE_NAME, | ||
| action="start", | ||
| status=StdoutStatus.ERROR, | ||
| message="DAS-CLI failed to instanciate a container of this service.", |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Delete the dead message variable.
Line 99 assigns message but the branch never reads it. Line 106 repeats the same literal inline. Bind the literal once and pass the variable.
♻️ Proposed cleanup
except (DockerError, PortBindingError) as e:
message = "DAS-CLI failed to instanciate a container of this service."
-
self.stdout(
ServiceResponse(
service=CLI_SERVICE_NAME,
action="start",
status=StdoutStatus.ERROR,
- message="DAS-CLI failed to instanciate a container of this service.",
+ message=message,
error=e,
container=self._get_container(),
),
severity=StdoutSeverity.ERROR,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| message = "DAS-CLI failed to instanciate a container of this service." | |
| self.stdout( | |
| ServiceResponse( | |
| service=CLI_SERVICE_NAME, | |
| action="start", | |
| status=StdoutStatus.ERROR, | |
| message="DAS-CLI failed to instanciate a container of this service.", | |
| message = "DAS-CLI failed to instanciate a container of this service." | |
| self.stdout( | |
| ServiceResponse( | |
| service=CLI_SERVICE_NAME, | |
| action="start", | |
| status=StdoutStatus.ERROR, | |
| message=message, | |
| error=e, | |
| container=self._get_container(), | |
| ), | |
| severity=StdoutSeverity.ERROR, | |
| ) |
🤖 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-cli/src/commands/atomdb_broker/atomdb_broker_cli.py` around lines 99 -
106, In the error branch of the service-start handling, update the
ServiceResponse construction to reuse the existing message variable instead of
repeating the literal string. Ensure the message assignment remains the single
source for the error text and remove any redundant or unused binding.
Comment on lines
+198
to
200
| short_help = SHORT_HELP_RESTART | ||
|
|
||
| help = SHORT_HELP_RESTART |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check which restart help constants the atomdb_broker docs module exports.
set -euo pipefail
fd --type f 'atomdb_broker_docs.py' das-cli/src --exec rg -n 'HELP_RESTART|SHORT_HELP_RESTART'
rg -n 'HELP_RESTART' das-cli/src/commands/atomdb_broker/Repository: singnet/das-toolbox
Length of output: 783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- atomdb_broker_docs.py ---'
sed -n '42,78p' das-cli/src/commands/atomdb_broker/atomdb_broker_docs.py
printf '%s\n' '--- atomdb_broker_cli.py ---'
sed -n '1,28p;184,207p' das-cli/src/commands/atomdb_broker/atomdb_broker_cli.py
printf '%s\n' '--- evolution_agent_cli.py ---'
sed -n '190,207p' das-cli/src/commands/evolution_agent/evolution_agent_cli.pyRepository: singnet/das-toolbox
Length of output: 2881
Assign HELP_RESTART to help. HELP_RESTART contains the full restart documentation, but the command currently uses SHORT_HELP_RESTART for both fields.
🤖 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-cli/src/commands/atomdb_broker/atomdb_broker_cli.py` around lines 198 -
200, Update the command definition’s help field to use HELP_RESTART instead of
SHORT_HELP_RESTART, while leaving short_help assigned to SHORT_HELP_RESTART.
Comment on lines
+50
to
84
| self.log("Stopping Attention Broker service...", severity=StdoutSeverity.INFO) | ||
|
|
||
| try: | ||
| self.stdout("Stopping Attention Broker service...") | ||
| self._attention_broker_manager.stop() | ||
|
|
||
| success_message = "Attention Broker service stopped" | ||
| exec_message = "Attention Broker service stopped" | ||
|
|
||
| self.stdout( | ||
| success_message, | ||
| severity=StdoutSeverity.SUCCESS, | ||
| ) | ||
| self.stdout( | ||
| dict( | ||
| AttentionBrokerServiceResponse( | ||
| ServiceResponse( | ||
| service=CLI_SERVICE_NAME, | ||
| action="stop", | ||
| status="success", | ||
| message=success_message, | ||
| status=StdoutStatus.SUCCESS, | ||
| message=exec_message, | ||
| container=self._get_container(), | ||
| ) | ||
| ), | ||
| ), | ||
| stdout_type=StdoutType.MACHINE_READABLE, | ||
| severity=StdoutSeverity.SUCCESS, | ||
| ) | ||
|
|
||
| except DockerContainerNotFoundError: | ||
| container_name = self._attention_broker_manager.get_container().name | ||
| warning_message = ( | ||
| f"The Attention Broker service named {container_name} is already stopped." | ||
| ) | ||
| self.stdout( | ||
| warning_message, | ||
| severity=StdoutSeverity.WARNING, | ||
| ) | ||
| message = f"The Attention Broker service named {container_name} is already stopped." | ||
|
|
||
| self.stdout( | ||
| dict( | ||
| AttentionBrokerServiceResponse( | ||
| ServiceResponse( | ||
| service=CLI_SERVICE_NAME, | ||
| action="stop", | ||
| status="already_stopped", | ||
| message=warning_message, | ||
| status=StdoutStatus.INFO, | ||
| message=message, | ||
| container=self._get_container(), | ||
| ) | ||
| ), | ||
| ), | ||
| stdout_type=StdoutType.MACHINE_READABLE, | ||
| severity=StdoutSeverity.WARNING, | ||
| ) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Capture the container once, before the stop.
QueryAgentStop, LinkCreationAgentStop, and CommandRouterStop bind container = self._get_container() at the top of the method and reuse it. This method calls _get_container() three times, including after the stop. Align with the neighboring implementations.
♻️ Proposed refactor
def _attention_broker(self):
+ container = self._get_container()
+
self.log("Stopping Attention Broker service...", severity=StdoutSeverity.INFO)
try:
self._attention_broker_manager.stop()
exec_message = "Attention Broker service stopped"
self.stdout(
dict(
ServiceResponse(
service=CLI_SERVICE_NAME,
action="stop",
status=StdoutStatus.SUCCESS,
message=exec_message,
- container=self._get_container(),
+ container=container,
),
),
severity=StdoutSeverity.SUCCESS,
)
except DockerContainerNotFoundError:
- container_name = self._attention_broker_manager.get_container().name
- message = f"The Attention Broker service named {container_name} is already stopped."
+ message = f"The Attention Broker service named {container.name} is already stopped."
self.stdout(
dict(
ServiceResponse(
service=CLI_SERVICE_NAME,
action="stop",
status=StdoutStatus.INFO,
message=message,
- container=self._get_container(),
+ container=container,
),
),
severity=StdoutSeverity.WARNING,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| self.log("Stopping Attention Broker service...", severity=StdoutSeverity.INFO) | |
| try: | |
| self.stdout("Stopping Attention Broker service...") | |
| self._attention_broker_manager.stop() | |
| success_message = "Attention Broker service stopped" | |
| exec_message = "Attention Broker service stopped" | |
| self.stdout( | |
| success_message, | |
| severity=StdoutSeverity.SUCCESS, | |
| ) | |
| self.stdout( | |
| dict( | |
| AttentionBrokerServiceResponse( | |
| ServiceResponse( | |
| service=CLI_SERVICE_NAME, | |
| action="stop", | |
| status="success", | |
| message=success_message, | |
| status=StdoutStatus.SUCCESS, | |
| message=exec_message, | |
| container=self._get_container(), | |
| ) | |
| ), | |
| ), | |
| stdout_type=StdoutType.MACHINE_READABLE, | |
| severity=StdoutSeverity.SUCCESS, | |
| ) | |
| except DockerContainerNotFoundError: | |
| container_name = self._attention_broker_manager.get_container().name | |
| warning_message = ( | |
| f"The Attention Broker service named {container_name} is already stopped." | |
| ) | |
| self.stdout( | |
| warning_message, | |
| severity=StdoutSeverity.WARNING, | |
| ) | |
| message = f"The Attention Broker service named {container_name} is already stopped." | |
| self.stdout( | |
| dict( | |
| AttentionBrokerServiceResponse( | |
| ServiceResponse( | |
| service=CLI_SERVICE_NAME, | |
| action="stop", | |
| status="already_stopped", | |
| message=warning_message, | |
| status=StdoutStatus.INFO, | |
| message=message, | |
| container=self._get_container(), | |
| ) | |
| ), | |
| ), | |
| stdout_type=StdoutType.MACHINE_READABLE, | |
| severity=StdoutSeverity.WARNING, | |
| ) | |
| def _attention_broker(self): | |
| container = self._get_container() | |
| self.log("Stopping Attention Broker service...", severity=StdoutSeverity.INFO) | |
| try: | |
| self._attention_broker_manager.stop() | |
| exec_message = "Attention Broker service stopped" | |
| self.stdout( | |
| dict( | |
| ServiceResponse( | |
| service=CLI_SERVICE_NAME, | |
| action="stop", | |
| status=StdoutStatus.SUCCESS, | |
| message=exec_message, | |
| container=container, | |
| ), | |
| ), | |
| severity=StdoutSeverity.SUCCESS, | |
| ) | |
| except DockerContainerNotFoundError: | |
| message = f"The Attention Broker service named {container.name} is already stopped." | |
| self.stdout( | |
| dict( | |
| ServiceResponse( | |
| service=CLI_SERVICE_NAME, | |
| action="stop", | |
| status=StdoutStatus.INFO, | |
| message=message, | |
| container=container, | |
| ), | |
| ), | |
| severity=StdoutSeverity.WARNING, | |
| ) |
🤖 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-cli/src/commands/attention_broker/attention_broker_cli.py` around lines
50 - 84, Update the stop method around self._attention_broker_manager.stop() to
capture self._get_container() once before stopping, then reuse that container
for both success and DockerContainerNotFoundError responses instead of calling
_get_container() multiple times; retain the existing stop behavior and messages.
Comment on lines
+148
to
159
| except (DockerError, PortBindingError) as e: | ||
| self.stdout( | ||
| dict( | ||
| AttentionBrokerServiceResponse( | ||
| action="start", | ||
| status="already_running", | ||
| message=warning_message, | ||
| container=container, | ||
| ) | ||
| ServiceResponse( | ||
| service=CLI_SERVICE_NAME, | ||
| action="start", | ||
| status=StdoutStatus.ERROR, | ||
| message="DAS-CLI failed to instanciate a container of this service.", | ||
| error=e, | ||
| container=container, | ||
| ), | ||
| stdout_type=StdoutType.MACHINE_READABLE, | ||
| severity=StdoutSeverity.ERROR, | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Every migrated start command exits with status 0 after a container failure. The shared root cause is the removed re-raise in the except (DockerError, PortBindingError) handlers. Command.safe_run in das-cli/src/common/command.py converts a propagated exception into click.exceptions.Exit(1); when the handler returns normally, the process exits 0 while the payload reports status: error. Shell callers, the bats integration suites, and the dashboard backend that shells out to das-cli all lose failure detection.
das-cli/src/commands/attention_broker/attention_broker_cli.py#L148-L159: addraise eafter theself.stdout(...)call in theexcept (DockerError, PortBindingError)block.das-cli/src/commands/context_broker/context_broker_cli.py#L158-L169: addraise eafter theself.stdout(...)call in theexcept (DockerError, PortBindingError)block.das-cli/src/commands/inference_agent/inference_agent_cli.py#L161-L172: addraise eafter theself.stdout(...)call in theexcept (DockerError, PortBindingError)block.das-cli/src/commands/jupyter_notebook/jupyter_notebook_cli.py#L108-L120: addraise eafter theself.stdout(...)call in theexcept (DockerError, PortBindingError)block.
Add bats cases under das-cli/tests/integration/ that force a port conflict for each service and assert a non-zero exit status.
📍 Affects 4 files
das-cli/src/commands/attention_broker/attention_broker_cli.py#L148-L159(this comment)das-cli/src/commands/context_broker/context_broker_cli.py#L158-L169das-cli/src/commands/inference_agent/inference_agent_cli.py#L161-L172das-cli/src/commands/jupyter_notebook/jupyter_notebook_cli.py#L108-L120
🤖 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-cli/src/commands/attention_broker/attention_broker_cli.py` around lines
148 - 159, Ensure each migrated start command re-raises the caught exception
after reporting the failure: add raise e in the DockerError/PortBindingError
handlers in das-cli/src/commands/attention_broker/attention_broker_cli.py lines
148-159, context_broker/context_broker_cli.py lines 158-169,
inference_agent/inference_agent_cli.py lines 161-172, and
jupyter_notebook/jupyter_notebook_cli.py lines 108-120. Add bats integration
cases under das-cli/tests/integration/ that force a port conflict for each
service and assert a non-zero exit status.
Source: Path instructions
Comment on lines
+30
to
+37
| setCurrentMachine((current) => { | ||
| if (!machineList.length) { | ||
| return null; | ||
| } | ||
| if (!current) { | ||
| return machineList[0]; | ||
| } | ||
| return machineList.find((machine) => machine.serverIp === current.serverIp) ?? machineList[0]; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset currentMachine for each applied configuration.
This code preserves the previous machine when its IP remains in the new configuration. A configuration change can also change the services for that IP. Reset currentMachine to machineList[0] or null on every applyInitialState call.
Based on learnings: "setDashboardBaseValues callback intentionally resets currentMachine to machineList[0] (or null) every time it's called."
🤖 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/components/global_providers/DashboardContextProvider.jsx`
around lines 30 - 37, Update the setCurrentMachine callback in applyInitialState
so it always resets currentMachine to machineList[0] when the list is non-empty,
or null when empty; remove the logic that preserves the previous machine by
matching serverIp.
Source: Learnings
Comment on lines
41
to
52
| hostList.forEach((hostIp) => { | ||
| const baseServices = machines.find((m) => m.serverIp === hostIp)?.services ?? []; | ||
| baseServicesByHostRef.current[hostIp] = baseServices; | ||
|
|
||
| if (fleetStreamsRef.current[hostIp]) { | ||
| setFleetServicesByHost((prev) => { | ||
| const next = { ...prev }; | ||
| delete next[hostIp]; | ||
| return next; | ||
| }); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C6 'useArchitectureTabMetrics\(' das-dashboard/srcRepository: singnet/das-toolbox
Length of output: 3019
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- hook outline ---'
ast-grep outline das-dashboard/src/hooks/useArchitectureTabMetrics.js
printf '%s\n' '--- hook implementation ---'
cat -n das-dashboard/src/hooks/useArchitectureTabMetrics.js
printf '%s\n' '--- runtime patch helper ---'
rg -n -C8 'function patchServicesWithRuntime|const patchServicesWithRuntime|export .*patchServicesWithRuntime' das-dashboard/src
printf '%s\n' '--- provider and machine state ---'
cat -n das-dashboard/src/components/global_providers/ArchitectureTabMetricsProvider.jsx
rg -n -C8 'setMachines|machines:' das-dashboard/src/components das-dashboard/src/hooks das-dashboard/src/context das-dashboard/src/api 2>/dev/null | head -240Repository: singnet/das-toolbox
Length of output: 13201
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- service row helper ---'
cat -n das-dashboard/src/utils/serviceRows.js
printf '%s\n' '--- dashboard context ---'
cat -n das-dashboard/src/components/global_providers/DashboardContextProvider.jsx
printf '%s\n' '--- initial-state callers ---'
rg -n -C10 'applyInitialState|setMachines|hosts\s*:' das-dashboard/src --glob '!**/node_modules/**'
printf '%s\n' '--- hook tests and related tests ---'
rg -n -C8 'fleetServicesByHost|patchServicesWithRuntime|useArchitectureTabMetrics' das-dashboard --glob '*test*' --glob '*spec*'Repository: singnet/das-toolbox
Length of output: 17196
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- refresh path ---'
cat -n das-dashboard/src/pages/setup_das/SetupDas.jsx | sed -n '84,155p'
printf '%s\n' '--- metrics stream implementation ---'
cat -n das-dashboard/src/api/MetricsStreamService.js
printf '%s\n' '--- consumers of merged services ---'
rg -n -C6 'fleetMergedServices|fleetServicesByHost|fleetStreamTick' das-dashboard/src
printf '%s\n' '--- exact hook callers ---'
rg -n -C12 'useArchitectureTabMetrics\(' das-dashboard/srcRepository: singnet/das-toolbox
Length of output: 16748
🏁 Script executed:
#!/bin/bash
set -e
node - <<'JS'
function matchesRuntime(serviceKey, runtime) {
if (runtime?.service_command_label === serviceKey) return true;
const name = String(runtime?.container_name ?? "").toLowerCase();
if (!name) return false;
const markers = { db: "mongodb", redis: "redis", morkdb: "morkdb", "adapter-backend": "adapter" };
const marker = markers[serviceKey];
if (marker) {
if (!name.includes(marker)) return false;
const label = runtime?.service_command_label;
return !label || label === "db" || label === serviceKey;
}
return name.includes(String(serviceKey).toLowerCase());
}
function patchServicesWithRuntime(baseServices = [], runtimeServices = []) {
const used = new Set();
return baseServices.map((row) => {
const runtime = runtimeServices.find((entry) => {
const name = entry?.container_name;
return name && !used.has(name) && matchesRuntime(row.service_key, entry);
});
if (!runtime) return row;
used.add(runtime.container_name);
const running = String(runtime.status ?? "").toLowerCase() === "running";
return {
...row,
service_command_label: runtime.service_command_label ?? row.service_command_label,
display_name: runtime.service_name ?? row.display_name,
container_name: runtime.container_name,
image: runtime.image ?? row.image,
port: running ? (runtime.port ?? row.port) : row.port,
age: runtime.age ?? row.age,
cpu_percent: running ? runtime.cpu_percent ?? 0 : null,
memory_mb: running ? runtime.memory_mb ?? 0 : null,
status: runtime.status ?? row.status,
service_health: runtime.service_health ?? row.service_health,
is_running: running,
};
});
}
const base = [{
service_key: "query",
display_name: "Query",
cpu_percent: null,
memory_mb: null,
}];
const runtime = [{
service_command_label: "query",
container_name: "query-container",
service_name: "Query",
status: "running",
cpu_percent: 37,
memory_mb: 128,
}];
const liveRows = patchServicesWithRuntime(base, runtime);
const deletedOnChangedMachines = {};
delete deletedOnChangedMachines["host-1"];
const preservedOnChangedMachines = {
"host-1": patchServicesWithRuntime(base, liveRows["host-1"] ?? [])
};
console.log(JSON.stringify({
liveRows,
deletionResult: deletedOnChangedMachines,
proposedFixResult: preservedOnChangedMachines,
runtimePreserved: preservedOnChangedMachines["host-1"]?.[0]?.cpu_percent === 37 &&
preservedOnChangedMachines["host-1"]?.[0]?.memory_mb === 128
}, null, 2));
JSRepository: singnet/das-toolbox
Length of output: 679
🏁 Script executed:
#!/bin/bash
set -e
node - <<'JS'
function matchesRuntime(serviceKey, runtime) {
if (runtime?.service_command_label === serviceKey) return true;
const name = String(runtime?.container_name ?? "").toLowerCase();
if (!name) return false;
const markers = { db: "mongodb", redis: "redis", morkdb: "morkdb", "adapter-backend": "adapter" };
const marker = markers[serviceKey];
if (marker) {
if (!name.includes(marker)) return false;
const label = runtime?.service_command_label;
return !label || label === "db" || label === serviceKey;
}
return name.includes(String(serviceKey).toLowerCase());
}
function patchServicesWithRuntime(baseServices = [], runtimeServices = []) {
const used = new Set();
return baseServices.map((row) => {
const runtime = runtimeServices.find((entry) => {
const name = entry?.container_name;
return name && !used.has(name) && matchesRuntime(row.service_key, entry);
});
if (!runtime) return row;
used.add(runtime.container_name);
const running = String(runtime.status ?? "").toLowerCase() === "running";
return {
...row,
service_command_label: runtime.service_command_label ?? row.service_command_label,
display_name: runtime.service_name ?? row.display_name,
container_name: runtime.container_name,
image: runtime.image ?? row.image,
port: running ? (runtime.port ?? row.port) : row.port,
age: runtime.age ?? row.age,
cpu_percent: running ? runtime.cpu_percent ?? 0 : null,
memory_mb: running ? runtime.memory_mb ?? 0 : null,
status: runtime.status ?? row.status,
service_health: runtime.service_health ?? row.service_health,
is_running: running,
};
});
}
const base = [{
service_key: "query",
display_name: "Query",
cpu_percent: null,
memory_mb: null,
}];
const runtime = [{
service_command_label: "query",
container_name: "query-container",
service_name: "Query",
status: "running",
cpu_percent: 37,
memory_mb: 128,
}];
const liveRows = patchServicesWithRuntime(base, runtime);
const previousByHost = { "host-1": liveRows };
const nextByHost = {
...previousByHost,
"host-1": patchServicesWithRuntime(base, previousByHost["host-1"] ?? []),
};
console.log(JSON.stringify({
liveRows,
deletedOnChangedMachines: {},
nextByHost,
runtimePreserved: nextByHost["host-1"][0].cpu_percent === 37 &&
nextByHost["host-1"][0].memory_mb === 128
}, null, 2));
JSRepository: singnet/das-toolbox
Length of output: 822
🏁 Script executed:
#!/bin/bash
set -e
node - <<'JS'
function patchServicesWithRuntime(baseServices = [], runtimeServices = []) {
const used = new Set();
return baseServices.map((row) => {
const runtime = runtimeServices.find((entry) => {
const name = entry?.container_name;
return name && !used.has(name) && (
entry?.service_command_label === row.service_key ||
name.toLowerCase().includes(String(row.service_key).toLowerCase())
);
});
if (!runtime) return row;
used.add(runtime.container_name);
const running = String(runtime.status ?? "").toLowerCase() === "running";
return {
...row,
container_name: runtime.container_name,
cpu_percent: running ? runtime.cpu_percent ?? 0 : null,
memory_mb: running ? runtime.memory_mb ?? 0 : null,
status: runtime.status ?? row.status,
is_running: running,
};
});
}
function runExistingBranch(machines, streams, services) {
const next = { ...services };
for (const machine of machines) {
const host = machine.serverIp;
if (streams[host]) delete next[host];
}
return next;
}
function runProposedBranch(machines, streams, services) {
const next = { ...services };
for (const machine of machines) {
const host = machine.serverIp;
if (streams[host]) {
next[host] = patchServicesWithRuntime(
machine.services,
next[host] ?? []
);
}
}
return next;
}
const machinesBefore = [{
serverIp: "host-1",
services: [{ service_key: "query", display_name: "Query" }],
}];
const machinesAfter = [{
serverIp: "host-1",
services: [{ service_key: "query", display_name: "Query" }],
}];
const liveRows = [{
service_key: "query",
display_name: "Query",
container_name: "query-container",
status: "running",
cpu_percent: 37,
memory_mb: 128,
is_running: true,
}];
const streams = { "host-1": { open: true } };
const liveState = { "host-1": liveRows };
console.log(JSON.stringify({
machinesIdentityChanged: machinesBefore !== machinesAfter,
existingBranch: runExistingBranch(machinesAfter, streams, liveState),
proposedBranch: runProposedBranch(machinesAfter, streams, liveState),
}, null, 2));
JSRepository: singnet/das-toolbox
Length of output: 503
Preserve live rows when machines changes identity
When refreshDashboardState() replaces machines while a stream is open, lines 45–50 delete fleetServicesByHost[hostIp]. Line 100 then shows static machine.services until the next WebSocket payload. Preserve the existing rows and patch them against baseServices.
🔧 Proposed fix
if (fleetStreamsRef.current[hostIp]) {
- setFleetServicesByHost((prev) => {
- const next = { ...prev };
- delete next[hostIp];
- return next;
- });
+ setFleetServicesByHost((prev) => ({
+ ...prev,
+ [hostIp]: patchServicesWithRuntime(baseServices, prev[hostIp] ?? []),
+ }));
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| hostList.forEach((hostIp) => { | |
| const baseServices = machines.find((m) => m.serverIp === hostIp)?.services ?? []; | |
| baseServicesByHostRef.current[hostIp] = baseServices; | |
| if (fleetStreamsRef.current[hostIp]) { | |
| setFleetServicesByHost((prev) => { | |
| const next = { ...prev }; | |
| delete next[hostIp]; | |
| return next; | |
| }); | |
| return; | |
| } | |
| hostList.forEach((hostIp) => { | |
| const baseServices = machines.find((m) => m.serverIp === hostIp)?.services ?? []; | |
| baseServicesByHostRef.current[hostIp] = baseServices; | |
| if (fleetStreamsRef.current[hostIp]) { | |
| setFleetServicesByHost((prev) => ({ | |
| ...prev, | |
| [hostIp]: patchServicesWithRuntime(baseServices, prev[hostIp] ?? []), | |
| })); | |
| return; | |
| } |
🤖 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/hooks/useArchitectureTabMetrics.js` around lines 41 - 52,
Update the fleetStreamsRef.current[hostIp] branch in the hostList iteration to
preserve existing fleetServicesByHost rows when machines changes identity.
Instead of deleting the live rows, retain them and patch or reconcile them
against the newly computed baseServices, so rendering continues using live data
until the next WebSocket payload.
Comment on lines
109
to
+113
| return () => { | ||
| intentionalCloseRef.current = true; | ||
| stream.close(); | ||
| }; | ||
| }, [host, appendSnapshot]); | ||
| }, [host, baseServices, appendSnapshot]); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Both metrics hooks key a stream effect on an unmemoized array. The shared root cause is the same: an array prop (baseServices, machines) changes identity on every render, and each hook drives WebSocket lifecycle and per-host service state from that identity. Derive a stable dependency key from the configured service set in both hooks.
das-dashboard/src/hooks/useServerTabMetrics.js#L109-L113: replace thebaseServicesdependency with a stable key string and read the array through a ref, so the socket does not close and reopen on every render.das-dashboard/src/hooks/useArchitectureTabMetrics.js#L41-L52: derive the effect dependency from a stable host and service key instead of themachinesarray identity, and patch the existing rows in the stream-reuse branch instead of deleting the host entry.
📍 Affects 2 files
das-dashboard/src/hooks/useServerTabMetrics.js#L109-L113(this comment)das-dashboard/src/hooks/useArchitectureTabMetrics.js#L41-L52
🤖 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/hooks/useServerTabMetrics.js` around lines 109 - 113, In
das-dashboard/src/hooks/useServerTabMetrics.js (lines 109-113), derive a stable
service-set key for the stream effect, replace the baseServices dependency with
that key, and read the current array through a ref so renders do not restart the
socket. In das-dashboard/src/hooks/useArchitectureTabMetrics.js (lines 41-52),
key the effect by stable host and service values rather than machines identity,
and update existing rows in the stream-reuse branch instead of deleting the host
entry.
| useQueryExecutionContext | ||
| } from "../../components/global_providers/QueryExecutionProvider"; | ||
| import { useQueryParameters } from "../../hooks/useQueryParameters"; | ||
| import { ApiErrorNotice } from "../../components/common/ApiErrorNotice"; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 '\buseQueryParameters\b' das-dashboard/src
fd -t f -i 'useQueryParameters.*' das-dashboard/srcRepository: singnet/das-toolbox
Length of output: 5570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,80p' das-dashboard/src/pages/query/QueryPage.jsx
printf '\nHook export:\n'
sed -n '185,200p' das-dashboard/src/hooks/useQueryParameters.jsxRepository: singnet/das-toolbox
Length of output: 2661
Restore the useQueryParameters import.
QueryPageContent calls useQueryParameters(), but QueryPage.jsx does not import it. Rendering the Query page therefore raises a ReferenceError.
🤖 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/query/QueryPage.jsx` at line 14, Restore the missing
useQueryParameters import in QueryPage.jsx so QueryPageContent can resolve its
hook call without a ReferenceError.
Comment on lines
+70
to
+72
| export function formatMemoryCell(agent) { | ||
| return agent.memory_mb == null ? "-" : `${Number(agent.memory_mb).toFixed(2)} GB`; | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Convert megabytes before rendering gigabytes.
memory_mb is rendered directly with a GB suffix. For example, 512 MB is displayed as 512.00 GB. Divide the value by 1024 before formatting it as GB, or change the suffix to MB.
Proposed fix
export function formatMemoryCell(agent) {
- return agent.memory_mb == null ? "-" : `${Number(agent.memory_mb).toFixed(2)} GB`;
+ return agent.memory_mb == null ? "-" : `${(Number(agent.memory_mb) / 1024).toFixed(2)} GB`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function formatMemoryCell(agent) { | |
| return agent.memory_mb == null ? "-" : `${Number(agent.memory_mb).toFixed(2)} GB`; | |
| } | |
| export function formatMemoryCell(agent) { | |
| return agent.memory_mb == null ? "-" : `${(Number(agent.memory_mb) / 1024).toFixed(2)} GB`; | |
| } |
🤖 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/utils/serviceRows.js` around lines 70 - 72, Update
formatMemoryCell to convert agent.memory_mb from megabytes to gigabytes by
dividing the numeric value by 1024 before applying toFixed(2), while preserving
the null placeholder and GB suffix.
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.
No description provided.