#Default args applied twice?
1 messages ยท Page 1 of 1 (latest)
LLM follow-up:
Looking at all this, I believe the answer is:
The default application in
setCallInputs()IS mostly redundant for GraphQL calls, but it's kept for:
- Safety/defensive programming: Even though dagql should apply defaults, having a fallback ensures things work correctly
- Possible edge cases: There might be internal call paths we haven't identified where defaults aren't pre-applied
- Historical reasons: This code might predate the dagql default handling
For LocalDefaults specifically, your approach is correct:
- Add them to
FieldSpecso dagql applies them for GraphQL calls- Keep them in
setCallInputsas a fallback for any edge casesThe redundancy doesn't hurt (it just checks
hasArg[name]and skips), and it provides defense-in-depth.
FYI <@&946480760016207902>
setCallInputs handles contextual args. Before setCallInputs, the argument is just metadata essentially (i.e. "a directory loaded from this path") and after it is the "real" directory
Oh wait, that's sort of outdated, I forgot I moved the contextual loading to CacheConfigForCall
But it also seems to apply regular defaults at the end? https://github.com/dagger/dagger/blob/main/core/modfunc.go#L190-L193
(context for all this is I'm trying to hook up my "local defaults" feature as cleanly as possible)
Current approach is to keep the local defaults (really I should call them "user defaults") separate as long as possible, and merge them as late / lazily as possible
Initially I just walked the typedefs at module loading ModuleSource.asModule and just squashed DefaultValue in the typedefs
Now I'm flipping it around, no squashing, I'm adding dynamic / lazy LocalDefaults() functions all the way down - in ModuleSource, Module, ModFunction
It feels cleaner and more robust, but, now the last mile is my problem ... ๐
As a nice side effect, I could even make caching of the .env configurable, so you could edit the .env and have the ongoing sessions apply it live at the next call ๐
Yeah I actually can't see why that would be necessary to do there anymore. Feels like it might be leftover from older times? I'd try removing that section and see if anything breaks ๐
I do recall there being some very subtle issues around zero values and defaults in certain SDKs, but I don't think this is related
you're not selling it @scenic sequoia ๐
"go ahead, remove it, it's totally fine, I think?"
"I see a bunch skeletons next to it, but that's probably unrelated"
Haha well my philosophy is that the tests are our source of truth. If they pass, then ship it. If we realize later that it did break something that tests missed, then we fix it and add the missing tests
Ok cool
@scenic sequoia while I have you: I'm actually wrangling an issue in where to patch...
I think I need to stay closer to how contextual args are handled, since just like contextual args, I need to resolve actual objects
I was naively patching ModFunc.FieldSpec() but it turns out, it's too early to make dagql calls of my own... which I need to resolve the objects...
So my question is: how are contextual args handled at schema construction? I don't see them referenced at all in Function.FieldSpec. But surely it's used to at least mark the field as optional, otherwise the client would refuse to make the call, complaining about missing argument?
Yeah you will want to integrate with this code: https://github.com/sipsma/dagger/blob/c8368d6abe2fa4b3d74db4c7484f24ad5fc6396f/core/modfunc.go#L204-L204
It's in a function that is determining the cache key for the function call because we need to make sure that the loaded arg values are incorporated into the cache key
Yes, at some point we mark it as optional or with a default value, finding exactly where
I can investigate (with my LLM buddy)
Knowing I'm on the right track is super helpful
I'm asking the LLM - study how contextual args / defaultpath are handled, let's piggy back on that
I'm starting to get the hand of this LLM-assisted codebase spelunking ๐
Very different way of using it than vibecoding
smart
Totally, as very loose anecdata I actually had a very good experience w/ the Gemini CLI when it came to answering questions about codebases specifically (whereas it is really bad at editing vs. Claude/Codex)
@scenic sequoia is there any scenario where Function.FieldSpec might be called in the context of a live dagql server?
Thanks LLM, I'll take it from here ๐
Not sure what you mean by live, like at a time after the types have been loaded?
Yeah, the LLM is trying to handle both cases in FieldSpec: if a dagql server is available, resolve it right away. If not, mark it for resolution later (in CacheConfigForCall). I feel like that's overkill, better to always do it in CacheConfigForCall. Just checking if maybe there's a use to also handling resolution in FieldSpec()
nevermind I think the LLM found the answer on its own
"my" implementation -> way to take accountability ๐
- "wow you really messed up"
- "you just gave me this code 30 seconds ago!" (โฏยฐโกยฐ๏ผโฏ๏ธต โปโโป
OK let's see if all this works. Thank you!