PR-09: parity frames CK API (#480)#509
Open
charliecreates[bot] wants to merge 1 commit intocharlie/issue-454-v2-pr09-error-state-group08from
Open
PR-09: parity frames CK API (#480)#509charliecreates[bot] wants to merge 1 commit intocharlie/issue-454-v2-pr09-error-state-group08from
charliecreates[bot] wants to merge 1 commit intocharlie/issue-454-v2-pr09-error-state-group08from
Conversation
|
orrery preview: https://pr-509.orrery-c4f.pages.dev/ |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
Contributor
Author
There was a problem hiding this comment.
The native CSPICE runner additions are substantial, but there are correctness and safety gaps: frames.ckcov has a concrete stack buffer overflow risk when intervalCount > 256, and ckgp/ckgpav depend on implicit, potentially non-deterministic selection of instrument id and coverage interval. Additionally, frames.ckupf currently behaves as load+unload by path rather than unloading an existing handle, which can diverge from real API semantics. Finally, the new v2 YAML specs don’t assert result shapes (empty properties), reducing the parity suite’s ability to catch contract regressions.
Additional notes (1)
- Readability |
packages/parity-checking/specs/methods/frames/ckgpav@v2.yml:1-22
ckgpandckgpavspecs passMGS_SPACECRAFTas therefargument. In CSPICE,refis typically a reference frame name (e.g.,J2000,MGS_SPACECRAFT, etc.), so this may be fine—but these tests currently rely on the runner’s implicit choice ofinstandsclkdp/tol(computed from coverage). If the chosen instrument’s frame does not matchref, you can get SPICE errors orfound=falsein ways that are hard to diagnose.
At minimum, the runner should surface chosen inst and sclkdp to make failures actionable.
Summary of changes
What changed
Parity coverage & catalogs
- Removed CK-related frames methods from the parity denylist (
frames.ckcov,frames.ckgp,frames.ckgpav,frames.cklpf,frames.ckobj,frames.ckupf) in:packages/parity-checking/catalogs/parity-denylist.jsonpackages/parity-checking/catalogs/parity-denylist.ts
- Updated completeness baselines to reflect the new coverage:
BASELINE_CANONICAL_METHOD_COVERAGE:143 → 149MAX_BASELINE_DENYLIST_SIZE:30 → 24
Native CSPICE runner: new CK frames calls
- Added new
CallIds and string dispatch mapping for CK frames functions inpackages/parity-checking/native/src/cspice_runner.c:ckgp,ckgpav,cklpf,ckupf,ckobj,ckcov
- Implemented JSON argument parsing and result serialization for these calls.
TS runner dispatch
- Added
tspiceRunnerdispatch entries for CK frames methods and their short aliases (frames.ckgp+ckgp, etc.) inpackages/parity-checking/src/runners/tspiceRunner.ts.
Method specs & workflow
- Added v2 parity method specs:
packages/parity-checking/specs/methods/frames/{ckcov,ckgp,ckgpav,cklpf,ckobj,ckupf}@v2.yml
- Added a legacy workflow fixture pack:
packages/parity-checking/workflows/legacy/frames.ck.basic@v1.yml
Tests updated
- Updated expected counts in:
packages/parity-checking/test/integration/completenessGuard.test.tspackages/parity-checking/test/parity.e2e.test.tspackages/parity-checking/test/parityScenarioCoverage.test.ts
Comment on lines
+5584
to
+5915
| case CALL_CKGP: { | ||
| if (tokens[argsTok].size < 2) { | ||
| write_error_json_ex( | ||
| "invalid_args", | ||
| "frames.ckgp expects args[0]=string ckPath args[1]=string ref", | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| const int pathTok = jsmn_get_array_elem(tokens, argsTok, 0, tokenCount); | ||
| const int refTok = jsmn_get_array_elem(tokens, argsTok, 1, tokenCount); | ||
| if (pathTok < 0 || pathTok >= tokenCount || tokens[pathTok].type != JSMN_STRING) { | ||
| write_error_json_ex("invalid_args", "frames.ckgp expects args[0] to be a string", NULL, NULL, NULL, NULL); | ||
| goto done; | ||
| } | ||
| if (refTok < 0 || refTok >= tokenCount || tokens[refTok].type != JSMN_STRING) { | ||
| write_error_json_ex("invalid_args", "frames.ckgp expects args[1] to be a string", NULL, NULL, NULL, NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| char *path = NULL; | ||
| char *ref = NULL; | ||
|
|
||
| strDetail[0] = '\0'; | ||
| const jsmn_strdup_err_t pathErr = | ||
| jsmn_strdup(input, &tokens[pathTok], &path, strDetail, sizeof(strDetail)); | ||
| if (pathErr != JSMN_STRDUP_OK) { | ||
| if (pathErr == JSMN_STRDUP_INVALID) { | ||
| write_error_json_ex("invalid_request", "Invalid JSON string escape", | ||
| strDetail[0] ? strDetail : NULL, NULL, NULL, NULL); | ||
| } else { | ||
| write_error_json("Out of memory", NULL, NULL, NULL); | ||
| } | ||
| goto done; | ||
| } | ||
|
|
||
| strDetail[0] = '\0'; | ||
| const jsmn_strdup_err_t refErr = | ||
| jsmn_strdup(input, &tokens[refTok], &ref, strDetail, sizeof(strDetail)); | ||
| if (refErr != JSMN_STRDUP_OK) { | ||
| free(path); | ||
| if (refErr == JSMN_STRDUP_INVALID) { | ||
| write_error_json_ex("invalid_request", "Invalid JSON string escape", | ||
| strDetail[0] ? strDetail : NULL, NULL, NULL, NULL); | ||
| } else { | ||
| write_error_json("Out of memory", NULL, NULL, NULL); | ||
| } | ||
| goto done; | ||
| } | ||
|
|
||
| SPICEINT_CELL(ids, 128); | ||
| ckobj_c(path, &ids); | ||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| free(path); | ||
| free(ref); | ||
| write_error_json("SPICE error in ckobj (frames.ckgp setup)", shortMsg, longMsg, | ||
| traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceInt nIds = card_c(&ids); | ||
| if (nIds <= 0) { | ||
| free(path); | ||
| free(ref); | ||
| write_error_json_ex( | ||
| "invalid_args", | ||
| "frames.ckgp expected args[0] CK path to contain at least one object id", | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceInt inst = ((const SpiceInt *)ids.data)[0]; | ||
|
|
||
| SPICEDOUBLE_CELL(cover, 512); | ||
| ckcov_c(path, inst, SPICEFALSE, "SEGMENT", 0.0, "SCLK", &cover); | ||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| free(path); | ||
| free(ref); | ||
| write_error_json("SPICE error in ckcov (frames.ckgp setup)", shortMsg, longMsg, | ||
| traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceInt intervalCount = wncard_c(&cover); | ||
| if (intervalCount < 1) { | ||
| free(path); | ||
| free(ref); | ||
| write_error_json_ex( | ||
| "invalid_args", | ||
| "frames.ckgp expected args[0] CK path to have at least one SCLK coverage interval", | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| SpiceDouble left = 0.0; | ||
| SpiceDouble right = 0.0; | ||
| wnfetd_c(&cover, 0, &left, &right); | ||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| free(path); | ||
| free(ref); | ||
| write_error_json("SPICE error in wnfetd (frames.ckgp setup)", shortMsg, longMsg, | ||
| traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceDouble sclkdp = (left + right) / 2.0; | ||
| SpiceDouble tol = (right - left) / 2.0; | ||
| if (tol < 1.0) { | ||
| tol = 1.0; | ||
| } | ||
|
|
||
| SpiceDouble cmat[3][3]; | ||
| SpiceDouble clkout = 0.0; | ||
| SpiceBoolean found = SPICEFALSE; | ||
| ckgp_c(inst, sclkdp, tol, ref, cmat, &clkout, &found); | ||
| free(path); | ||
| free(ref); | ||
|
|
||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| write_error_json("SPICE error in ckgp", shortMsg, longMsg, traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| if (found != SPICETRUE) { | ||
| fputs("{\"ok\":true,\"result\":{\"found\":false}}\n", stdout); | ||
| goto done; | ||
| } | ||
|
|
||
| fputs("{\"ok\":true,\"result\":{\"found\":true,\"cmat\":", stdout); | ||
| json_print_mat3_rowmajor(cmat); | ||
| fputs(",\"clkout\":", stdout); | ||
| fprintf(stdout, "%.17g", (double)clkout); | ||
| fputs("}}\n", stdout); | ||
| goto done; | ||
| } | ||
|
|
||
| case CALL_CKGPAV: { | ||
| if (tokens[argsTok].size < 2) { | ||
| write_error_json_ex( | ||
| "invalid_args", | ||
| "frames.ckgpav expects args[0]=string ckPath args[1]=string ref", | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| const int pathTok = jsmn_get_array_elem(tokens, argsTok, 0, tokenCount); | ||
| const int refTok = jsmn_get_array_elem(tokens, argsTok, 1, tokenCount); | ||
| if (pathTok < 0 || pathTok >= tokenCount || tokens[pathTok].type != JSMN_STRING) { | ||
| write_error_json_ex("invalid_args", "frames.ckgpav expects args[0] to be a string", NULL, NULL, NULL, NULL); | ||
| goto done; | ||
| } | ||
| if (refTok < 0 || refTok >= tokenCount || tokens[refTok].type != JSMN_STRING) { | ||
| write_error_json_ex("invalid_args", "frames.ckgpav expects args[1] to be a string", NULL, NULL, NULL, NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| char *path = NULL; | ||
| char *ref = NULL; | ||
|
|
||
| strDetail[0] = '\0'; | ||
| const jsmn_strdup_err_t pathErr = | ||
| jsmn_strdup(input, &tokens[pathTok], &path, strDetail, sizeof(strDetail)); | ||
| if (pathErr != JSMN_STRDUP_OK) { | ||
| if (pathErr == JSMN_STRDUP_INVALID) { | ||
| write_error_json_ex("invalid_request", "Invalid JSON string escape", | ||
| strDetail[0] ? strDetail : NULL, NULL, NULL, NULL); | ||
| } else { | ||
| write_error_json("Out of memory", NULL, NULL, NULL); | ||
| } | ||
| goto done; | ||
| } | ||
|
|
||
| strDetail[0] = '\0'; | ||
| const jsmn_strdup_err_t refErr = | ||
| jsmn_strdup(input, &tokens[refTok], &ref, strDetail, sizeof(strDetail)); | ||
| if (refErr != JSMN_STRDUP_OK) { | ||
| free(path); | ||
| if (refErr == JSMN_STRDUP_INVALID) { | ||
| write_error_json_ex("invalid_request", "Invalid JSON string escape", | ||
| strDetail[0] ? strDetail : NULL, NULL, NULL, NULL); | ||
| } else { | ||
| write_error_json("Out of memory", NULL, NULL, NULL); | ||
| } | ||
| goto done; | ||
| } | ||
|
|
||
| SPICEINT_CELL(ids, 128); | ||
| ckobj_c(path, &ids); | ||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| free(path); | ||
| free(ref); | ||
| write_error_json("SPICE error in ckobj (frames.ckgpav setup)", shortMsg, longMsg, | ||
| traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceInt nIds = card_c(&ids); | ||
| if (nIds <= 0) { | ||
| free(path); | ||
| free(ref); | ||
| write_error_json_ex( | ||
| "invalid_args", | ||
| "frames.ckgpav expected args[0] CK path to contain at least one object id", | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceInt inst = ((const SpiceInt *)ids.data)[0]; | ||
|
|
||
| SPICEDOUBLE_CELL(cover, 512); | ||
| ckcov_c(path, inst, SPICEFALSE, "SEGMENT", 0.0, "SCLK", &cover); | ||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| free(path); | ||
| free(ref); | ||
| write_error_json("SPICE error in ckcov (frames.ckgpav setup)", shortMsg, longMsg, | ||
| traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceInt intervalCount = wncard_c(&cover); | ||
| if (intervalCount < 1) { | ||
| free(path); | ||
| free(ref); | ||
| write_error_json_ex( | ||
| "invalid_args", | ||
| "frames.ckgpav expected args[0] CK path to have at least one SCLK coverage interval", | ||
| NULL, | ||
| NULL, | ||
| NULL, | ||
| NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| SpiceDouble left = 0.0; | ||
| SpiceDouble right = 0.0; | ||
| wnfetd_c(&cover, 0, &left, &right); | ||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| free(path); | ||
| free(ref); | ||
| write_error_json("SPICE error in wnfetd (frames.ckgpav setup)", shortMsg, longMsg, | ||
| traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| const SpiceDouble sclkdp = (left + right) / 2.0; | ||
| SpiceDouble tol = (right - left) / 2.0; | ||
| if (tol < 1.0) { | ||
| tol = 1.0; | ||
| } | ||
|
|
||
| SpiceDouble cmat[3][3]; | ||
| SpiceDouble av[3] = {0.0, 0.0, 0.0}; | ||
| SpiceDouble clkout = 0.0; | ||
| SpiceBoolean found = SPICEFALSE; | ||
| ckgpav_c(inst, sclkdp, tol, ref, cmat, av, &clkout, &found); | ||
| free(path); | ||
| free(ref); | ||
|
|
||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| write_error_json("SPICE error in ckgpav", shortMsg, longMsg, traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| if (found != SPICETRUE) { | ||
| fputs("{\"ok\":true,\"result\":{\"found\":false}}\n", stdout); | ||
| goto done; | ||
| } | ||
|
|
||
| fputs("{\"ok\":true,\"result\":{\"found\":true,\"cmat\":", stdout); | ||
| json_print_mat3_rowmajor(cmat); | ||
| fputs(",\"av\":", stdout); | ||
| json_print_double_array(av, 3); | ||
| fputs(",\"clkout\":", stdout); | ||
| fprintf(stdout, "%.17g", (double)clkout); | ||
| fputs("}}\n", stdout); | ||
| goto done; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
CALL_CKGP/CALL_CKGPAV silently pick the first object id returned by ckobj_c and the first coverage interval (wnfetd_c(..., 0, ...)). That makes results dependent on CK contents/order and can be non-deterministic across kernels or even across SPICE versions if ordering changes. It also hides the case where a CK contains multiple instruments with different coverage.
This is especially risky for parity since it can pass while validating the wrong instrument/interval.
Suggestion
Make instrument/interval selection explicit and deterministic. Options:
-
Extend API args to accept
instand/orsclkdp(and possiblytol), avoiding introspection entirely. -
If keeping introspection, at least:
- sort
ids(or choose min id) deterministically, - choose the largest interval or the midpoint of the union of all intervals, and
- return
instand chosen interval in the JSON result for transparency.
For example:
- pick
inst = ((SpiceInt*)ids.data)[0]only afterckobj_coutput is ordered deterministically. - compute
left/rightby iterating all intervals and selecting the widest.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing deterministic selection and returning the chosen inst + interval metadata.
Comment on lines
+5973
to
+6027
| case CALL_CKUPF: { | ||
| if (tokens[argsTok].size < 1) { | ||
| write_error_json_ex("invalid_args", "frames.ckupf expects args[0] to be a string", NULL, NULL, NULL, NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| const int pathTok = jsmn_get_array_elem(tokens, argsTok, 0, tokenCount); | ||
| if (pathTok < 0 || pathTok >= tokenCount || tokens[pathTok].type != JSMN_STRING) { | ||
| write_error_json_ex("invalid_args", "frames.ckupf expects args[0] to be a string", NULL, NULL, NULL, NULL); | ||
| goto done; | ||
| } | ||
|
|
||
| char *path = NULL; | ||
| strDetail[0] = '\0'; | ||
| const jsmn_strdup_err_t pathErr = | ||
| jsmn_strdup(input, &tokens[pathTok], &path, strDetail, sizeof(strDetail)); | ||
| if (pathErr != JSMN_STRDUP_OK) { | ||
| if (pathErr == JSMN_STRDUP_INVALID) { | ||
| write_error_json_ex("invalid_request", "Invalid JSON string escape", | ||
| strDetail[0] ? strDetail : NULL, NULL, NULL, NULL); | ||
| } else { | ||
| write_error_json("Out of memory", NULL, NULL, NULL); | ||
| } | ||
| goto done; | ||
| } | ||
|
|
||
| SpiceInt handle = 0; | ||
| cklpf_c(path, &handle); | ||
| free(path); | ||
|
|
||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| write_error_json("SPICE error in cklpf (frames.ckupf setup)", shortMsg, longMsg, | ||
| traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| ckupf_c(handle); | ||
| if (failed_c() == SPICETRUE) { | ||
| char shortMsg[1841]; | ||
| char longMsg[1841]; | ||
| char traceMsg[1841]; | ||
| capture_spice_error(shortMsg, sizeof(shortMsg), longMsg, sizeof(longMsg), traceMsg, | ||
| sizeof(traceMsg)); | ||
| write_error_json("SPICE error in ckupf", shortMsg, longMsg, traceMsg); | ||
| goto done; | ||
| } | ||
|
|
||
| fputs("{\"ok\":true,\"result\":null}\n", stdout); | ||
| goto done; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
CALL_CKUPF does not actually "unload" a CK handle provided by the caller; instead it loads the CK again via cklpf_c(path, &handle) and immediately calls ckupf_c(handle). This changes the semantics to "load+unload", not "unload".
That may be intentional for parity convenience, but it no longer tests the real API surface/behavior (e.g., unloading an existing handle) and could mask handle-lifetime bugs in callers.
Suggestion
Align parity behavior with the real ckupf contract:
- Accept
handleas an integer argument and callckupf_c(handle)directly. - If you need a convenience helper, introduce a separate parity-only method like
frames.ckupfFromPath(or keepframes.cklpfreturning a handle so tests can callckupf).
This will make the parity runner validate the same lifecycle contract users rely on.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit switching frames.ckupf to accept a numeric handle and updating the spec/tests accordingly.
Comment on lines
+1
to
+22
| schemaVersion: 2 | ||
| manifest: | ||
| id: methods/frames/ckgp@v2 | ||
| kind: method | ||
| contract: | ||
| contractMethod: frames.ckgp | ||
| canonicalMethod: frames.ckgp | ||
| aliases: [] | ||
| result: | ||
| type: object | ||
| properties: {} | ||
| setup: | ||
| kernels: | ||
| - $FIXTURES/mgs-minimal | ||
| workflow: | ||
| steps: | ||
| - op: invokeLegacyCall | ||
| cases: | ||
| - id: frames.ckgp.mgs-minimal | ||
| args: | ||
| - $FIXTURES/mgs-minimal/mgs_hga_hinge_v2.bc | ||
| - MGS_SPACECRAFT |
Contributor
Author
There was a problem hiding this comment.
The new v2 method specs declare result.type: object with empty properties: {} for calls that clearly return structured data (ckgp returns found, cmat, clkout; ckgpav adds av; ckcov returns idcode, intervals, etc.).
With empty result schemas, you’re not asserting output shape at all, which reduces the value of parity tests and can let breaking JSON contract changes slip through.
Suggestion
Define the expected result schema properties for each method spec so parity verifies structure, not just that the call succeeds.
For frames.ckgp, something like:
found: boolean- when
found: true:cmat: number[9](row-major),clkout: number
For frames.ckcov:
idcode: integerintervals: number[]with even lengthlevel: string,timsys: string,needav: boolean
If conditional schemas are supported, use oneOf/if/then to model found.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit filling in schemas for all six new v2 specs.
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.
Stack step PR-09 in the issue #454 migration chain.