Skip to content

add observation skill container - #3601

Open
paul-nechifor wants to merge 1 commit into
mainfrom
paul/fix/observe-skill
Open

add observation skill container#3601
paul-nechifor wants to merge 1 commit into
mainfrom
paul/fix/observe-skill

Conversation

@paul-nechifor

Copy link
Copy Markdown
Contributor

Closes DIM-1505

Problem

  • Observation skill is broken when running dimsim simulator
  • We have too many independent observation skills.

Solution

  • Unify all observation skills into one.
  • This way, it no longer goes through each sim, just listen to /color_image

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change replaces robot-specific cached-image observation with a shared stream-backed observation skill and wires it into the affected robot agents. A focused runtime check confirmed that ObserveSkill reports transport and decoding failures as camera-frame timeouts, which can lead users to diagnose an inactive camera instead of the underlying stream failure.

Confidence Score: 4/5

Not safe to merge until non-timeout image-stream failures are distinguished from genuine frame timeouts.

A focused executable harness injected a concrete stream failure and observed the shared observation skill convert it into a timeout response, directly demonstrating the incorrect failure behavior.

Files Needing Attention: dimos/agents/skills/observe_skill.py needs exception handling that catches only the stream timeout condition and preserves or separately reports other stream errors.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding and linked it to the corresponding review comment.
  • The contract-validation work showed that before capture, color_image.get_next(timeout=5.0) raised RuntimeError, and after capture, skill.observe() returned a timeout result with No camera frame received.
  • No production code changes were made; the reviewer recommended narrowing the handler to the actual timeout exception and adding this regression case to the permanent test suite.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 ObserveSkill reports non-timeout stream failures as camera timeouts

    • Bug
      • ObserveSkill.observe() returns EXECUTION_TIMEOUT when color_image.get_next() raises a non-timeout RuntimeError, thereby presenting a transport/decoding failure as a missing camera frame.
    • Cause
      • The broad except Exception at dimos/agents/skills/observe_skill.py:37 catches every stream exception and unconditionally builds the timeout result at lines 38-41.
    • Fix
      • Catch only the concrete timeout exception emitted by the stream implementation; allow transport/decoding exceptions to propagate or map them to a distinct execution-failure result. Add the injected RuntimeError regression test.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "add observation skill container" | Re-trigger Greptile

Comment on lines +37 to +41
except Exception:
return SkillResult.fail(
"EXECUTION_TIMEOUT",
f"No camera frame received within {self._frame_timeout} seconds; "
"the camera may not be running.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stream failures are mislabeled as timeouts

The broad except Exception converts transport, decoding, and other image-stream failures into EXECUTION_TIMEOUT and tells the caller that no camera frame arrived. A reproduced RuntimeError from color_image.get_next() takes this path, so agents and operators cannot distinguish a broken stream from an inactive camera. Catch only the concrete timeout exception, and propagate or return a distinct execution failure for other errors.

Artifacts

Focused non-timeout ObserveSkill harness source

  • The authored harness injects a RuntimeError through the color-image stream and compares direct stream behavior with ObserveSkill behavior, proving the masking path.

Direct stream RuntimeError capture

  • The direct color-image stream call raised the injected RuntimeError and exited successfully, establishing the non-timeout input condition.

ObserveSkill masks RuntimeError as timeout capture

  • Calling ObserveSkill after the identical RuntimeError injection returned EXECUTION_TIMEOUT and a no-camera-frame message, confirming error masking.

View artifacts

T-Rex Ran code and verified through T-Rex

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
4507 1 4506 69
View the top 1 failed test(s) by shortest run time
dimos.cli.test_cli_startup::test_help_startup_time
Stack Traces | 8.23s run time
def test_help_startup_time() -> None:
        """`dimos --help` must finish in under {HELP_TIMEOUT_SECONDS}s."""
        start = time.monotonic()
        result = subprocess.run(
            [sys.executable, "-m", "dimos.cli.dimos", "--help"],
            capture_output=True,
            text=True,
            timeout=HELP_TIMEOUT_SECONDS + 5,  # hard kill safety margin
        )
        elapsed = time.monotonic() - start
        assert result.returncode == 0, f"dimos --help failed:\n{result.stderr}"
>       assert elapsed < HELP_TIMEOUT_SECONDS, (
            f"dimos --help took {elapsed:.1f}s (limit: {HELP_TIMEOUT_SECONDS}s). "
            f"Check for heavy imports in the CLI entrypoint or GlobalConfig."
        )
E       AssertionError: dimos --help took 8.2s (limit: 8s). Check for heavy imports in the CLI entrypoint or GlobalConfig.
E       assert 8.230978411000024 < 8

elapsed    = 8.230978411000024
result     = CompletedProcess(args=['.../dimos/dimos/.venv/bin/python', '-m', 'dimos.cli.dimos', '--help'], returncod...x1b[0m\n\x1b[2m╰──────────────────────────────────────────────────────────────────────────────╯\x1b[0m\n\n", stderr='')
start      = 506.307514042

dimos/cli/test_cli_startup.py:75: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@mintlify

mintlify Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
dimensional 🟢 Ready View Preview Aug 21, 2026, 6:08 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant