Repository navigation
fix: accept pinned Flare derived analysis after sidecar save - #155
Conversation
Match the pinned producer augmentation without relaxing original observation integrity or raw metric recomputation. Exercise its write order and reject divergent fields even with refreshed packet digests. Agent-assisted-by: Codex (author) Signed-off-by: GUOGUO <xwdong1998@163.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Dong1017
left a comment
There was a problem hiding this comment.
LGTM 5a2d16e within the corrective software/source-compatibility scope of #147.
Independent reviewer role: this review was performed separately from the implementation/local-agent work. Inspected exact base a4a0fbd62defe2bc53ab090e1fa50bd7139663a6, all four changed files, pinned Flare bce032c014bae34502ba22d3304fa084d5b40afe, the reopening finding and existing PR discussion. No new actionable finding.
Verified clearance of the original blocker: Flare saves sidecars before measurements-only augmentation. The adapter now accepts the unchanged observation or exactly the documented analysis.http_prefill_tokens_per_second addition, for the two permitted groups and pinned formula. Every original field and other analysis entry remains in the comparison; this is not a blanket omission of analysis/integrity checks. The production file outside this assertion replacement is unchanged, including raw SSE validation, selected raw TTFT recomputation and failure behavior.
Regression review: producer-shaped fixture preserves write order; both augmented groups and unchanged groups are asserted. Five divergence types on both copies refresh digests and require rejection with empty metrics. Poisoned aggregate/selected-observation summaries still produce the raw metric with original bytes preserved. Existing actual validation_command integration retains valid-pair PASS and incomplete-peer INCONCLUSIVE/no metric.
CI: Ready 40289712837 has all four jobs successful; Python 3.11 logs record 989 passed, 9 skipped, 135 subtests and Ruff success. This CI ran synthetic merge ea1b7f9f98b2b03a99feb7a13540c39fa54e94a4 combining this head with the stated base. Latest declaration run 40292251455 and exact-head knowledge-model-policy both now read success; head was rechecked immediately before posting.
Limits: independent static/source/test-contract review plus eight standalone arithmetic JSON-round-trip checks; I did not rerun the full fixture/integration suite locally or run models/services/GPUs. The implementer's reported 34 focused tests are not relabeled as my execution. This supports the corrective PR's software readiness, not real-service/performance/agent efficacy, automatic integration or fresh whole-Issue acceptance. Keep #147's re-acceptance decision separate.
|
Owner-directed corrective integration completed: guarded squash of reviewed head Green evidence: four Ready jobs, declarations, and refreshed exact-head knowledge-model-policy success from 40293232635. The first merge attempt was rejected while a new status became pending; no bypass was used. It was retried only after the head/status were re-read as unchanged/success and GitHub reported clean. Actual squash tree |
Governing Issue and delivery slice
Refs #147
Flare saves
observation.jsonbeforerun()addsanalysis.http_prefill_tokens_per_secondforttft-4096andprefill-65536. Whole-object membership therefore rejected a normal producer packet. The adapter now accepts the unchanged observation or that exact addition using the pinned producer formula. Every original field and every other analysis field must still agree; raw SSE checks and selected client TTFT recomputation remain in place.Knowledge authorship declaration
Not applicable: no protected knowledge path changes. Material agent authorship is retained in the commit trailer.
Changed artifacts and evidence
Recheck of the existing six P0 criteria
metrics={}; existing real-command integration covers incomplete-peer INCONCLUSIVE.Checks and dependencies
bce032c014bae34502ba22d3304fa084d5b40afe, sidecar write, later augmentation and measurements write.a4a0fbd62defe2bc53ab090e1fa50bd7139663a6.5a2d16eeaa02c63d2b355665db933d5cea576685.python -m pytest tests/test_flare_adapter.py::test_pinned_producer_augments_measurements_after_saving_sidecars -qwith the new regression/fixture and base production code failed withValueError: observation sidecar differs.python -m pytest tests/test_flare_adapter.py -o addopts='' -q— 34 passed in 739.12s, including the unchanged realvalidation_commandintegration assertions.ruff check src/inferrsi/model/validators/flare.py tests/fixtures/flare_packet.py tests/test_flare_adapter.pypassed.git diff --checkpassed.knowledge-model-policystatus is success / Not applicable.Remaining work and Issue closure
LGTM 5a2d16eeaa02c63d2b355665db933d5cea576685.