Repository navigation
Deliver a native run's report only for the call that began it - #361
Merged
Merged
Conversation
) One integration commit on main 70ddaa3 (recorded outputs, #356): the net change 31a40a1..d83ab7d4a of px/native-attempt-fix-20261009, 15 files, applied as a squash of that range. The author branch and its history are untouched. Supersedes 3d2f3f4 (the per-entry Completed gate): the reader gate 86a1856, its revert 124a434 and the restore ec9047d are folded into this net diff, so the documentation 3d2f3f4 wrote is carried here in its final form and this commit removes no documentation sentence of main. What changes (unchanged from the author head): - BacktestEngine::fill_report publishes the retained rows only while the latest run or stream_begin call began (reached reset_run_state). A call refused without beginning (a begin outside Ready, a refused bar array or run option, a calendar or timezone refusal, a begin over a live stream) gets the empty report, and strategy_native_run_v1 answers PF_NATIVE_E_RUN_FAILED for it instead of handing out the earlier run's report. - NativeExecutionConsumer keeps two bools, rows_current_ and batch_current_, behind a private RAII AttemptScope opened first by run_simple, run_tf, run_rich and stream_begin. strategy_stream_fill_report goes through native_stream_snapshot_report and fill_snapshot_report, so a live stream's rows survive a batch call refused over it, which reads empty. - No engine.hpp, ReportC or public API layout change, no codegen change, no version bump. The two bools are report-presentation bookkeeping and are not hashed; identity of the state hashes is owed on the parity population. - Tests kept: the 14 repro assertions (RED on 31a40a1) and the old, twin, current-partial and live-stream rows in tests/test_native_c_api.c, test_checked_settings.cpp and test_native_example_batch.cpp; one obsolete row in test_run_failure_codes.cpp (a second call on a Failed handle is refused, 0 trades). Integration against #356: - One textual conflict, docs/design/native-feature-parity.md rows OT6 and OT7 (adjacent lines changed by different sides). Kept #356's OT6 ("73 runtime exports") and the author's OT7 anchor (native_execution_consumer.cpp:7636). Nothing from either side was dropped. - The other five files #356 also changed (ADR 0001, native-engine.md, native_c_api.h, c_abi.cpp, engine_report.cpp) merged cleanly and keep every recorded-output implementation and doc line. - Six documentation anchors that the author's insertions moved, on lines the author's diff left alone, are carried by an exact line map: parity page lines 126, 159 and 1679, native-engine.md line 647, pine-to-native.md lines 129 and 466. - Not built, not run and not anchor-checked here (no compiler or project helper on this lane); the verification runbook is in the lane report. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…controls, guide wording Follow-up to 44f7d0f (Deliver a native run's report only for the call that began it, onto #356), which integrated the 3d2f3f4 .. d83ab7d4a range onto main 70ddaa3. This commit answers the final review of that work (one P1, two P2) and changes no source under src/ or include/, no PF_API export and no admission behavior. P1, documentation content pin. The stream-report call in strategy_stream_fill_report (src/c_abi.cpp:770) sits inside the window c_abi.cpp:308-963 that docs/design/native-feature-parity.md row 8 pins by content, and a line-neutral edit does not preserve a content hash. The row's claim was re-read against the integrated source first: 73 PF_API definitions in c_abi.cpp, the 12-symbol stream family (strategy_stream_begin .. strategy_stream_fill_report), no export added or removed. The pin is refreshed from sha256:d19d72e6... to sha256:70be189a... by hashing the window's lines joined with newlines. All 15 content pins of the published pages were recomputed by text; 15 of 15 now match. P2, controls. tests/test_native_c_api.c gains check_nested_run_report_ownership: a genuine C callback, after a closed trade, makes a second strategy_native_run_v1 on its own handle over a poisoned output. The nested call must answer PF_NATIVE_E_RUN_FAILED with the empty report, while the outer run ends Failed (Contract, read from the state, not assumed Running) and publishes its own partial trade. tests/test_checked_settings.cpp gains test_snapshot_restores_batch_gate: after a batch call refused over a live stream, a normal and a throwing (injected allocation failure) strategy_stream_fill_report are followed by BacktestEngine::fill_report with no run in between, which must still answer empty. Both files only gain lines: the 14 reproduction assertions, the old, twin, live and started-then-failed controls are untouched. P2, guide. docs/pages/native-engine.md "A run's report." no longer says every listed refusal leaves the lifecycle, the run identity and the retained rows as it found them. It states the cause-by-cause behavior that exists unchanged on the base: a begin on a Completed, Failed or unconfigured handle and a refused bar array or run option preserve them; a calendar or timezone refusal comes after the consumed run-number high-water moved and fails the lifecycle; a begin while Running latches Failed (Contract, Begin; Contract, Configure on a generated source host). It also states that the batch reader stays empty after a snapshot and that a nested run call reads empty while the outer run keeps its rows. This rewords sentences introduced by 44f7d0f (and before it by 3d2f3f4, 86a1856, ec9047d): the dropped sentence "leaves the lifecycle, the run identity and the retained rows as it found them" is replaced, not restored. Not built, not run, not guard-checked here (no compiler or project helper on this lane). Focused GREEN and the integrated-tree proof are owed on a spot box. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…rd checks The measured final run (green1, guards/05-native-versions.log) failed the declared source guard scripts/check_native_cpp_versions.py on 1c89e87 while the same guard passed on the newmain base 70ddaa3: ValueError: pineforge out-of-line method must belong to engine_script_run_v19 Cause: native_stream_snapshot_report was defined after the closing brace of `inline namespace engine_script_run_v19` in src/native_execution_consumer.cpp. The guard compares every qualified `Name::member(` match in `namespace pineforge` with the matches inside the epoch, so the bridge's `NativeExecutionConsumer::fill_snapshot_report(` call counted as outside the epoch (guards-mid/diag.mid.txt: {'NativeExecutionConsumer::fill_snapshot_report': 1}); the next comparison, namespace_functions, would have named the bridge itself. Fix, two line-neutral hunks and nothing else: - src/native_execution_consumer.cpp: the six-line bridge (two comment lines, the three-line definition, one blank line) moves above the epoch's closing brace. The definition and the fill_snapshot_report, rows_are_current and AttemptScope bodies are byte-identical; one comment line gains ", in the engine epoch". The pinned native_run_spec_v3 digest block stays where it was. - src/c_abi.cpp: the forward declaration beside the two native_c_host hooks is reopened in `inline namespace engine_script_run_v19`, on its own single line (the shape market_driver.hpp uses for its native_run_spec_v3 forward declaration), so it names the same entity as the definition. The call pineforge::native_stream_snapshot_report( in strategy_stream_fill_report is unchanged: qualified lookup finds the member of the inline namespace. The mangled name gains the epoch component, and the two files above are the only mentions of the bridge in the tree. No PF_API export, header, layout, hash, checker, allowlist or version changes. Both files keep their line counts (c_abi.cpp 1173, native_execution_consumer.cpp 11557) and every line outside the touched hunks keeps its number, so no file:line anchor and none of the content pins of the published pages move. All 21 pin occurrences were re-hashed by text on this tree and match (the six c_abi.cpp windows start at line 308); 137 anchors into these two files were scanned and none touches a changed line. No documentation changes. Not compiled or run here (local-only lane). A read-only text replay of the guard's ownership comparison (stdlib only, the guard itself not executed) reproduces the measured difference exactly on 1c89e87; on this tree TYPE_DEF, ALIAS_DEF, METHOD_DEF and namespace_functions agree for the consumer .cpp and .hpp. The build, ctest, the declared source guards and the doc gates still have to be re-run on a spot box. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Apply the retained docdelta1 patch after the private bridge epoch repair. This corrects 1c89e87 and the earlier 44f7d0f citation mapping. Replace the broad claim "A calendar or timezone refusal comes after the consumed run-number high-water moved" with its begin_ready scope. Replace "A begin while the handle is Running" with the actual batch/reentrant path and the repeated-stream exception. Four citations now point at the source they describe. Runtime behavior is unchanged. The author timed out; the patch and sidecar predate the bound, while its detailed report was written after it. Final independent review and exact-head runtime checks remain required.
luisleo526
marked this pull request as ready for review
October 9, 2026 09:54
luisleo526
added a commit
that referenced
this pull request
Oct 9, 2026
* docs: record the native attempt-report fix under 1.5.0 Rename the Unreleased heading to "1.5.0 — Unreleased" and add one bullet for #361: a batch run call's report belongs to its own attempt, a call that does not begin a run gets the empty report and a failure status instead of OK or an earlier payload, a started-then-failed run and the explicit stream snapshot keep their reports, and the engine-owned gate covers the C, native-module and generated wrappers with no change to emitted source. The recorded-outputs entry is unchanged byte for byte. CHANGELOG.md only: no source, version, date, tag or patch-line change. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * docs: scope the batch report promise to engine pre-begin refusals A call that reaches the engine answers its own attempt's report. The empty report and PF_NATIVE_E_RUN_FAILED now cover only calls the engine refuses before it begins a run. The C entry's PF_NATIVE_E_HANDLE and PF_NATIVE_E_ARGUMENT checks and native-module argument preflight still return before the engine runs and leave caller output untouched, as native_c_api.h and native_module.hpp already state. Changelog wording only: no source, header or test change. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <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.
What this fixes
A native strategy handle can be reused for a new run. When that new run was refused before it began (an invalid setting, a bad input, a rejected reuse), a caller that then read the report could receive the previous run's report, and the native C entry point could return that earlier run's success result. Nothing was computed wrongly, but the caller was told about a run it had not asked for.
The shared report serializer now presents only rows that belong to the attempt the current call began:
What does not change
Public and generated layouts, hash definitions and emitted sources are unchanged. Readers of recorded outputs keep their documented "last begun run" snapshot semantics. A caller that uses one fresh handle per run was never exposed.
Evidence
pineforge/paritystatus on this request.Release
Patch release 1.4.1: an engine fix only; the transpiler's output is unchanged.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Ko17oHtRco8iZ9vJptBAj8