Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/nested-subselect-ir-completeness.md
Original file line number Diff line number Diff line change
@@ -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.
106 changes: 106 additions & 0 deletions docs/agents/skills/automatic/SKILL.md
Original file line number Diff line number Diff line change
@@ -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/<nnn>-<topic>.md`.
- The doc contains alternatives, tradeoffs, and a clearly selected route.

2. Plan -> Tasks gate:
- A plan doc exists/was updated in `docs/plans/<nnn>-<topic>.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/<nnn>-<topic>.md`
- `plan`: create/update `docs/plans/<nnn>-<topic>.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.
9 changes: 8 additions & 1 deletion docs/agents/skills/workflow/SKILL.md
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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?`
Expand All @@ -27,13 +29,18 @@ 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.
If user seems to suggest skipping a mode but is not explicitly saying which mode to use, then the agent must ask the user `Do you want to continue with {name of next mode} or continue straight to {user suggested 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

Expand Down
219 changes: 219 additions & 0 deletions docs/reports/006-nested-subselect-ir-completeness.md
Original file line number Diff line number Diff line change
@@ -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.

4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading