POC of Corrected Coords LUA

[nagisml]

POC example for a CSV read and correcting coordinates

Test files:

• DB with fake test caches
• LUA Code
• CSV file with fake coords

corrected_coords.zip

[AgreeDK]

@nagisml
Nice POC, and a very practical use case for the macro system (#938). 👍

I've tested it locally: test_macro_runtime.py passes, and the full suite is green when merged on top of beta.

What I like:

• Coordinate parsing reuses parse_coords(), so the macro accepts every format the rest of OpenSAK does.
• _wrap turns errors into clean Lua errors, so pcall in the example script works as intended.
• The final filter escapes quotes in the GC codes correctly.

Two design questions I'd like to settle before the API grows:

1. read_csv() can read any file on disk (absolute paths and ~). The concept document (§9) says macros of unknown origin preferably shouldn't get free file system access. It's read-only and there's no network access, so the risk is limited. Still, should we restrict it to the macro folder (and maybe a user-chosen data folder) now, before more file functions are added?
2. Partially filled rows silently clear data. A row with lat but an empty lon falls through to clear_corrected(), so a typo in the CSV can remove coordinates the user has already solved. Safer: only a row with no coordinates at all clears, and a half-filled row raises an error.

A minor one: every row triggers a full refresh of the table row, map pin and detail panel. That's fine for small lists, but slow with thousands of rows (the CSV limit is 10 MB). Batching the refresh could come later.

Needed before merge: this conflicts with #948 in tests/e2e-tests/test_e2e_filter.py (all three of your PRs fix the same flaky test). I'll merge #948 first. Could you then rebase on beta and drop the change to that file?

[AgreeDK]

@nagisml
Following up on the two design questions above, here's what I'd suggest. Both are cheap to do now while the API is small, and much harder to change once macros out there depend on the current behaviour.

1. File access in read_csv()

I'd restrict reading to a few allowed locations, plus an explicit way for the user to pick anything else:

• the macro file's own folder and its subfolders (covers the common case, a CSV next to the macro, like in the example)
• the OpenSAK macro folder (so macros run from the editor without being saved still have somewhere to read from)
• files the user picks in a file dialog during the run, e.g. via a new opensak.choose_file(). This covers "my solved list is somewhere else", with the user's visible consent instead of silently.

Technically it's only a few lines: resolve the path with Path.resolve() so ../ and symlinks can't escape, then check it with is_relative_to() against the allowed roots plus the files the user picked in this run.

The reason to do it now: permissions are easy to widen later and hard to take back. If we allow arbitrary paths now and tighten it later, existing macros break. The same rule should also apply to the writing functions later (export_gpx, export_csv), where the risk is clearly higher, so it's nice to have the pattern in place from the start.

2. Partially filled rows

I'd flip the default: empty means "do nothing", not "clear". An empty cell in a CSV is far more often a mistake, or a cache not solved yet, than a wish to delete data. Concretely:

• no coordinates in the row → skip it and log it as skipped
• only lat or only lon → error for that row ("lon missing"), and the rest carries on via pcall
• clearing happens only explicitly, e.g. with the value clear in the coords column

opensak.clear_corrected() is already explicit, so it's only the example script's logic that needs changing. Worth getting right, because example scripts get copied, and what we show as an example becomes how people write their own macros.

Later, once opensak.confirm() exists, the script could also show a summary first ("12 will be set, 2 cleared, continue?"), but that doesn't belong in this POC.

Happy to discuss if you see it differently. You know the macro runtime better than I do at this point.

[nagisml]

@AgreeDK need to leave soon so a first quick reply
File Handling

• We need to make a possibility to leave the OSAK folder structure with macros otherwise it's to limited (eg I store my GPX exports directly on a OneDrive folder for mobile sync)
• I would propose something like a approved folderlist for read/write. OSAK folders should be there for default. Any other folder needs to be added with an explicit approval eg manually or by a macro question when hitting a not yet approved folder

Partially filled rows

• Points regarding securing the CC updates will be checked later. Thanks for the feedback
• Maybe we should define where to store the "example" lua scripts and how to handle the tests like the csv. Ideas? I took the macro/example folder for that and like it as we can prepare multiple examples which are delivers to users to be used as templates.

[nagisml]

@AgreeDK conflicts solved. Up to you if want to merge or wait for the hardenings

[nagisml]

@AgreeDK btw any idea how to document the LUA OpenSAK calls? Starting early would help here as well.

[AgreeDK]

@nagisml
Thanks for all your work & feedbacks, I will review it all and come back with my comment/suggestions later today.

[AgreeDK]

@nagisml
Thanks for the quick reply, and for solving the conflicts! Here are my thoughts on your points.

1. Approved folder list for read/write

Good idea, and the OneDrive case is a real need. I agree with an approved folder list. A few details I think are important:

• Approval must come from OpenSAK, not from the macro. When a macro hits a folder that isn't approved yet, OpenSAK itself shows a dialog with the full path and the choices "this time only / always / deny". A macro must never be able to approve a folder on its own, otherwise the mechanism is worthless against macros of unknown origin.
• Read and write are approved separately. A folder you only read CSVs from doesn't need write access.
• The default list shouldn't give write access to the database folder. A macro that can overwrite .db files directly is exactly what we want to avoid. The macro folder and an export folder seem like sensible defaults.
• The list should be visible and editable in Settings, and paths should be checked after Path.resolve(), so ../ and symlinks can't escape.

2. Example scripts and test CSV

macros/examples/ in the repo is a good place, and I like the idea of shipping several examples as templates. Three things need sorting out:

• Packaging: the folder must be included in opensak.spec (PyInstaller datas), otherwise it won't exist in the release builds.
• Read-only location: inside the macOS app bundle and the MSIX package, the files can't be edited. So the examples should be offered as templates that get copied to the user's macro folder, e.g. via an "Open example…" entry in the Macros menu, or on first run.
• Tests: tests should use their own fixtures under tests/ rather than depend on the example files. But a single test that loads every example script with Lua (compile only, not run) would make sure the examples don't silently break when the API changes.

3. Documenting the Lua API

Agreed, starting early is a good idea. I'd make the code the single source of truth:

• One API registry in runtime.py: each function is described in one place, with name, signature, description, example and "since API version". The same registry builds the opensak table in Lua, so docs and code can't drift apart.
• A script generates docs/macros/api.md from the registry, and a test fails if an exposed function is undocumented. Same idea as test_no_missing_keys for the language files.
• Later: the registry could also generate a type stub file for the Lua Language Server (---@meta). That would give people autocompletion and inline docs in VS Code while writing macros, almost for free once the registry exists.

4. Merge now or wait?

I'd prefer to wait for the folder restriction. If we merge now, unrestricted read access ships in the next beta, and the next PR would change how paths behave anyway. The partially-filled-rows fix is only a few lines in the example script, so it could go in at the same time.

No rush. Thanks again for all the work on this!