Skip to content

fix(matrix): harden PineGenericMatrix — bounds, exceptions, validation - #2

Merged
luisleo526 merged 15 commits into
mainfrom
fix/generic-matrix-validation
May 10, 2026
Merged

luisleo526 merged 15 commits into
mainfrom
fix/generic-matrix-validation

Conversation

@luisleo526

Copy link
Copy Markdown
Collaborator

Summary

Phase 2 follow-up to #1. Hardens `PineGenericMatrix` against the C++ review punch list:

Blockers

  • Bounds-check every index op (get/set/row/col/row_ref/remove_/swap_/submatrix/add_*) — throws `std::out_of_range` instead of UB
  • Validate insert/slice indices (negative, over-range)

Major

  • `new_` rejects negative dims
  • `elements_count` overflow guard (int64 accumulator)
  • Strong exception guarantee for column-mutating ops (`add_col`/`remove_col`/`swap_columns`/`reshape`) — build-then-swap
  • `add_col` on empty matrix rejected with clear error
  • `concat` shape validation, `transpose`/`reshape` static_assert default-constructible

Minor

  • bool sort compile-rejected for parity with general spec
  • self-concat aliasing test
  • consistent static_assert message naming
  • Series ring-buffer + value-semantics tests
  • `new_` split into init/no-init overloads
  • `numeric_limits::max()` over `INT32_MAX`
  • bool spec duplication TODO doc

Test plan

  • All 20 generic_matrix ctest pass
  • Codegen sibling repo verified compatible (670 pytest pass with engine header)

luisleo526 added 15 commits May 10, 2026 23:23
Throw std::invalid_argument when concat is called with shape-incompatible
operands (column-count mismatch for vertical, row-count mismatch for
horizontal). Applied to both the general PineGenericMatrix<T> spec and
the bool specialization. Adds tests for both throw and sanity paths.
Both PineGenericMatrix<T>::transpose and ::reshape construct the
destination buffer with T{}. Add static_assert(std::is_default_constructible_v<T>)
to surface a clear diagnostic at compile time when instantiated with a
UDT whose default constructor is deleted. Adds a positive UDT test plus
an SFINAE-style probe documenting the guard.
Add std::out_of_range throws to get/set/row/col/row_ref/remove_row/
remove_col/swap_rows/swap_columns/submatrix/add_row/add_col on both
the general and bool specializations. Replaces silent UB on negative
or over-range indices. Tests cover negative + over-range for each op
on both specs.
Submatrix now requires from <= to on both axes; throws
std::invalid_argument otherwise. add_row/add_col idx already
allow append (idx == size) per B1 commit.
new_ on both general and bool specializations now throws
std::invalid_argument when rows or cols < 0.
Accumulate row sizes into int64_t and throw std::overflow_error
if the total exceeds std::numeric_limits<int>::max(). Return type
remains int for Pine ABI parity. Practical overflow needs ~2^31
cells (test infeasible by memory); a normal-total test guards
against accumulator regressions.
add_col, remove_col, swap_columns, and reshape now build a fresh
data buffer locally and swap it in only after all element copies
succeed. A throw mid-operation leaves the matrix unchanged.
Applied to both the general and bool specializations.
Test uses a UDT whose copy ctor throws on the Nth copy to verify
state preservation for add_col and remove_col.
Previously add_col on a matrix with no rows silently assigned
empty rows then UB-inserted into them. Both specializations now
throw std::logic_error with guidance to use add_row first.
The general spec already static_asserts that sort requires int,
bool, or std::string. The bool spec previously had a working
sort() with no guard. Make bool spec sort() a template member
that static_asserts on instantiation, matching general-spec
behavior of compile-time rejection.
Verify concat(m, m, true) and concat(m, m, false) produce
correctly doubled dimensions and content (no aliasing corruption
between the source row buffer and the destination).
All static_assert messages now use the 'matrix.<op>: ...' style
matching the runtime exception messages.
… (m8)

Add ring-buffer wraparound test (push beyond max_len, verify
oldest entries are evicted and newest occupy [0]), and a
value-semantic aliasing test (mutating a pushed source after
push() does not bleed into the series; mutating a retrieved
copy does not bleed into older history).
Replace the single new_(rows, cols, T init = T{}) with two
overloads: one taking an explicit init value, and a no-init
form requiring T to be default-constructible. Avoids silently
requiring default construction when an init value is supplied
and gives a localized static_assert on the no-init form.
Replace INT32_MAX in reshape overflow checks with
std::numeric_limits<int>::max() for type safety and consistency
with the M2 elements_count guard.
@luisleo526
luisleo526 merged commit 3ed7f8f into main May 10, 2026
4 checks passed
luisleo526 added a commit that referenced this pull request May 11, 2026
Pin the new real-bar magnifier contract with three end-to-end tests:

  test_stop_fills_in_correct_script_bar
    30 1m bars + script_tf=15m -> 2 chart bars. A long entry on bar 0
    plus a stop at 95.0 (which only the 1m bar inside chart-bar #2
    crosses) must fill at exactly 95.0 with closed_trade_exit_bar_index
    == 1, proving the fill is routed to the actual 1m sub-bar where
    price crossed the stop, not to a synthesized chart-bar tick.

  test_distribution_irrelevant_when_real_sub_bars
    Same 30 1m bars + four distinct (distribution, samples) pairs
    (ENDPOINTS+4, UNIFORM+8, COSINE+16, FRONT_LOADED+12). All four runs
    must produce identical trade count, exit price, and exit bar.
    Guards against any future magnifier change reintroducing synthetic
    ticks inside real lower-TF bars.

  test_legacy_path_used_when_single_sub_bar
    input_tf == script_tf with bar_magnifier=true and 8-sample UNIFORM.
    Confirms the legacy synthesized-distribution path is still active
    when only one sub-bar arrives per script bar (16 sample ticks for
    2 sub-bars, not 8 — i.e. real-bar mode does NOT engage).

Co-Authored-By: Claude Opus 4.7 <[email protected]>
luisleo526 added a commit that referenced this pull request May 12, 2026
…ch test

- Bump corpus submodule pin to ba217ed (post-merge of corpus/ta-isolate-probes
  PR #2): adds 12 ta-isolate diagnostic probes, 37-regex tv_trades refresh,
  typed-matrix family `ohlcv_start_ms` pins, and VCP TV-anomaly inputs.json.
- Register tests/test_generic_matrix_bool_dispatch.cpp (10 tests) — locks in
  PineGenericMatrix<bool> set/get/transpose round-trip + hotCount loop +
  168-bar mask population synthetic test. All passing.

ctest: 30/30 (including 2 new test files this branch added — bool_dispatch
and magnifier_real_bars).
@luisleo526
luisleo526 deleted the fix/generic-matrix-validation branch July 2, 2026 04:28
luisleo526 added a commit that referenced this pull request Sep 17, 2026
…sidual lot (R4-D L10y)

yukozb-gold-ny-orb-v19-close-20-00 #2/#3 and ahtisham-ee-decoded-
volatility-expansion-ahtisham #126/#127: with default_qty_value=100 the
legacy owner admits the entry and books it as a main lot plus a residual
margin-call lot closed again on the entry bar; the switched route refused
the whole entry.  The adapter now sizes and admits the full-equity entry
the way the owner's fill-time affordability and margin-call slice do, and
samples the residual lot's open extremes like the owner.  Both population
reproductions replay identically to the owner.  Pinned with embedded bars
in tests/test_l10y_full_equity_entry_split.cpp (W21c, qwen3.8-max).

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01RKCCnMS51GLmRbA3AHtBDb
luisleo526 added a commit that referenced this pull request Sep 20, 2026
…rval, after a pending lazy seal (R5 lane L6d, item 1)

A script interval none of its own inputs sealed -- a session close clips
it short of its nominal end (the last "60" of a 09:30-16:00 session,
every "D" over an intraday input), or the feed has a hole over its last
slot -- is sealed lazily by the first input of a later interval, inside
contribute_input. The subscription pump ran on that same input BEFORE
contribute_input, so the lazily sealed calculation of script bar k ran
after input i+1 had already reached the declared series: it could read a
bucket input i+1 completed (and that holds i+1), find a gapped series
already cleared by i+1, read the boundary bucket i+1 closed, or read the
lookahead bucket i+1 opens. Measured in R3b STEP 1 row 14 (class iii,
proposed there, not implemented): a one-input leak for a bare host. On
87b3b06 the last script bar of a session read the NEXT session's first
input bar through a "15" series.

Ruling (supervisor): the pump is ordered against the SCRIPT interval, not
the raw input. When input i+1 arrives and script bar k is still lazily
unsealed, the kernel first runs bar k's sealed calculation, with the
series as its own contributing inputs left them, then delivers input
i+1's subscription bars, then proceeds with i+1's own aggregation,
matching and calculation point:

    on_native_input(i+1) -> seal(k) -> deliver(i+1) -> calc(i+1)

The lazy-seal block of contribute_input is factored into
seal_stale_script() and called from consume_confirmed_input ahead of the
pump, inside the `!subscriptions_.empty()` branch only; contribute_input
calls the same helper where the block was, which then finds nothing to
seal.

BEHAVIOUR CHANGE, and its whole scope: a bare native host that combines
`subscriptions` with a lazily sealed script interval. For it the
deliveries riding on the sealing input move after the lazily sealed
calculation (NativeTimeframeBarContext and the buckets themselves are
unchanged; only their position relative to that one on_native_bar).
Byte-identical by construction for everything else:
- a run without subscriptions never enters the branch and keeps its lazy
  seal inside contribute_input, at the same statement position;
- a subscribed run with no stale script bar executes the statement order
  it always did (the helper returns before touching anything). That is
  every run whose input and script timeframes are equal, where each
  input seals its own interval -- hence every Pine adapter run: the
  adapter declares no subscriptions on this tree, and R3b's kernel route
  is taken only at input_tf == script_tf (its aggregated-chart path
  keeps the adapter's own deferral #2, which is this same ordering).

Host-free: no new virtual, no field, no epoch bump, no C ABI change. No
durable state is added, so nothing new is folded into any hash. The
pinned tokens of scripts/check_native_cpp_versions.py are untouched.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
(cherry picked from commit 995070090fc1422e90aa99d725637da56b5b7c0f)
luisleo526 added a commit that referenced this pull request Sep 24, 2026
… new/delete form through INT17b's one allocator

Four slice-B rows defined their own partial set of the replaceable global
allocation functions to count heap allocations -- the pattern INT17b found
aborting under the Linux sanitizers job (alloc-dealloc-mismatch: libstdc++'s
temporary buffers take the NOTHROW operator new, left as ASan's, and give it
back through a SIZED operator delete the test had routed to free()):

  test_native_intrabar_scratch      PERF-L1  plain/array/nothrow new, plain/sized delete
  test_adapter_quiet_bar (+ its     PERF-L4  plain/array new, plain/sized delete
    _probe and _ungated builds)
  test_adapter_command_allocations  PERF-L5  plain/array new, plain/sized delete
  test_native_match_allocations     PERF-L5  plain/array new, plain/sized delete

Each now includes tests/global_allocation_replacement.hpp (all twenty forms,
one malloc/free allocator) instead:

  * intrabar_scratch, command_allocations, match_allocations take the
    difference of global_allocation::allocations around the span they
    counted before (the run; the one re-issued strategy.exit call; the
    window between script bars kFirstCounted and kLastCounted), the
    style of INT17b's counted conversions; their on/off flag had no other
    reader and is gone.
  * quiet_bar keeps its body: `allocations` is now a reference to the
    shared count (the _l4d twin's precedent), reset where it was reset,
    and `count_allocations` still says whether a bar records its count.
    Every recorded span lies inside the run it was counted over, so each
    per-bar entry is the count it was.

Nothing under src/ or include/ changes (engine code has no over-aligned
allocation, so the aligned forms add nothing to a count). The four rows
print exactly what they printed before the change, line for line, on the
Mac release build: intrabar 0.003-0.045 allocations/bar per path, quiet
bars 4 per stretch (nothing live: 4) magnifier off and on, the re-issued
exit 301 allocations over 42 calls, the matcher 0 per window for every
book. No assertion, bound or pinned value changes.

Fail-before, the sanitizers profile on spark (Linux aarch64, GCC 13.3,
ci_verify's own configure argv, the job's ASAN_OPTIONS/UBSAN_OPTIONS plus
detect_stack_use_after_return=0): the four TUs as the lanes wrote them,
compiled with the sanitizer build's own compile and link commands --

  test_adapter_quiet_bar            aborts: AddressSanitizer:
  test_adapter_command_allocations    alloc-dealloc-mismatch (operator new vs free)
      #1 operator delete(void*, unsigned long)  <the test's own sized delete>
      #2 std::__detail::__return_temporary_buffer<PendingSameBarCommand>
      #5 std::stable_sort  in PineExecutionAdapter::flush_pending_same_bar_commands
      allocated by: operator new(unsigned long, std::nothrow_t const&) (asan)
  test_native_intrabar_scratch      passes (8 passed)
  test_native_match_allocations     passes (ok)

-- the mechanism INT17b fixed, so both adapter rows would have failed CI's
sanitizers job; the two kernel rows are converted so no counting test is
left with a partial set. After: all four pass there. A grep of tests/ for
global operator new/delete definitions now finds only the helper and the
uncompiled legacy base tests/test_pending_order_mirror_no_alloc.cpp that
INT17b left as it is.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant