diff --git a/.changeset/nested-subselect-ir-completeness.md b/.changeset/nested-subselect-ir-completeness.md new file mode 100644 index 0000000..2b3459e --- /dev/null +++ b/.changeset/nested-subselect-ir-completeness.md @@ -0,0 +1,9 @@ +--- +"@_linked/core": patch +--- + +Preserve nested array sub-select branches in canonical IR so `build()` emits complete traversals, projection fields, and `resultMap` entries for nested selections. + +This fixes cases where nested branches present in `toRawInput().select` were dropped during desugar/lowering (for example nested `friends.select([name, hobby])` branches under another sub-select). + +Also adds regression coverage for desugar preservation, IR lowering completeness, and updated SPARQL golden output for nested query fixtures. diff --git a/docs/agents/skills/automatic/SKILL.md b/docs/agents/skills/automatic/SKILL.md new file mode 100644 index 0000000..40e558e --- /dev/null +++ b/docs/agents/skills/automatic/SKILL.md @@ -0,0 +1,106 @@ +--- +name: automatic +description: Execute the full workflow cycle (ideation -> plan -> tasks -> implementation -> review) without waiting for user confirmation between modes. Use only when the user explicitly requests automatic mode. Never auto-suggest or auto-enter this mode. Pause after review and ask whether to continue with wrapup or iterate. +--- + +# Instructions + +## Trigger + +Run only when the user explicitly requests `automatic` mode. +Do not auto-enter this mode. +Do not suggest this mode in startup mode prompts. + +## Objective + +Run the standard workflow end-to-end without waiting for user confirmation between internal mode transitions: + +1. `ideation` +2. `plan` +3. `tasks` +4. `implementation` +5. `review` + +Then pause. + +## Core behavior + +1. Execute each mode in sequence using the same requirements and artifact rules as the corresponding individual mode skills. +2. Keep momentum: do not stop between internal mode transitions unless blocked by missing mandatory input, hard failures, or safety constraints. +3. In ideation, always document alternatives (at least two viable routes when applicable), evaluate tradeoffs, propose the best route, and explicitly select it before entering plan mode. +4. After selecting the route, continue immediately into plan mode, then tasks mode, then implementation mode, then review mode. +5. Pause after review and ask the user to choose exactly one: + - `wrapup` + - `iterate` + +## Mandatory transition gates + +Never transition to the next mode until the current mode's required on-disk artifacts are complete. + +Before leaving each mode, enforce all checks below: + +1. Ideation -> Plan gate: + - A concrete ideation doc exists/was updated in `docs/ideas/-.md`. + - The doc contains alternatives, tradeoffs, and a clearly selected route. + +2. Plan -> Tasks gate: + - A plan doc exists/was updated in `docs/plans/-.md`. + - The plan includes architecture decisions, expected file changes, pitfalls, and explicit contracts. + - The plan is focused on the chosen route (not a list of all routes). + +3. Tasks -> Implementation gate: + - The same plan doc includes phased tasks. + - Every phase has explicit validation criteria. + - Dependency graph / parallelization notes are present. + +4. Implementation -> Review gate: + - Implementation work is executed phase by phase against the tasked plan. + - Validation for each completed phase is run and recorded. + - The plan doc is updated after each completed phase before moving on. + +If any gate check fails, stop and fix the missing artifact first. Do not implement code before the tasks gate is satisfied. + +## Artifact rules + +Use the same artifact contract as the normal workflow: + +- `ideation`: create/update `docs/ideas/-.md` +- `plan`: create/update `docs/plans/-.md` +- `tasks`: update the active plan doc with phases/tasks/validation +- `implementation`: execute phases and update the active plan doc after completed phases +- `review`: emit findings first, then update plan/docs according to resolved now-vs-future decisions + +Follow numbering and conversion rules from workflow mode. + +## Iterate loop + +If the user chooses `iterate` after review: + +1. Continue with another cycle focused on the review-identified gaps: + - ideation -> plan -> tasks -> implementation -> review +2. Reuse the same active plan document for this loop. +3. Append/refine phases/tasks in that existing plan document instead of creating a new plan file. +4. After the follow-up review, pause again and ask the same decision: + - `wrapup` + - `iterate` + +Repeat until the user selects `wrapup` or stops. + +## Handoff to wrapup + +Do not enter `wrapup` automatically. +Enter `wrapup` only after the user explicitly chooses `wrapup` at a review pause point. + +## Guardrails + +- Do not bypass required validation in implementation. +- Do not skip required artifact updates. +- Do not silently change the chosen route without documenting why. +- If critical ambiguity appears that blocks correct implementation, ask focused clarification questions, then resume automatic progression. +- If implementation was started prematurely, pause and backfill the missing plan/tasks artifacts before continuing. +- Treat missing `docs/plans` as a hard blocker for implementation mode. + +## Exit criteria + +- Reached review mode and paused with explicit `wrapup` vs `iterate` question, or +- User explicitly selected `wrapup` and control has been handed off to wrapup mode. diff --git a/docs/agents/skills/workflow/SKILL.md b/docs/agents/skills/workflow/SKILL.md index 041d95e..086c3f6 100644 --- a/docs/agents/skills/workflow/SKILL.md +++ b/docs/agents/skills/workflow/SKILL.md @@ -1,6 +1,6 @@ --- name: workflow -description: Enforce the explicit mode cadence (ideation -> plan -> tasks -> implementation -> review -> wrapup) with transition gates and optional single-mode overrides. +description: Enforce the explicit mode cadence (ideation -> plan -> tasks -> implementation -> review -> wrapup) with transition gates and optional single-mode overrides, and route explicit automatic-mode requests to the automatic meta workflow. --- # Instructions @@ -12,6 +12,8 @@ description: Enforce the explicit mode cadence (ideation -> plan -> tasks -> imp ## Mode selection at task start - If the user has already explicitly chosen a mode (or explicitly called a mode skill), enter that mode directly. +- `automatic` is a special meta mode and can only be entered when the user explicitly requests it. +- Never auto-suggest `automatic` in startup prompts. - **Auto-enter ideation**: If the user is clearly brainstorming — weighing trade-offs, thinking out loud, exploring options — enter `ideation` mode directly without asking. This exception applies **only to ideation** (the first mode in the sequence). For all other starting modes (plan, implementation, etc.), always ask first. - If the user is not clearly ideating, ask using this prompt pattern: `Should we start with exploring the options in ideation mode? Or do you want to go straight to planning the details in plan mode?` @@ -27,6 +29,10 @@ description: Enforce the explicit mode cadence (ideation -> plan -> tasks -> imp 5. `review` 6. `wrapup` +Meta mode: + +- `automatic` (explicit-only): run the full workflow without user confirmations between internal mode transitions; handoff behavior is defined in `docs/agents/skills/automatic/SKILL.md`. + ## Transition gates Only switch to the next sequential mode with explicit user confirmation. When completing any mode, the agent must ask: `Shall we switch to {name of next mode}?`. Never skip a mode unless explicitly told to. @@ -34,6 +40,7 @@ If user seems to suggest skipping a mode but is not explicitly saying which mode When review identifies remaining work, the agent may loop `review -> tasks -> implementation -> review`, but every switch still requires explicit user confirmation. When review defers work for future scope, the agent may switch `review -> ideation`, with explicit user confirmation. Requests related to PR preparation/publishing are an explicit exception and should route directly to `wrapup` mode. +These transition gates apply to standard modes. `automatic` mode is an explicit exception and manages internal transitions autonomously until its review pause point. ## Required artifacts by mode diff --git a/docs/reports/006-nested-subselect-ir-completeness.md b/docs/reports/006-nested-subselect-ir-completeness.md new file mode 100644 index 0000000..19d06f3 --- /dev/null +++ b/docs/reports/006-nested-subselect-ir-completeness.md @@ -0,0 +1,219 @@ +--- +summary: Final report for preserving nested sub-select paths in canonical IR (desugar -> lowering -> projection/resultMap), plus automatic-mode guardrails and workflow artifacts. +packages: [core] +--- + +# Overview + +This change set fixes an IR fidelity gap where nested sub-select branches were present in `toRawInput().select` but dropped before canonical IR was built. The fix ensures `build()` emits complete canonical IR (patterns + projection + resultMap) for nested array-based sub-select structures. + +In the same scope, wrapup also includes a process hardening update: automatic mode now has explicit transition gates that prevent implementation from starting before ideation/plan/tasks artifacts exist on disk. + +# Original problem + +Observed mismatch: + +- DSL query shape (example): `pluralTestProp.where(...).select([name, friends.select([name, hobby])])` +- Raw pipeline input (`toRawInput().select`) contained both selected branches. +- Canonical IR after build/lowering emitted only the first branch (`... -> name`), dropping nested `friends -> name/hobby`. + +Impact: + +- Canonical IR became under-specified for downstream result materialization. +- Store layers needed compensating augmentation logic to reconstruct missing nested select paths. + +# Final implementation decisions + +## Decision 1: Preserve full sub-select tree in desugar + +Changed the desugar contract so `multi_selection` retains *all* child selection kinds, not only plain `selection_path`. + +Rationale: + +- Information loss happened in desugar before lowering. +- Recovering dropped branches later is brittle and store-coupled. + +## Decision 2: Recurse in lowering for `multi_selection` + +Changed projection seed collection to recurse across nested child selections in `multi_selection`, flattening to `ProjectionSeed[]` only at lowering output. + +Rationale: + +- Keeps lowering aligned with recursive selection semantics. +- Produces complete projection/resultMap without store augmentation. + +## Decision 3: Update affected golden behavior explicitly + +Updated SPARQL golden for `nestedQueries2` to include the newly preserved nested branch (`bestFriend.name`), including required traverse and projected vars. + +Rationale: + +- Golden output must match actual DSL intent once branch loss is fixed. + +# Architecture and pipeline notes + +## Desugar stage (`IRDesugar`) + +Contract change: + +- `DesugaredMultiSelection.selections` now stores `DesugaredSelection[]` (recursive). + +Behavior change: + +- `toSubSelections` no longer filters child selections down to `selection_path` only. +- Mixed nested structures survive desugar intact (e.g., `selection_path` + nested `sub_select`). + +## Lowering stage (`IRLower`) + +Contract change: + +- `collectProjectionSeeds` for `multi_selection` now recursively flattens child `DesugaredSelection` nodes. + +Behavior change: + +- Nested sub-select branches now contribute projection seeds, traverse patterns, and resultMap entries. + +## End-to-end effect + +For nested DSL selections, canonical IR now includes: + +- all required traversals for selected nested branches +- projection entries for nested leaves +- corresponding resultMap aliases/keys + +# Files changed and responsibilities + +## Core pipeline + +- `src/queries/IRDesugar.ts` + - recursive `DesugaredMultiSelection` structure + - preservation of nested children in `toSubSelections` + +- `src/queries/IRLower.ts` + - recursive flattening of `multi_selection` into projection seeds + +## Fixtures and tests + +- `src/test-helpers/query-fixtures.ts` + - new fixture: `pluralFilteredNestedSubSelect` + +- `src/tests/ir-desugar.test.ts` + - new regression: nested `sub_select` retention inside `multi_selection` + +- `src/tests/ir-select-golden.test.ts` + - new regression: lowered IR includes nested traversals/properties for array sub-select branches + - parity table updated with `pluralFilteredNestedSubSelect` + +- `src/tests/sparql-select-golden.test.ts` + - `nestedQueries2` golden updated for preserved nested branch output + +## Workflow/skill docs + +- `docs/agents/skills/automatic/SKILL.md` (new) + - automatic meta-mode definition + - mandatory transition gates to prevent skipping plan/tasks before implementation + +- `docs/agents/skills/workflow/SKILL.md` + - explicit-only automatic mode mention + - transition-gate exception clarified for automatic mode + +- `AGENTS.md` + - workflow note updated to mention automatic explicit-only behavior + +# Public API surface impact + +No new public exports/classes/functions were added to `@_linked/core`. + +Behavioral impact: + +- Queries using nested array-based sub-select compositions now emit more complete canonical IR (and therefore richer SPARQL plans/output) consistent with DSL intent. + +# Validation summary + +Targeted phase validations: + +- `npm test -- --runInBand src/tests/ir-desugar.test.ts -t "preserves nested sub-select paths inside multi-selection arrays"` -> pass +- `npm test -- --runInBand src/tests/ir-select-golden.test.ts -t "build preserves nested sub-select projections inside array selections"` -> pass +- `npm test -- --runInBand src/tests/ir-desugar.test.ts src/tests/ir-select-golden.test.ts src/tests/sparql-select-golden.test.ts` -> pass + +Integration validation: + +- `npm test` -> pass (18 passed suites, 2 skipped, 477 passed tests) +- `npm run sync:agents` -> pass + +# Cleanup and documentation checks + +- Dead code removal: none identified in changed scope. +- Clarifying code comments: none added; current code remained readable without extra inline commentary. +- Documentation coverage: + - workflow/automatic skill docs updated + - plan and report artifacts created + - golden expectations aligned with runtime behavior + +# Process deviations and notes + +- The code implementation landed before strict mode artifact sequencing was completed in automatic mode. +- This was corrected by: + - adding mandatory gates to automatic skill + - backfilling full plan/tasks/implementation tracking in `docs/plans/001-...` + - rerunning and recording validations phase-by-phase + +Residual process gap: + +- Per-phase commits (one commit per phase) were not created during this corrected rerun. + +# Known limitations and remaining gaps + +1. No dedicated integration test yet in this repo for an external store function like `augmentSelectIrFromRawPaths` removal; this repo validates canonical IR and SPARQL generation behavior directly. +2. Existing key-collision behavior is unchanged; newly preserved branches can surface collisions that were previously hidden by dropped paths. + +# Deferred/future work + +No additional deferred ideation doc was created in this scope. + +Potential follow-up (optional): + +- add a focused collision-policy test matrix for nested selections with repeated local property names under different parent branches. + +# PR readiness checklist + +- [x] Core behavior fix implemented +- [x] Regression tests added/updated +- [x] Full test suite green +- [x] Workflow docs updated for automatic mode guardrails +- [ ] Changeset bump level selected (`patch`/`minor`/`major`) +- [ ] Plan removal after report approval + +# Draft PR title + +`Preserve nested sub-select branches in canonical IR lowering` + +# Draft PR body + +## Summary + +This PR fixes a canonical IR fidelity gap where nested array sub-select branches were dropped between desugar and lowering. + +Example shape that now lowers correctly: + +- `pluralTestProp.where(...).select([name, friends.select([name, hobby])])` + +## What changed + +- Preserve recursive `multi_selection` children in desugar. +- Recurse through `multi_selection` in lowering projection seed collection. +- Add regression fixture/test coverage for nested branch preservation. +- Update affected SPARQL golden (`nestedQueries2`) to match preserved nested branch output. +- Add automatic-mode transition gates to prevent skipping plan/tasks before implementation. + +## Validation + +- Targeted desugar/IR/SPARQL suites: pass +- Full `npm test`: pass +- Skill sync (`npm run sync:agents`): pass + +## Notes + +- No new public exports. +- Behavioral change is intentional: canonical IR now includes nested selected paths that were previously dropped. + diff --git a/package-lock.json b/package-lock.json index 02bc724..15a87d4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@_linked/core", - "version": "0.0.0", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@_linked/core", - "version": "0.0.0", + "version": "1.1.0", "license": "MIT", "dependencies": { "next-tick": "^1.1.0", diff --git a/src/queries/IRDesugar.ts b/src/queries/IRDesugar.ts index 23c33d8..02de19b 100644 --- a/src/queries/IRDesugar.ts +++ b/src/queries/IRDesugar.ts @@ -77,7 +77,7 @@ export type DesugaredEvaluationSelect = { export type DesugaredMultiSelection = { kind: 'multi_selection'; - selections: DesugaredSelectionPath[]; + selections: DesugaredSelection[]; }; export type DesugaredSelection = @@ -281,7 +281,7 @@ const toSubSelections = (select: SelectPath): DesugaredSelection => { // Multiple selections in a sub-select return { kind: 'multi_selection' as const, - selections: selections.filter((s): s is DesugaredSelectionPath => s.kind === 'selection_path'), + selections, }; } return toCustomObjectSelect(select); diff --git a/src/queries/IRLower.ts b/src/queries/IRLower.ts index f90e87a..58808d0 100644 --- a/src/queries/IRLower.ts +++ b/src/queries/IRLower.ts @@ -291,11 +291,9 @@ export const lowerSelectQuery = ( } if (selection.kind === 'multi_selection') { - return selection.selections.map((path) => ({ - kind: 'path' as const, - path: combineWithParentPath(parentPath, path), - key, - })); + return selection.selections.flatMap((nestedSelection) => + collectProjectionSeeds(nestedSelection, key, parentPath), + ); } if (selection.kind === 'evaluation_select') { diff --git a/src/test-helpers/query-fixtures.ts b/src/test-helpers/query-fixtures.ts index 72380d1..f17459c 100644 --- a/src/test-helpers/query-fixtures.ts +++ b/src/test-helpers/query-fixtures.ts @@ -286,6 +286,15 @@ export const queryFactories = { p2.bestFriend.select((p3) => ({name: p3.name})), ]), ]), + pluralFilteredNestedSubSelect: () => + Person.select((p) => + p.pluralTestProp + .where((pp) => pp.name.equals('Moa')) + .select((pp) => [ + pp.name, + pp.friends.select((f) => [f.name, f.hobby]), + ]), + ), selectDuplicatePaths: () => Person.select((p) => [ p.bestFriend.name, diff --git a/src/tests/ir-desugar.test.ts b/src/tests/ir-desugar.test.ts index c9f7bf5..4db41d5 100644 --- a/src/tests/ir-desugar.test.ts +++ b/src/tests/ir-desugar.test.ts @@ -36,6 +36,11 @@ const asEvaluation = (s: unknown): DesugaredEvaluationSelect => { return s as DesugaredEvaluationSelect; }; +const asMultiSelection = (s: unknown): DesugaredMultiSelection => { + expect((s as any).kind).toBe('multi_selection'); + return s as DesugaredMultiSelection; +}; + describe('IR desugar conversion', () => { // === Basic selection === @@ -313,6 +318,26 @@ describe('IR desugar conversion', () => { // Should not throw — contains double-nested sub-selects }); + test('preserves nested sub-select paths inside multi-selection arrays', async () => { + const query = await capture(() => queryFactories.pluralFilteredNestedSubSelect()); + const desugared = desugarSelectQuery(query); + + expect(desugared.selections).toHaveLength(1); + const outer = asSubSelect(desugared.selections[0]); + const outerMulti = asMultiSelection(outer.selections); + + expect(outerMulti.selections).toHaveLength(2); + expect(outerMulti.selections[0].kind).toBe('selection_path'); + + const nestedFriends = asSubSelect(outerMulti.selections[1]); + expect(nestedFriends.parentPath).toHaveLength(1); + expect(nestedFriends.parentPath[0].kind).toBe('property_step'); + + const innerMulti = asMultiSelection(nestedFriends.selections); + expect(innerMulti.selections).toHaveLength(2); + expect(innerMulti.selections.every((s) => s.kind === 'selection_path')).toBe(true); + }); + // === Where with query context === test('desugars where with query context', async () => { diff --git a/src/tests/ir-select-golden.test.ts b/src/tests/ir-select-golden.test.ts index eddecc2..0100af7 100644 --- a/src/tests/ir-select-golden.test.ts +++ b/src/tests/ir-select-golden.test.ts @@ -440,6 +440,12 @@ const transformationCases: SelectCase[] = [ minProjection: 1, minPatterns: 1, }, + { + name: "pluralFilteredNestedSubSelect", + run: () => queryFactories.pluralFilteredNestedSubSelect(), + minProjection: 3, + minPatterns: 2, + }, { name: "selectDuplicatePaths", run: () => queryFactories.selectDuplicatePaths(), @@ -647,4 +653,40 @@ describe("IR pipeline behavior", () => { expect(buildSelectQuery(ir)).toBe(ir); }); + + test("build preserves nested sub-select projections inside array selections", async () => { + const query = await captureQuery(() => + queryFactories.pluralFilteredNestedSubSelect() + ); + const ir = buildSelectQuery(query); + + expect(ir.projection.length).toBeGreaterThanOrEqual(3); + expect( + ir.patterns.some( + (p) => + p.kind === "traverse" && + p.property.endsWith("/pluralTestProp") + ) + ).toBe(true); + expect( + ir.patterns.some( + (p) => p.kind === "traverse" && p.property.endsWith("/friends") + ) + ).toBe(true); + + const projectedProperties = ir.projection + .filter( + (item): item is { + alias: string; + expression: {kind: "property_expr"; sourceAlias: string; property: string}; + } => item.expression.kind === "property_expr" + ) + .map((item) => item.expression.property); + expect(projectedProperties.some((prop) => prop.endsWith("/hobby"))).toBe( + true + ); + expect( + projectedProperties.filter((prop) => prop.endsWith("/name")).length + ).toBeGreaterThanOrEqual(2); + }); }); diff --git a/src/tests/sparql-select-golden.test.ts b/src/tests/sparql-select-golden.test.ts index bfd58ee..b63eb6a 100644 --- a/src/tests/sparql-select-golden.test.ts +++ b/src/tests/sparql-select-golden.test.ts @@ -956,13 +956,17 @@ WHERE { const sparql = await goldenSelect(queryFactories.nestedQueries2); expect(sparql).toBe( `PREFIX rdf: -SELECT DISTINCT ?a0 ?a1_firstPet ?a1 +SELECT DISTINCT ?a0 ?a1_firstPet ?a2_name ?a1 ?a2 WHERE { ?a0 rdf:type <${P}> . ?a0 <${P}/friends> ?a1 . + ?a1 <${P}/bestFriend> ?a2 . OPTIONAL { ?a1 <${P}/firstPet> ?a1_firstPet . } + OPTIONAL { + ?a2 <${P}/name> ?a2_name . + } }`); }); });