Repository navigation
Fail the run when str.match or a calendar lookup meets a memory failure - #830
luisleo526 wants to merge 1 commit into
Conversation
str.match returned the empty string after any exception it met, and the
four session-calendar lookups of the native path (the entry session
point, the risk day, and the day and ordinal reads of the C callback
host) answered "out of session" or "no day" after any exception. A
memory failure, or a regular-expression resource limit, therefore let a
run finish with a wrong result instead of failing.
A std::bad_alloc now reaches the run's out_of_memory failure (status 1).
Container length errors and the system errors that mean "out of memory
or resources" carry the same code. Every other exception keeps its
fallback.
The regex library's error_space, error_complexity and error_stack depend
only on the pattern and the input, never on memory, so they are not
out_of_memory and not retryable: they stop the run with pine_string_error
and the new reason regex_resource_limit. One inline function maps the 13
standard error codes to that outcome or to an invalid pattern.
An invalid regular expression now stops the run with pine_string_error
(reason invalid_regex_pattern), as TradingView does, instead of returning
an empty string. Patterns the engine's regular-expression library does
not support stop the run too, with the same error; on the Linux build
these include lookbehind, named groups, inline flags, comment groups,
atomic groups, conditionals, \p{...} classes and {,n} repeats. The message
is TradingView's text with the engine's bar prefix, "Error on bar N: ",
or "Error before the first bar: " when no bar is being executed; one
function, run_error_bar_prefix, builds the prefix from the ambient bar
context. The catalog gains the two reasons and their message templates;
its diff reads one changed entry, so a release that carries this change
is a minor one.
The new tests use a counting replacement of the global operator new
(tests/fail_alloc_seam.hpp) to refuse each allocation of a run in turn:
every allocation must end in the out_of_memory failure or in a
closed-trade list (and its totals) identical to the clean run, never in a
different completed result, and the sweeps print the indexes at which a
run completed. They fail on the base tree at the allocations the old code
swallowed. The table of the 13 regex error codes is its own small test:
a one-line change of the mapping turns the expected rows red.
ADR-0001 gets one row for the kernel's literal of TradingView's
invalid-pattern text, which the residuals gate requires for a text that
spells a Pine namespace, and the two pinned phrase counts of its unit
tests move by one. The allocation-bound manifest lists the two
declarations of the new function (ceiling 1413 -> 1415, pinned count
1597 -> 1599) and its site inventory is re-emitted. The lines added to
the sources move the lines the documentation cites below them; the
anchors that follow them are re-pointed by the number of lines added
(digits only, no sentence changes).
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; formal verification proved 0 of 4 changed function(s) (4 not verified).
Formal verification. PR-changed functions: 0/4 verified (0 proven, 0 may-equivalent, 0 distinguished) · 4 not verified (4 unsupported).
Not verified on this run: session\_day (unsupported), risk\_day (unsupported), session\_point\_resolved (unsupported), str\_match (unsupported).
Graphify review — findings
Makes str.match stop the run with a typed error instead of silently returning an empty string or "no match". A memory failure now reaches out_of_memory, an invalid pattern raises pine_string_error with TradingView's message behind the new run_error_bar_prefix (Error on bar N: , or Error before the first bar: ), and a pattern over the regex library's limits raises pine_string_error with the non-retryable reason regex_resource_limit. Patterns the engine's regex library doesn't support (on Linux: lookbehind, named groups, inline flags, atomic groups, \p{...} and so on) now fail the same way, while lookahead, lazy quantifiers, back-references and \b still work. A memory failure during a session-calendar lookup now fails the run with a typed error rather than treating the bar as out of session. The failure catalog, its diff, the CR150 source-bound inventory, the changelog and the docs are updated to match, and ADR-0001 line references are refreshed.
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1991 functions depend on the 1243 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
strategy_native_submit_v1()— 20 callers, 6 callees - worse:
strategy_native_run_v1()— 22 callers, 3 callees - worse:
str_tostring()— 8 callers, 7 callees - worse:
note_run_failure()— 18 callers, 3 callees - worse:
native_run_spec_digest()— 17 callers, 3 callees - worse:
strategy_native_state_v1()— 12 callers, 4 callees - new:
run_match()— 3 callers, 7 callees - worse:
number_style_text()— 1 callers, 7 callees - …and 2 more — each is listed as a finding
Verification — 1991 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1943 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
1420 of 1420 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety (
noncode-change,uncovered-code) — the selection is not trustworthy on its own (see below). CI should run the whole suite.
benchmarks/tests/test_aggregate.py— full-run-safetybenchmarks/tests/test_check_provenance.py— full-run-safetybenchmarks/tests/test_compare.py— full-run-safetybenchmarks/tests/test_public_names.py— full-run-safetybenchmarks/tests/test_run_all.py— full-run-safetybenchmarks/tests/test_run_strategy_feeds.py— full-run-safetydocker/pineforge_sink_finalize_test.py— full-run-safetydocker/pineforge_sink_replay_rq_test.py— full-run-safetydocker/pineforge_sink_replay_test.py— full-run-safetydocker/pineforge_sink_to_json_test.py— full-run-safetydocker/quota_columns_test.py— full-run-safetydocker/quota_convert_test.py— full-run-safetydocker/quota_finalize_test.py— full-run-safetydocker/replay_rq_engine_test.py— full-run-safetydocker/run_json_artifact_window_test.py— full-run-safetydocker/run_json_bin_test.py— full-run-safetydocker/run_json_bounded_transport_test.py— full-run-safetydocker/run_json_bulk_equity_fuzz_test.py— full-run-safetydocker/run_json_bulk_equity_test.py— full-run-safetydocker/run_json_codes_test.py— full-run-safetydocker/run_json_converter_metadata_test.py— full-run-safetydocker/run_json_converter_session_test.py— full-run-safetydocker/run_json_diagnostics_test.py— full-run-safetydocker/run_json_equity_scan_test.py— full-run-safetydocker/run_json_fused_test.py— full-run-safetydocker/run_json_harnesses_test.py— full-run-safetydocker/run_json_metrics_only_test.py— full-run-safetydocker/run_json_misc2_test.py— full-run-safetydocker/run_json_misc_test.py— full-run-safetydocker/run_json_pfbar_test.py— full-run-safetydocker/run_json_provenance_test.py— full-run-safetydocker/run_json_quota_supervisor_test.py— full-run-safetydocker/run_json_replay_test.py— full-run-safetydocker/run_json_selected_sink_test.py— full-run-safetydocker/run_json_sink_lexical_test.py— full-run-safetydocker/run_json_sink_summary_test.py— full-run-safetydocker/run_json_sink_test.py— full-run-safetydocker/run_json_sink_vectors_test.py— full-run-safetydocker/run_json_spool_work_test.py— full-run-safetydocker/run_json_stringio_test.py— full-run-safetydocker/run_json_symbol_feeds_test.py— full-run-safetydocker/run_json_syminfo_test.py— full-run-safetydocker/run_json_trade_scan_test.py— full-run-safetydocker/run_json_window_budget_test.py— full-run-safetydocker/run_json_wrapper_work_test.py— full-run-safetyinclude/pineforge/native_run_spec.hpp— full-run-safetyscripts/derive_corpus_feeds_self_test.py— full-run-safetyscripts/fingerprint_self_test.py— full-run-safetyscripts/run_corpus_self_test.py— full-run-safetyscripts/test_abi_receipt_skips.py— full-run-safety- … and 1370 more
non-code file(s) changed (
CHANGELOG.md,ci/cr150-source-bounds.yaml,ci/cr150-source-sites.json,docker/run_failure_codes.json,docker/run_failure_codes_diff.json…) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md,docs/adr/0001-kernel-adapter-boundary.md,docs/coverage.md,docs/design/BARS-BIN-INPUT.md,docs/design/arena-bound.md…) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Docs that may be stale (advisory)
docs/design/o1-selected-capture.md§ Presentation hooks and compatibility (lines 109-147): references changed symbolsHostdocs/pages/abi-stability.md§ Symbol inventory (lines 111-240): references changed symbolsHostdocs/pages/abi-stability.md§ What's *not* guaranteed (lines 360-666): references changed symbolsHost
Formal verification
Could not verify: Could not verify session\_day.
The verifier did not have enough to check session\_day, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: unsupported: member function — receiver/self (`this`) not synthesizable from the simple-type literal pools — abstaining rather than guessing a receiver
Could not verify: Could not verify risk\_day.
The verifier did not have enough to check risk\_day, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: unsupported: member function — receiver/self (`this`) not synthesizable from the simple-type literal pools — abstaining rather than guessing a receiver
Could not verify: Could not verify session\_point\_resolved.
The verifier did not have enough to check session\_point\_resolved, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: unsupported: member function — receiver/self (`this`) not synthesizable from the simple-type literal pools — abstaining rather than guessing a receiver
Could not verify: Could not verify str\_match.
The verifier did not have enough to check str\_match, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: C++ compile failed: | perror
· 3 grounded finding(s) anchored inline below; 7 more finding(s) on lines outside this diff (see the check run).
| return 1; | ||
| } | ||
|
|
||
| Outcome run_c(std::size_t nth) { |
There was a problem hiding this comment.
run_c()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
|
||
| enum class InjectionPoint { first_match, whole_run }; | ||
|
|
||
| RunObservation run_match(std::size_t nth, InjectionPoint injection = InjectionPoint::first_match, |
There was a problem hiding this comment.
run_match()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
|
||
| } // namespace | ||
|
|
||
| int main() { |
There was a problem hiding this comment.
main()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Superseded by #831: same tree, with a commit message that names the documentation sentence the change removes. |
What was wrong
str.matchand four session-calendar lookups of the native path (the entry session point, the risk day, and the day and ordinal reads of the C callback host) turned every exception into an ordinary answer: "no match", "out of session", "no day". A memory failure, or a regular-expression resource limit, therefore let a run finish with a wrong result instead of failing.What changes
out_of_memoryfailure (status 1). Container length errors and the system errors that mean "out of memory or resources" carry the same code. Every other exception keeps its fallback.error_space,error_complexityanderror_stack) depends only on the pattern and the input, so it is neitherout_of_memorynor retryable: it stops the run withpine_string_errorand the new reasonregex_resource_limit.pine_string_error(reasoninvalid_regex_pattern), as TradingView does, instead of returning an empty string. The message is TradingView's text with the engine's bar prefix,Error on bar N:, orError before the first bar:when no bar is being executed. One function,run_error_bar_prefix, builds the prefix.Scope in plain words
The engine uses the C++ standard library's ECMAScript regular expressions. TradingView accepts some constructs that this library rejects, lookbehind among them. On the Linux build the library also rejects named groups, inline flags such as
(?i), comment groups, atomic groups, conditionals,\p{...}classes and{,n}repeats; a pattern that uses one now stops the run withinvalid_regex_pattern, where before it returned an empty string. Lookahead, lazy quantifiers, back-references, POSIX classes and\bare accepted.Known inconsistency, handled in a follow-up
Known inconsistency, handled in a follow-up: the two older stops word the no-bar case differently from the new shared function; aligned after 1.12.0. With no bar context installed, the shared function prints
Error before the first bar:, the technical-analysis length stops print no prefix, and the map-capacity stop printsError on bar 0:.Tests
The new tests use a counting replacement of the global
operator newto refuse each allocation of a run in turn. Every allocation must end in theout_of_memoryfailure or in a closed-trade list (and its totals) identical to the clean run, never in a different completed result; the sweeps print the allocation indexes at which a run completed. On the base tree they fail at the allocations the old code swallowed. The table of the 13 standard regular-expression error codes is its own small test: a one-line change of the mapping turns the expected rows red.Release
Minor (1.12.0): the failure catalog gains the reasons
invalid_regex_patternandregex_resource_limitofpine_string_errorand their message templates, so its diff against the last release reads one changed entry. ADR-0001 gets one row for the kernel's literal of TradingView's invalid-pattern text.CHANGELOG.mdhas the bullets.🤖 Generated with Claude Code