Repository navigation
Task/10321 do not silently rebind otconfig file references - #842
Merged
randy-seng merged 4 commits intoSep 29, 2026
Merged
Conversation
randy-seng
added this pull request to stack #837
September 11, 2026 15:57
randy-seng
force-pushed
the
task/10321-do-not-silently-rebind-otconfig-file-references
branch
from
September 14, 2026 14:02
fcb6f24 to
657ba14
Compare
randy-seng
force-pushed
the
task/10321-do-not-silently-rebind-otconfig-file-references
branch
2 times, most recently
from
September 14, 2026 22:31
9e1cf4f to
657ba14
Compare
randy-seng
force-pushed
the
task/10321-do-not-silently-rebind-otconfig-file-references
branch
2 times, most recently
from
September 21, 2026 12:46
6535a6c to
31c54c8
Compare
randy-seng
force-pushed
the
task/10321-do-not-silently-rebind-otconfig-file-references
branch
2 times, most recently
from
September 25, 2026 15:21
66415ec to
86be125
Compare
briemla
approved these changes
Sep 28, 2026
randy-seng
force-pushed
the
task/10321-do-not-silently-rebind-otconfig-file-references
branch
from
September 29, 2026 09:08
86be125 to
862e777
Compare
Base automatically changed from
task/10283-load-tracks-and-videos-from-s3-by-selecting-a-time-range
to
main
September 29, 2026 09:18
Loading an otconfig had no error handling at all. A track or video file the config references but that resolves nowhere raises FileNotFoundError out of OtConfigParser, and UnableToLoadOtconfigFile was raised but caught nowhere in the codebase, so both reached the browser as a traceback. Saving has explained its failures since it was written; loading now does the same, and the view is not refreshed after a load that failed. Reporting deliberately uses _get_window_position() rather than the existing __show_error, which reads self.treeview_sections and so raises MissingInjectedInstanceError when no treeview is injected -- reporting a failure must not itself fail. This is the channel the substitution reporting in the next commit needs.
When a track or video file is not where the otconfig says, the parser loads a same-named file sitting beside the otconfig instead. That fallback is kept: it was added deliberately in 138ed22 to keep a project openable after it was moved or copied, and test_parse_videos_alternative_files still asserts it. The defect is the silence. The only signal was logger().warning, so a project referencing data the user never named was indistinguishable from one that resolved correctly. An ottrk's name encodes camera and instant, so a same-named file is the same camera at the same moment -- but re-detected or re-tracked output reuses the name with different detections, and a re-encoded video keeps its name. The substitution can therefore swap analysis-relevant content while every name matches. Every substitution is now reported to the user in one message naming what was asked for and what was used. Both paths now resolve through a single _resolve, so videos and track files cannot drift apart -- previously only videos had a test and the two carried duplicate logic. Reporting is an optional register() rather than a constructor argument: OTCloud constructs OtConfigParser with three positional args and has no channel to report through, so a required parameter would break it at its next pin bump. For the same reason the report method is not added to the ViewModel port, whose abstract surface OTCloud implements; gui_application types view_model concretely instead. The cli registers nothing, so a headless run keeps the log warning as its only signal.
…arting Reporting went straight to info_box, which in the nicegui ui ends in ui.notify. That resolves the client through nicegui's slot stack and raises RuntimeError when the stack is empty, which is the case whenever no browser is attached. The startup preload is exactly that case: register_observers() runs before start_ui(), and nicegui_application builds the webserver by preloading any --config file first. So a preloaded otconfig containing a rebound reference raised out of the parse, out of PreloadInputFiles.load, out of the webserver property, and the server never started. Reporting a problem must not create a worse one, so the box is now best effort and the log is the guarantee. Also from review: - The three near-identical log-and-show blocks are now one _tell_user, which is where the guard lives. __show_error keeps its own pre-existing exposure to self.treeview_sections and is left alone. - A failed load named only the wrapper. UnableToLoadOtconfigFile carries a fixed string and keeps the real reason in __cause__, so the user was told "Error while loading otconfig file. Abort loading!" and nothing about which file was missing. - _resolve returns the substitution it made rather than a bare path, so callers stop re-deriving whether one happened by comparing paths.
The message renders paths with str(Path), so on Windows the separator is a backslash. Deriving the expectation from the same Path objects keeps the test green on every runner.
randy-seng
force-pushed
the
task/10321-do-not-silently-rebind-otconfig-file-references
branch
from
September 29, 2026 09:18
862e777 to
f3e4cab
Compare
randy-seng
deleted the
task/10321-do-not-silently-rebind-otconfig-file-references
branch
September 29, 2026 09:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
An OTConfig that names a missing track or video file silently loads a same-named file sitting next to the config instead, logging a warning and nothing more. The project looks correctly loaded while referencing data the user never named — and since re-detected output reuses filenames, the substitute can hold different detections.
The fallback stays; substitutions are now reported to the user.
Two supporting changes: loading an OTConfig had no error handling at all, so a missing file reached the browser as a traceback (commit 1); and both parse paths now share one
_resolve, so videos and track files can't drift apart.Risk
Commit 3 fixes a bug commits 1–2 introduced.
info_boxends inui.notify, which raiseswhen no browser is attached.
register_observers()runs beforestart_ui(), andnicegui_application.py:141preloads--configwhile building the webserver — so apreloaded otconfig with a rebound reference stopped the server from starting. Reporting is
now best effort; the log is the guarantee.
No config or format change. Revert = drop the three commits.
Notes for the reviewer
_resolvefirst — it's the behaviour change. TheFileNotFoundErrortext ispreserved verbatim.
test_parse_videos_alternative_filesis unchanged and still passes. That's the point: itasserts behaviour this change preserves.
register(), and the report method is not on theViewModelport — OTCloud constructs the parser with three positional args and implements that port.
OP#10321