You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Please work on this issue: https://github.com/dakusui/jqplusplus/issues/84
Try not to ask me questions as much as possible but figure out questions from what you can see on the
issue and what you have in the repo.
Commit changes for each phase and for change after the defined phases.
Prefix the pull request's name with {your model name such as sol, terra, or luna}-{effort level}: but
please do not merge it as I will do that by myself.
Also, post a comment at the end of the work to record the tokens consumed for this work.
You may find other pull requests that try to fix the same issue, but please ignore them because they
may be depending on invalid assumptions due to my past tries.
Summary
Add opt-in inherited-array composition with "$super" splice and "$super*" element-wise pairing.
Retain unresolved splice deltas in the node-pool cache and ground markers only after all inheritance resolves.
PR #86 (3e4d55f) implements issue #84, the specification that supersedes #74. The important clarified rules are deferred grounding of pending deltas and rejection of marker-family strings outside direct array elements.
Executive summary
Recommended; no correctness blocker found. PR #86 is a strong, specification-faithful implementation. It correctly treats a $super array as a substitutable pending delta, while correctly refusing any composition involving a $super* delta. It also rejects the $super*-parent / $super-child case that exposed PR #81's deferred-flattening defect.
It is close to PR #85 in semantic quality. PR #86 has the clearer end-to-end test for the two equivalent associativity layouts. PR #85 remains slightly ahead overall on review confidence because it has a substantive unit-test layer, a separate syntax-reference update, and more finely partitioned autotests. PR #86 needs test and polish follow-up, not a redesigned core.
Evidence
Reviewed head: 3e4d55f, based on 1956c65.
go test ./..., go test -race ./..., and go vet ./... pass.
make build and the full autotest suite pass: 181 passed, 0 failed.
GroundArrayComposition runs at the right lifecycle point: after file- and node-level inheritance, before key-side evaluation. It validates keys, scalar values, nested arrays, filenames, unknown $super extensions, and raw: escapes. This both preserves cached fragment intent and enforces #84's reserved-namespace rule.
I found no semantic flaw in the reviewed #84 scope. In particular, the targeted $super* parent → $super child → later-base reproduction errors at the composition boundary rather than accidentally succeeding after flattening.
1.2 Design clarity
The dedicated internal/array_composition.go is a good feature boundary. Its small arrayComposition classification (none, splice, pair) makes the merge decision legible, and the central incompatibility condition expresses the specification's actual state rule rather than broadly rejecting marked arrays. Propagating errors from the inheritance merge is the right integration point for both file- and node-level resolution.
The grounding walk is compact, but parentIsArray is indirect: it really means that the current array is nested inside another array, where markers are out of context. A context enum, as in PR #85, would be more self-documenting and make it easier to retain precise error paths.
1.3 Readability
Names consistently distinguish splice, pair, composition kind, grounding, and raw strings. Slice construction and the pair-rule switch are straightforward to audit. Comments explain the subtle cache-safety and deferred-grounding decisions.
The main readability improvement would be location-rich errors. Merge errors receive a key prefix, but grounding errors do not include a document path, and Go map iteration makes the first reported invalid key nondeterministic when several keys are invalid.
1.4 Efficiency
Splice and pairing create a fresh outer slice while sharing immutable JSON-like elements. This prevents node-pool cache corruption without recursive copying. Capacities are sized from the splice count or longest paired side, so the hot composition paths do not show an avoidable allocation.
Grounding scans direct arrays twice (classification, then recursive validation). It is a modest and reasonable cost for configuration data; no benchmark was supplied, so this is a structural rather than measured efficiency conclusion.
2. Tests
2.1 Coverage
The 26 autotests map well to #84 Phase 0. Positives cover every splice position and repeated splice, default replacement, $includes delta carry, node-level inheritance, pairing prefix/queue/tails and {} padding, raw values and keys, raw filenames, and both equivalent associativity layouts. The layout-A/layout-B pair is an especially good executable form of #84 section 4.3.
Negatives cover unresolved deltas; mixed and duplicate markers; all six cross-kind pairing combinations; non-array inheritance; all three combinations in which a $super* delta meets a marked delta; unknown spellings; and markers in a key, scalar, nested inner array, or filename.
The key gap is the lack of dedicated feature unit tests. The unit-test diff only adjusts an existing deep-merge test for the new error return. Slice freshness, direct merge behavior, recursively marked child-array pairs, marker classification, and both incompatible transitions are therefore covered only by slower end-to-end fixtures. PR #85 offers a strong model for filling this gap.
Please also add a $super* parent → $super child case with a later concrete base array. marked-superstar-inherited verifies the immediate error, but the later-base variant proves rejection happens at delta composition rather than only at final grounding. It locks in the exact defect that made #81 look correct under ordinary tests.
2.2 Design clarity
Test directories are concise and mostly single-purpose. The grouped splice and pairing fixtures make related examples easy to read together, while the matching associativity fixtures deliberately share an expected result. That is excellent specification design.
The trade-off is diagnosis granularity: a failure in the grouped splice or pairing fixture needs inspection of several fields to isolate the rule. A separate valid recursively marked child-array fixture would also make #84's recursive array-pair rule visible.
2.3 Readability
Fixtures are compact JSON with descriptive directory names. Negative cases assert meaningful error categories rather than exact full messages, avoiding coupling to harmless diagnostic changes. Focused unit-test names are the notable missing layer for future maintainers.
3. Documentation
3.1 Readability and organization
The documentation follows the repository's terminology rules. evaluation-model.adoc moves clearly from default array replacement through explicit composition, deferred deltas, grounding, and invalid combinations; terminology.adoc defines the new concepts. Examples make append, prepend, wrapping, pairing, and raw: understandable.
Unlike PR #85, PR #86 does not add a syntax-reference entry. That is not a correctness problem, but a concise syntax summary would improve discoverability for readers who do not start with the conceptual evaluation model.
3.2 Correctness
The AsciiDoc source matches #84 and the code: replacement remains the default, splice deltas carry and compose, pairing cannot compose with any marked delta, cross-kind pairs error, grounding follows inheritance, and out-of-context markers require raw: escaping. It also correctly places the eval: exemption after grounding.
Verdict
Recommended with test follow-up, not blocked by a semantic defect. PR #86 is substantially stronger than #78–#81 under the current #84 specification. It is comparable to PR #85 in implementation correctness; #85 retains a narrow overall lead because its unit tests and documentation provide more durable review evidence, while #86 has the better explicit associativity-layout regression.
Before merge, add the later-base $super* → $super regression and a small internal/array_composition_test.go layer covering marker classification, fresh slices, marked nested-array pairing, and both incompatible transitions.
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
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.
Closes #84
prompt
Summary
"$super"splice and"$super*"element-wise pairing.raw:escapes in values, keys, and inheritance filenames.Validation
go test ./...make build && tools/bin/autotest(181/181 passed)make doc