Skip to content

feat: default value for logos chat client#151

Open
kaichaosun wants to merge 2 commits into
mainfrom
chat-client-config
Open

feat: default value for logos chat client#151
kaichaosun wants to merge 2 commits into
mainfrom
chat-client-config

Conversation

@kaichaosun

@kaichaosun kaichaosun commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Adds an opinionated LogosChatClient preconfigured with the Logos stack.

Changes:

  • LogosChatClient with open(transport, db_path, db_key, registry_url).
    db_path/db_key are caller-supplied;
    registry_url overrides the preconfigured endpoint (placeholder until deployed).
  • chat-cli builds via LogosChatClient::open, so it always uses the registry;
    --registry-url is now an endpoint override.
    Removed the dead run_logos_delivery.

Fix #126

Comment thread crates/client/src/builder.rs
Comment thread crates/client/src/builder.rs Outdated
Comment thread crates/client/src/builder.rs Outdated
Comment thread crates/client/src/builder.rs Outdated
Comment thread crates/client/src/logos.rs
Comment thread crates/client/src/logos.rs
Comment thread crates/client/src/logos.rs
Comment thread crates/client/examples/message-exchange/main.rs
@kaichaosun kaichaosun requested review from jazzz and osmaczko June 26, 2026 03:02

@jazzz jazzz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My previous concerns still remain.

  • Blocker: Changing ChatClientBuilder to require an ephemeral base configuration has negative impacts on code maintenance and will be hard to undo in the future.
    • This change does not seem necessary at this time, as it does not add any required functionality while removing beneficial properties.
    • The motivation of removing a few lines of code doesn't justify the increased maintenance costs in developer applications.
  • Forcing Ephemeral be the root of the builder pathway introduces fragmentation, which will become developer pain.
    • The focus should be on creating an API that we want in the future( rather that what is available now), and working towards it with little to no breaking code changes in demos and developer applications.

@@ -0,0 +1,61 @@
//! The opinionated Logos client.
//!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Having LogosChatClient located within the "Generic client" seems like it will be full of friction as the objects have separate concerns.

the "GenericClient will tend to be more generic/abstract" and the LogosChatClient will tend towards importing specific implementations and services.

[Sand] LogosChatClient would benefit from being a separate crate, to keep imports separated. That way it can import LogosCore specific implementations and GenericClient could remain free of these implementation details

@kaichaosun kaichaosun Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Split LogosChatClient to its own crate looks too early, we can do it later.
The landscape may also change as we move to production ready, and move ephemeral/in memory dependencies to test.

@kaichaosun kaichaosun force-pushed the chat-client-config branch from 616df84 to e2c635b Compare June 29, 2026 07:53
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.

Create a LogosChatClient, which is default prefconfigured with Logos Specific services

2 participants