Loop() -> Sync()

[dull furnace]

@granite tapir another async request... How hard is it to add standard sync() behavior in a core object? I'd want to control which functions are lazy vs. which force a sync...

[versed flume]

I think this is done by the daqgl schema definition, it's a field / option you can add to a function?

[dull furnace]

I see Syncer[] for the actual sync() function

But not sure if there are gotchas or best practices

[granite tapir]

Yeah you register it like this: https://github.com/sipsma/dagger/blob/ca58af8d8c615586caa2043a3afccc20dfff4799/core/schema/container.go#L48-L48

The object has to implement that interface. It's sorta tied to buildkit internal stuff rn but you can just ignore all that and have the Evaluate method return nil, nil and nothing else. Sync will still result in the dagql query getting synchronously evaluated

[dull furnace]

ah cool, I was just worrying about the buildkit in the return type...

How do I control which of my functions are lazy vs. not?

I guess nothing special then. Just do that in my own logic

(orthogonal to Syncer() ?)

Or rather, Syncer is a layer on top of my logic

[granite tapir]

Yeah currently you trigger synchronous query evaluation by hitting a scalar return type or via a sync call

[dull furnace]

If I use my own explicit Sync function vs Syncer[], what do I miss out on? Might it break users in weird ways?

Oh I see, the same sync/async evaluation rules that apply to modules, also apply to core functions

[granite tapir]

Well you need the SDKs to realize that it is synchronous (i.e. in Go .Sync takes a ctx and returns an error, it impacts the sdk behavior). The way Syncer works is by returning an ID for the object, which is technically a scalar and thus trigger gql evaluation, but then the SDKs see that it returns an ID and make the signature the actual object rather than the ID type. If that's making any sense 😅

[dull furnace]

At the moment I feel like I mess with the flow a little bit, because eg. if you call dag.Llm().Loop() (no error check, no context) it actually makes API queries and will routinely fail with an error...

So in addition to using Syncer, I should also be careful to respect the convention - always be lazy when you return an object; always be synchornous when you return a scalar. right?

Another reason I'm hesitating, is that Loop() (my current sync() equivalent) takes an optional argument (maxLoops)

[granite tapir]

I wouldn't actually say that's a hard rule necessarily. You inherently get laziness from the fact that queries aren't evaluated until they reach a scalar (and then after that every selection in the query is cached in dagql).

The only consideration I usually have to make around this when implementing APIs is trying to max cacheability. So like one pattern would be "maximum laziness until scalar reached" and the other would be "do as much work as possible in each API". The tradeoff is that sometimes by doing more work in each API, you can get better caching because it's scoped to one part of the work, rather than scoped to the cache digest of the whole chain.

I'm realized as I type this out it's very hard to explain clearly in words Honestly I wouldn't worry too much about it yet, just do what feels clean/right in the implementation and if it makes sense to re-arrange where work happens later then we can do so then

[granite tapir]

What's the return value of Loop, an Llm object?

[dull furnace]

yes

But how come my Loop() implementation is successfully un-lazying then? 🤔

[granite tapir]

Someone must be making a call that has .Loop() somewhere in the chain and ends in either a scalar or .Sync. Once that's hit, then the whole query is evaluated, including your .Loop implementation

Or I suppose if you are passing around the LLM object between function calls, it has to be evaluated because objects are passed around by ID, and .id returns a scalar and thus results in evaluation. I suppose that's a more sneaky/less obvious source of synchronous evaluation

[dull furnace]

Mmmm but is there any reason passing a Llm object around causes more evaluation than passing around a Container?

[granite tapir]

Well yeah, because Container is based on buildkit LLB, which brings a whole new level of laziness. Even when e.g. withExec is synchronously evaluated it's actually just putting together LLB which doesn't get evaluated until later APIs like .stdout.

But you are aren't running stuff in buildkit w/ LLB so you lose that level of laziness

[dull furnace]

@fresh pendant if loop() were automatically called on, say, lastReply() , history(), and all the functions to get state back out... But withPrompt() stayed lazy, would that feel right to you?

[granite tapir]

I think I see what you were saying earlier more now btw; if you don't want .Loop to get evaluated when an Llm is passed around between modules then you'd need the implementation itself to be lazy. That would be closer to the behavior of e.g. Container

[dull furnace]

Ok cool, I just conducted a very scientific survey, and all 2 of the respondents said:

• sync() is more consistent than loop()
• +1 to matching the usual convention, so you can easily guess what will be lazy and what won't
• arguments to loop() should be arguments to llm() instead

Gonna try that now

[fresh pendant]

Yes absolutely

[dull furnace]

@granite tapir quick question, my Sync() takes a dagql.Server in argument (I need it for introspection). My other functions (like lastReply, history) don't - but now I want them to call Sync(). Before I persist the dagql.Server in my Llm type, I was wondering if I could get it from core.Query? Since I already carry that

[granite tapir]

I'd avoid persisting dagql.Server in the Llm type, that's what caused that bug I looked at the other week (the objects are cached and so you can end up in weird situations where you get a cached dagql.Server from a cache object that's not the one you wanted)

Using core.Query sounds reasonable, then you can make sure you are always getting the one you expected without thinking through caching

[dull furnace]

but I don't know how to get the dagql.Server from the core.Query

[granite tapir]

Oh, it would be extremely similar to what you added earlier but just return the server for the current client rather than for the main one

[dull furnace]

oh I see

right I guess I added that dag field 🙂

In that case I won't need to return a new context right?

[granite tapir]

nope the ctx you already have is the right one for that case

[dull furnace]

(ignore my last question, brainfart)

time for a break soon 😅

[dull furnace]

😭

✔ llm | with-prompt "hello how are you?" | history 0.0s
🧑 💬hello how are you?
🤖 💬Hello! I'm just a program, so I don't have feelings, but I'm here and ready to help you. How can I assist you today?

✔ llm | with-prompt "hello how are you?" | sync | history 0.7s
🧑 💬hello how are you?
🤖 💬Hello! I'm here to help, so feel free to ask me anything you need assistance with. How can I help you today?
🤖 💬Hello! I'm here to help, so feel free to ask any questions you have. How can I assist you today?
│🤖 Hello! I'm here to help, so feel free to ask any questions you have. How can I assist you today?

✔ llm | with-prompt "hello how are you?" | sync | sync | history 1.6s
🧑 💬hello how are you?
🤖 💬Hello! I'm here to help, so feel free to ask me anything you need assistance with. How can I help you today?
🤖 💬Hello! I'm here to help, so feel free to ask me anything you need assistance with. How can I help you today?
🤖 💬Hello! I'm here to help, so feel free to ask if you have any questions or need assistance with anything. How can I help you today?
│🤖 Hello! I'm here to help, so feel free to ask if you have any questions or need assistance with anything. How can I help you today?

OK I don't know how to handle that. Not even sure where the root cause of the issue is

Actually I think I do know

[dull furnace]

@fresh pendant @granite tapir I think something I did might have broken custom objects in the state... Llm.withDirectory() still works, but Llm.withToyWorkspace() doesn't, it sees not tools... bisecting now

I worry it's something to do with contexts like last time..

[fresh pendant]

looking at the commit... so that would mean srv.clientFromContext(ctx).dag isn't the correct context?

[dull furnace]

Yup, it looks like something in this commit breaks module objects (and only module objects): https://github.com/shykes/dagger/commit/a319e650c4c31b6d9faa39918fceff8765e0a4f9

https://tenor.com/view/baby-one-more-time-britney-spears-gif-1874225940573415150

@granite tapir I think maybe getting the dagql.Server in the way I do it, has unintended consequences...

OK I fixed it by plumbing through arguments directly