Skip to content

Comments

Omi#51

Open
thestumonkey wants to merge 13 commits intomycelia-tech:sky-devfrom
Ushadow-io:omi
Open

Omi#51
thestumonkey wants to merge 13 commits intomycelia-tech:sky-devfrom
Ushadow-io:omi

Conversation

@thestumonkey
Copy link
Contributor

  • optimised pipeline page with less agressive polling
  • fixed conversation display on pipeline page
  • added support for omi-opus from mobile ushadow
  • better logging

@thestumonkey
Copy link
Contributor Author

This should now work with omi necklace and ushadow mobile streaming https://github.com/Ushadow-io/Ushadow

alias: "p",
type: "number",
describe: "Port to serve on.",
default: 5173,
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you sure that it's safe to change ports?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Bakuutin does this port important here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I didn't mean to change this here, however on my ushadow instance I have changed the backend port to 8888. In general AFAIK 5173 gtends to be used for the front end for hot reloads, not a backend that tends to be in the 8000's. 5173 ends up conflicting with lots of things, it's not a good default port to ship/

Copy link
Collaborator

Choose a reason for hiding this comment

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

so let's check if 5173 is used in the repo. I'm sure it is. so don't change port here in one place. and better to not change them in unrelated PR. let's make separate PR with ports migration and double check that all works

@Bakuutin
Copy link
Collaborator

Thanks a lot for the contribution @thestumonkey!

I wanted to flag a couple of implementation details. These are things I can fix on my side if you prefer, but I wanted to call them out so we’re aligned going forward.

For Mongo access, we try to avoid using getRootDB() or calling the collection directly, e.g.:

db.collection("audio_chunks").countDocuments({ vad: null })

Instead, please go through the resource abstraction, like:

await mongo({
  action: "count",
  collection: "audio_chunks",
  query: {
    vad: null,
  },
})

This gives us proper traceability and access logs, which is important for auditing and debugging. Direct getRootDB() usage will also fail at runtime at some point in future releases, since it’s something we’re actively steering away from.

Also, in AudioPipelinePage.tsx, we can use a WebSocket subscription instead of periodic polling, there is an example in frontend/src/lib/jobs.ts how to subscribe to database changes.

Let me know whether you’d like to adjust these parts yourself, or if you’d rather I handle it.

And, again, thanks for the contribution ❤️

@skywinder skywinder changed the base branch from dev to sky-dev January 30, 2026 09:14
@skywinder
Copy link
Collaborator

@thestumonkey can you explain, how can I test it now?

@thestumonkey
Copy link
Contributor Author

@Bakuutin
For the audiopipeline, I agree on the websocket solution, however this is a quick fix as the current deployed implmentation is awful, so what we have here is an improvement and I don't think it should block the PR.

I can change the getDBroot thing, however
I want to talk to you aqbout this mongo resource abstraction as I am used to using endpoints and routers. It is very difficult to see what is going on in devtools when the application is running as everything is a request to the mongo erndpoint from the front end.
Is it not preferreable to make some proper routers for key resources?

@thestumonkey
Copy link
Contributor Author

@skywinder I have been waiting for you on discord for some time to get you up and running? We were going to do this yesterday?

Applied PR feedback from @Bakuutin on mycelia-tech#51:
- Replaced getRootDB() with getMongoResource(auth)
- Converted all direct database calls to use mongo resource abstraction
- Ensures proper access logging and traceability
- Maintains compatibility with future releases

All database operations now go through the resource layer:
- db.collection().countDocuments() → mongo({ action: "count", ... })
- db.collection().find().toArray() → mongo({ action: "find", ... })

This provides:
- Access control and audit logging
- Centralized permission enforcement
- Consistent security checks

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@thestumonkey
Copy link
Contributor Author

updated mongo db as requested. failing test is not due to this PR

alias: "p",
type: "number",
describe: "Port to serve on.",
default: 5173,
Copy link
Collaborator

Choose a reason for hiding this comment

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

so let's check if 5173 is used in the repo. I'm sure it is. so don't change port here in one place. and better to not change them in unrelated PR. let's make separate PR with ports migration and double check that all works

@skywinder
Copy link
Collaborator

the rest looks fine. But last thing that I want - is to break something with this PR. so better be safe.

@skywinder
Copy link
Collaborator

@thestumonkey ping on this. Would love to merge as soon as you revert the ports changes.

@thestumonkey
Copy link
Contributor Author

Ok changed back to 5173 to reduce risk for now, although this should still get changed at some point imo

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.

3 participants