fix(delivery): C-79 and C-83 — two refusals on the live FAO path - #222
Merged
Conversation
C-79: the store port failed OPEN, and had zero tests The comment beside `_ContractStorePort.upload`'s result check calls it "the whole mechanism". It is: pipeline-core's store, on a metadata failure AFTER the file is uploaded, logs and RETURNS success=False rather than raising, so a caller that discards the result ships a file with no metadata document -- invisible to the consumer. That happened to run-0's historical artifact on 2026-07-27. The check was `if success is False`. A result that was None, or lacked the attribute, or carried a non-bool, sailed through as though the upload had worked. Now `is not True`. The dead to_dict() fallback went with it: an unrecognised result should be refused and named, not adapted to silently. The message reports what it actually received, because success=None (a moved contract) and success=False (a reported failure) send an operator to different places. tests/test_store_port.py -- 16 tests over both partners, where there were none. The standing excuse for source-scanning manager facts is that managers need Appwrite env and a views-models path manager; the port needs neither, so it never applied here. Its trigger fired on 2026-08-03 and nobody noticed. The entry read "the 3.0.0 bump is the next occasion"; the bump landed, C-44 closed on a wheel-level suite verification, and the return contract was never re-read. test_register_integrity cannot catch that -- its checks are structural and none asks whether a named external event has occurred. C-83: a failed import reported as a wrong declaration get_queryset() returns None for ANY exception importing config_queryset.py; declared_data_format(None) defaults to 'dataframe'; the format guard then tells the operator to set data_format: 'feature_frame' in a file that already says exactly that. launch_config.assert_queryset_was_importable now runs FIRST, and both managers read the queryset once and reuse it. The refusal says what happened and steers away from the config file -- "This is NOT a declaration problem: do not edit data_format until the module imports" -- toward the traceback pipeline-core logged. Logs before it raises. Three guards, because order IS the fix: the refusal fires and names the real fault; an importable queryset passes (the format question belongs to the next check); and per partner, get_queryset() is called exactly once with importability checked first. Not fixed here, deliberately: upstream still returns None for any import exception. We stopped passing it into a function whose contract is to default. Raising upstream would be better and is not ours; waiting for it would have left the misleading message on the live FAO path meanwhile. Mutation-proven three ways -- revert the polarity (4 fail), delete the importability check (1 fails), reverse the order (1 fails). 352 passed / 40 xfailed / 0 failed. ruff clean. Register 83/12/71. Co-Authored-By: Claude Opus 5 (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
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.
B4. Both Tier 2, both on the path that delivers to FAO.
C-79 — the store port failed open, and had zero tests
The comment beside
_ContractStorePort.upload's result check calls it "the whole mechanism". It is: pipeline-core's store, on a metadata failure after the file is uploaded, logs and returnssuccess=Falserather than raising. A caller that discards the result ships a file with no metadata document — invisible to the consumer rather than absent. That is what happened to run-0's historical artifact on 2026-07-27.The check was
if success is False. A result that wasNone, or lacked the attribute, or carried a non-bool, sailed straight through as though the upload had worked. Nowis not True. The deadto_dict()fallback went with it — an unrecognised result should be refused and named, not adapted to silently. The message reports what it actually received, becausesuccess=None(a moved contract) andsuccess=False(a reported failure) send an operator to different places.tests/test_store_port.py— 16 tests over both partners, where there were none. The standing excuse for source-scanning manager-side facts is that the managers need Appwrite env and a views-models path manager to instantiate. The port needs neither: it takes a store object and calls four methods on it. A fake store was always enough; nobody had tried.Its trigger fired two days ago and nobody noticed. The entry read "the 3.0.0 bump is the next occasion". The bump landed 2026-08-03, C-44 closed on a wheel-level verification of the suite, and the return contract was never re-read.
test_register_integrity.pycannot catch that — its ten checks are structural, and none asks whether a named external event has occurred. That gap is C-82's.C-83 — a failed import reported as a wrong declaration
get_queryset()returnsNonefor any exception while importingconfig_queryset.py.declared_data_format(None)defaults to'dataframe'. The format guard then says:…pointing at a file that already says exactly that, while someone is fixing a failed run.
launch_config.assert_queryset_was_importablenow runs first, and both managers read the queryset once and reuse the value. The refusal says what happened and steers away from the config file — "This is NOT a declaration problem: do not edit data_format until the module imports" — toward the traceback pipeline-core logged, and toward a missing sibling checkout or dependency. It logs before it raises (ADR-008).Three guards, because order is the fix. The refusal fires and names the real fault; an importable queryset passes (the format question belongs to the next check, and separating them is the whole point); and per partner,
get_queryset()is called exactly once with importability checked first.Not fixed here, deliberately: upstream still returns
Nonefor any import exception, so the ambiguity survives at its source. We stopped passing it into a function whose contract is to default. Raising upstream would be better and is not ours to do — and waiting would have left the misleading message on the live path meanwhile.Verification
Mutation-proven three ways: revert the polarity (4 tests fail), delete the importability check (1 fails), reverse the order (1 fails).
Register 83 / 12 open / 71 resolved.