Incremental improvements: refactors, edge cases, robustness#1
Open
Incremental improvements: refactors, edge cases, robustness#1
Conversation
The validator parsed INTEGER columns with strtod, which happily accepts "1.5", "1e-3", "NaN" and "Inf" as valid doubles. That meant an INTEGER schema column would silently pass fractional or non-finite values, defeating the point of choosing INTEGER over FLOAT. Enforce that the parsed value is finite and has no fractional part before accepting it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ConfigLoader's ${VAR} regex only matched [A-Z0-9_], so lowercase or
mixed-case env vars such as ${home} or ${my_token} were left untouched
in the rendered TOML. POSIX env var names permit both cases; broaden
the pattern to [A-Za-z_][A-Za-z0-9_]* so every valid name is expanded.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HttpClient.send can throw InterruptedException; the catch-all was swallowing it and clearing the thread's interrupt flag. During a shutdown-driven interrupt the FileProcessor executor would then fail to notice it should wind down. Catch InterruptedException explicitly and re-interrupt the current thread before returning. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ordering solely by processed_at DESC leaves ties unresolved, so two rows sharing a timestamp (possible when clock resolution is coarse, or when tests insert with a shared Instant) may swap positions between calls. Break ties on the monotonic id column so recent() is stable and matches the ordering the dashboard uses for its id-based polling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Number.isFinite accepted fractional ids like 1.5 and negative values, which then flowed into better-sqlite3 where 1.5 gets coerced to 1 and negative ids always miss. Require a positive integer so the API fails loudly and consistently for malformed inputs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Five atomic fixes across engine (C++), cli (Java) and dashboard (TS).
1.5,1e-3,NaN,Inffor INTEGER schema columns; now rejected as TYPE_MISMATCH.${VAR}regex was uppercase-only, silently dropping${home}/${my_token}; broadened to POSIX env-var names.InterruptedException, so executor shutdown couldn't observe the interrupt.processed_at DESCalone left ties unresolved; addedid DESCas a stable secondary sort.Number.isFiniteaccepted1.5and negatives, which better-sqlite3 silently coerced; now requires a positive integer.Test plan
cmake --build engine/build --target valenginecompiles cleanly (verified locally; pre-existing unrelated -Wunused-result break in test_fuzz.cpp left untouched)mvn -pl cli test(not run; no mvn available in sandbox)npm test(not run; no node available in sandbox)Do not merge.