https://www.boundaryml.com logo
Powered by
windows path handling
# contributing
j
I've been having codex go through and write a detailed report of various potential windows issues with a particular focus on path handling. It's found quite a few areas where bugs could surface. Is this the type of thing that should be documented in a design doc? I'm planning on going through each potentially problematic surface, reading the code around it and having codex explain it's purpose to me then coming up with an appropriate plan to fix it with future maintainability in mind. I'm happy to document the purpose/intent/decision making behind each of these changes for future reference as well as coming up with a brief guide for how to avoid readding these types of buggy surfaces in the future if you think that'd be useful for the project. Also plan on adding/fixing tests so the current sketchy surfaces don't later regress. With agents my hope is that testing + examples of good path handling should help nudge most new code towards better path handling going forward even without providing them with explicit guidance. If you do want me to document this stuff you'll just have to point me to where/how you want it documented alongside any guidance for doing so and then I'll figure it all out. I'm still early in this process mostly just interrogating codex for good concrete info across the entire project and I don't plan on actually implementing the wide range of fixes until after I've come back with a proposed plan to share with you. However if I'm going to be documenting the decision making involved it's best if I do that while making the actual decisions instead of adding it later. @baml-sam
b
We can use notion for discussion! If you give me your address I can set up a page for us to use to sync
j
Sounds good, do I need to make an account or anything? I've never used notion before.
b
Email address will be fine!
j
Messaged you it 🙂
b
Got your DM, invited!
j
holy notion's signup flow UI is super bugged
I eventually got through it but idk what it did to get through it
It did not want to work for me at all lol
I see the shared doc now though so it's working at least
Someone for sure vibe coded that entire flow, that was a wild experience lmao
b
😅
j
I've added 3 examples of what I'm doing so far for documentation @baml-sam Let me know if you want anything formatted or collated differently. Or if you want me to approach the issues a different way. These are the types of issues Codex has been showing me and they do appear to be largely valid issues even if the circumstances where they appear can be somewhat niche.
(22 more to go 😂)
b
we should maybe replace 
walkdir
with 
glob
which i think respects gitignores - i feel like there was a reason we were doing 
walkdir
directly though. maybe something around 
baml.toml
discovery or 
baml_src/
dir handling
re VFS paths and monaco paths - i agree that the current impl is messy, but: 1. what bugs are we experiencing today because of this? (i don't remember what the issue you ran into was) 2. are the notion docs your suggestions, or are they codex output? changes how i'm reviewing them 3. i kinda want type safe guarantees around what types contain raw windows paths vs raw monaco paths vs baml-internal storage. i know vfs' API requires us to pass in 
std::path::Path
but maybe we could add a type-safe wrapper around it? but also whether we do this depends on (1). all of the currently proposed patterns do nothing to prevent agents from making these same mistakes over and over again, which is why i'm flagging it.
j
Notion docs are mostly codex summaries that I'm directing and asking questions about and collating. I started by having it do a wide pass and documenting that then iterating over it to make it more sensible and remove any obvious false positives. Then I've set it up so I can step through each issue one by one and interrogate codex about its findings until I actually get the gist of what it's saying and it's suggestions make sense about the code they're describing. It's not just jesus take the wheel but it's mostly codex words not mine I'm just making sure they make sense. The only part so far that's my words is the My take: comment in 03. Oh and the titles. I rewrite those myself it gives me awful titles lol. Not all of these should be actioned on and I imagine for some they'll need adjustment to be actioned on as it's obviously going to miss some nuance here. But also because I don't know the history of why things were done I also don't really know which things are worth fixing vs not hence doing it this way. If these were straightforward bug fixes I'd just fix them and make a PR but most of this stuff is kinda subjective and nuanced which is why I'm documenting instead of fixing to start with. If the summaries are too long to quickly review for you I can cut them down in size and make them more concise. These first attempts were just me trying to keep them as concise as possible while not throwing away information and hard citations.
The bugs experienced today actually are weird edge case windows behaviour. Some stuff like not handling \ or D: and case sensitivity seem to actually have concrete bugs behind them. For example even with the hacky windows patch I'm using to get my playground working the open in browser button is still broken.
And the hacky patch I have that gets it working on my machine right now I believe doesn't actually fully solve all bugs. I'd have to double check which exactly still get through but I think there's another edge case or two that aren't handled.
For the preventing agents from making the mistakes part, my hope is that tests should at least make the core windows path behaviour clear and that prior examples of proper path handling in the source should nudge agents towards a correct solution. With testing ideally you can provide the windows path logic and edge cases directly to what an agent interacts with to avoid this stuff coming back. Although who knows, maybe agents will be too stubborn lol
And yeah writing brand new modules without direct interaction with other code might let this stuff slip through. I'm not sure how to solve that problem though.
Honestly all of this makes me want to delete Windows from society 😭
Also idm if you think it's more effort advising me around which things to fix compared to just fixing them yourselves. If you wanna go find and fix all the Windows bugs yourself and not have to deal with explaining context to someone unfamiliar that's all good. Or if you wanted I could give you the document listing all of Codex's prior findings. But I'm more than happy to help if it's actually helpful for you guys.
b
Sorry about the delay, was out sick for a while last week!
I'm more than happy to let you drive the Windows fixes, it's important to us to have all the tooling working well on Windows but we all have stuff on our plates right now
j
Sounds like I'm going about it in the right way to be actually helpful then lol I've finished my main voice loop for reframeweb so I have plenty of time to go figure out these bugs now, I'll implement that one fix and document the rest. 🙂
b
maybe lgtm for https://app.notion.com/p/gloochat/03-CLI-discovery-doesn-t-prune-excluded-directories-39dbb2d26216803e9b38c2f57b1e9824?source=copy_link? but i'm wondering if we should use the 
glob
crate (not totally sure if it's API-compatible with how we use 
walkdir
)
j
I'll ask codex for 03 and see what it thinks
b
re (3), i'd rather delegate to 
.gitignore
and 
.jjignore
etc
rather than hardcoding exclusions
and then https://github.com/BoundaryML/baml/pull/3997 should be converted to passing around 
QualifiedTypeName
instead of stringifying symbols everywhere
i think that's everything you were waiting for me to review?
did i miss anything?
j
Nope that's it so far. Yep that does sound more sensible for 03, I'll try get codex to get glob working and tell me what it might break by doing so instead of simply hoping it's fine 😂
Made a PR with the first 3 fixes in it. https://github.com/BoundaryML/baml/pull/4103 Also up to 10 in notion so far.
b
left comments and have coopted @avery (BAML) to help out with reviewing the other docs in notion
a
hey @jubjub just went over issues 4-10 in notion and triaged them/your proposed solutions
several of them were only issues in the old 
engine/
and 
typescript/
dirs, which are for the old DSL language that we're going to be deprecating soon
our bad for not letting you know these are getting deprecated sorry about that, 
baml_language/
and 
typescript2/
are the new toolchain
the other ones look good and i think the proposed solutions make sense and should fix the issues
j
Are they truly unused code paths? The engine directory seems to still be relevant and causing bugs. Idk maybe I'm wrong, I'll test it later.
I originally excluded those when doing the pass but codex seemed to think the code was still being used outside of those projects. It might have been wrong but I interrogated it a little so if it is wrong it was being really stubborn about it 😂
a
it's causing bugs for the old LSP and compiler
just for the old DSL
there are still people actively using that but most of them are on macs; feel free to fix the issues it's just that we're probably not going to prioritize anything in those directories as much
j
Okay yeah that makes sense. I'll get codex to make the judgement with this context and see what it thinks. Ignoring bugs is the cheapest fix so that is an attractive idea 😂
Okay yeah looks like we can just ignore the legacy bugs. 06, 08 and 09 can just be ignored.
I added Resolution pages for them and they're linked at the start of the documentation in notion so it's clear what happened and that they're not being fixed when you open 06, 08 or 09.
I'll bake that info into the wider findings document I have and see how many bugs that assumption can help us "solve"
Been doing other projects for the past week but I already went through and found all the situations where vfs/physical paths were being mixed up a week or so ago and now today I'm filling in the last group of findings. I'll also work on fixing 04, 05, 07 and make a PR for them later today or tomorrow depending on when I have time. I'm also working on making a custom receiver for a emergency button my grandma uses (so I can add phone notifications) so I need to spend some time doing packet capture and reversing the protocol today but after that I should have nothing else to do. I imagine I'll be able to finish the last of these fixes this week 🙂 , I've done all the ground work now investigating bugs and figuring out how each part of the project interacts with each other so it's just verification/implementation and then it should all be working on Windows. Was the last github PR successful? It failed CI because of the crate name but I think it might need to be ran through CI again now that I fixed it.
Finished adding all my findings to notion and also created a concise summary page explaining why they kept popping up and how people can avoid adding new path bugs in the future.
Once that first batch PR passes CI and is accepted I'll add the 04, 05 and 07 fixes in another PR. All ready to go, one of the fixes just depends on code added in the first batch so it's cleaner to wait. I had Codex have a go at doing the 3 fixes at once and it actually wasn't as much work to refine them as I expected because they're smaller/easier fixes 🙂
a
sam approved that last PR, just ran CI and it's failing some stuff
if u can fix that i can just merge it soon
looks to be failing precommit, would recommend installing prek
j
Pushed a fix now. Hopefully this is the last change needed lol.
Although codex was lazy and didn't want to install prek so it might have missed something.
It eventually got prek to work but it can't verify Linux CI on my machine so it might still fail who knows lol
And yep an issue came up that I wasn't able to test for on my machine. Also typescript dependency stuff drifted. Fixing both now.
a
ping me if i ever dont hit the rerun button on the CI
for some reason it isnt auto running on your pr
and makes me approve it manually
j
I think it's just a permissions thing
Well at least rust is passing now but typescript still has one failure 😭
Okay I think this should finally have no blockers left @avery (BAML)
@avery (BAML) the PR seems to be ready now other than the 2 permissions related failures, idk how those are meant to be handled
@avery (BAML) I fixed the thing code rabbit was complaining about and now it needs approval to run CI again
a
sorry been busy with other stuff will look at merging it soon
j
Nah it's alg do it whenever lol
It's only me xD
a
it should be fairly stable as i dont think we're changing that part of the workspace very often
what else were you looking at making prs for or is this it
j
Check notion for findings I need confirmation on and after this PR is accepted I have another smaller one
There shouldn't be anymore after I do the rest of the stuff in notion, but it got up to 26 findings xD
At least one is just a comment change tho so it's not actually that many big changes
But I've finished adding stuff to notion now, I don't imagine there's anything more to find
Most of it is just making sure vfs paths aren't being used for physical access
@avery (BAML) could you approve the CI run for this PR? https://github.com/BoundaryML/baml/pull/4447
a
done
and yeah looked into it unfortunately there's no way to let ci run on your prs without adding you to org
j
It's crazy how software permissions have regressed over time. I feel like permissions peaked with TeamSpeak server permissions and have been downhill ever since 😂
Modern software permissions make Group Policy look amazing
@avery (BAML) could you run it again, canary changed and it broke things lol
2 Views
PreviousNext
Powered by