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
Fold postfix chains into a fused RowPlan executed in plasm-runtime, replace the JSON walker, and ship .with{k: expr} with now, len, when, arith (/ and string +), and temporal day diffs.
Co-authored-by: Cursor <cursoragent@cursor.com>
…RowPlan
Document the new .with{col: expr} postfix in the canonical language definition
(EBNF, invariants, pitfalls) and expand plasm-row-compute with expression
semantics, chaining order, disambiguation, and RowPlan/Polars execution notes.
Add glossary entries for .with and RowPlan.
Co-authored-by: Ryan Roberts <ryan-s-roberts@users.noreply.github.com>
Verdict: The PR's doc updates are substantially aligned with the implementation. Commits a3cb9688 / 3bc99d88 correctly extend the canonical language spec, row-compute user guide, glossary, concepts overview, and plasm_tool.txt. Fenced doc examples parse under the language-matrix fixture (doc_fenced_plasm_examples_parse_under_language_matrix ✅).
What landed well
Single-source grammar: EBNF WITH_EXPR delegates to plasm-row-compute.md#derived-columns-with — avoids duplicating expression semantics in two places.
Layering: Formal grammar + invariants stay in plasm-language-definition.md; agent-oriented examples and chaining guidance stay in plasm-row-compute.md.
Training surface:plasm_tool.txt postfix list includes .with{k: expr}.
Runtime architecture: New "Execution engine" section documents RowPlan → Polars adapter and collect barriers — matches plasm_core::row_plan / plasm_runtime::row_compute.
Gaps found (and follow-up)
Issue
Antipattern
Severity
program_param.txt still omits .with while plasm_tool.txt was updated
Maintenance Nightmare (split training assets)
High — agents read both prompts
.dedupe / .distinct added to language invariants + EBNF but absent from plasm-row-compute.md on this branch
Easter Egg Hunt
Medium
WITH_EXPR operator precedence (*// before +/-) not documented
Branch cursor/system-documentation-quality-4a43 (based on this PR) adds:
.with to program_param.txt
Dedupe/distinct section + precedence note in plasm-row-compute.md
Binding-forms / suffix-pipeline touch-ups in plasm-language-definition.md
Row compute quick link in doc-site/docs/index.md
Cherry-pick or merge that commit into feat/polars-row-compute before merge.
Holistic note
doc-site/docs/crates/index.md still describes plasm-runtime generically — consider one clause mentioning row_compute now that row plans execute there. Parser-modules table in the language definition could reference plasm_core::row_plan::with_parse alongside expr_parser/postfix.rs for .with body parsing.
The reason will be displayed to describe this comment to others. Learn more.
Training-set split:plasm_tool.txt now lists .with{k: expr}, but sibling asset program_param.txt (same postfix bullet, line 8) was not updated in this PR. Agents receive both strings on different tool paths — please sync, or you'll get contradictory training (Maintenance Nightmare). Fixed on branch cursor/system-documentation-quality-4a43.
| Search filter | `e#~"…"{field=…}` | wire is the **Search**-capability param (homograph-safe vs Create/Update params) |
| Relation hop | `receiver.r#` (or wire) | `r#` resolves to a declared relation wire; a filter wire after `.` yields `RelationSegmentWrongRole` except LHS-binding coercion (see [Binding RHS shapes](#binding-rhs-shapes-label--)) |
| Projection / postfix | `[field,…]`, `.sort(field)`, `.group_by(field)`, … | wire names resolve to `rows:` field symbols under the row entity |
| Projection / postfix | `[field,…]`, `.sort(field)`, `.group_by(field)`, `.with{col: expr}`, `.dedupe(…)`, `.distinct(…)`, … | wire names resolve to `rows:` field symbols under the row entity |
The reason will be displayed to describe this comment to others. Learn more.
Invariants and EBNF now list .dedupe(…) / .distinct(…) alongside .with, but plasm-row-compute.md on this branch still has no user-facing section for dedupe/distinct (only filter / group / .with). Agents looking at the row-compute guide won't find these ops — Easter Egg Hunt. Recommend a short dedupe section in row-compute (formal EBNF stays here).
The reason will be displayed to describe this comment to others. Learn more.
Expression table is good, but WITH_EXPR precedence is unspecified: implementation parses *// tighter than +/- (see row_plan/with_parse.rs). Worth one line here so score * 2 + 1 vs (score + 1) * 2 isn't guesswork.
Row compute lowers fused [`ComputeOp`](https://github.com/PlasmTools/plasm-core/blob/main/crates/plasm-core/src/plasm_monad/payload/compute.rs) chains to a [`RowPlan`](https://github.com/PlasmTools/plasm-core/blob/main/crates/plasm-core/src/row_plan/plan.rs) IR in `plasm_core::row_plan`, then executes through a Polars-backed adapter in `plasm_runtime::row_compute`. Collect barriers (program return, paging, invoke-arg holes, render) are the only legal materialization points — render and derive remain outside the fused pipeline.
The reason will be displayed to describe this comment to others. Learn more.
Execution engine section is accurate and welcome. Consider cross-linking collect barrier semantics back to program-return / paging docs (plasm-long-operations.md) so readers know when materialization happens, not just where in the crate graph.
Standardize agent/runtime instrumentation, wire sqlx-tracing and HTTP MakeSpan,
and lock parent/child lineage with force-flush tests on real call sites.
Co-authored-by: Cursor <cursoragent@cursor.com>
Align program_param.txt with plasm_tool.txt (.with, dedupe/distinct).
Document dedupe/distinct and .with operator precedence in row-compute
guide; extend language spec binding/suffix/parser tables; fix stale
matrix doc path; add row-compute quick links; note RowPlan execution in
crates index; document OTLP semantic span name contracts in plasm-otel.
Co-authored-by: Ryan Roberts <ryan-s-roberts@users.noreply.github.com>
- List .dedupe/.distinct in plasm_tool.txt Core surface (parity with program_param)
- Clarify .with RowIdentity and chained-RHS continuation in row-compute guide
- Cross-link OTLP semantic span contract from oss-core-trace-artifacts and crates index
- Align language spec binding table with row-compute continuation rules
Co-authored-by: Ryan Roberts <ryan-s-roberts@users.noreply.github.com>
Reviewed doc-site pages, training prompts, and implementation (binding_contract.rs, row_plan/*, with_parse.rs, language matrix) against the PR diff.
What landed well
Split, not sprawl:.with expression semantics live in plasm-row-compute.md; the language definition keeps EBNF + pointers — good single-sourcing.
Surface coverage: EBNF gains with / dedupe / distinct; invariants, binding table, suffix pipeline, parser-module table, and agent pitfalls were updated.
Agent-facing prompts:plasm_tool.txt / program_param.txt now teach .with, .dedupe, .distinct.
Discoverability: glossary entries, index quick-link, mkdocs nav, and crates index mention RowPlan / Polars execution.
Trace lane:oss-core-trace-artifacts.md now points operators at the plasm-otel span-name contract.
Gaps vs grammar / semantics (actionable)
Continuation contract still incomplete in the canonical spec.binding_contract.rs assigns ContinuationCapability::Terminal to compute bindings whose last op is .filter, .with, .sort, .dedupe, .group_by, or .aggregate — only [projection] and .limit inherit the parent’s RelationDot anchor. The Continuation rules block (~L541–551) still says “row-preserving postfix — projection, .limit(1), .singleton()” and never tabulates this. Agents will keep writing filtered = issues.filter{…} then labels = filtered.labels unless the spec states the rule once, with a link from row-compute.
Row-compute “terminal” wording is .with-specific in places; .filter / .sort obey the same relation-dot terminal rule (while filtered.with{…} on the next line still works because the RHS is a full expression, not bare label.relation). Consolidate into one subsection.
OTel span inventory: this PR adds many plasm_agent.* spans (oauth_link.*, plan.dry_run, plan.step_materialize, …) and span_graph_tests contracts. The doc-site only links to the README examples table, which lists three illustrative names — not the expanded contract operators need for dashboards.
Minor navigation / maintenance (outside this diff):concepts.md mentions .with but only links the language definition; crates/plasm-core/README.md still points at the old monorepo path for the language spec.
Antipattern check
Pattern
Verdict
Everything Document
OK — row plane split to plasm-row-compute.md
Easter Egg Hunt
Mostly OK — cross-links exist; continuation table would reduce hunting
Assumption Gap
Fix continuation + concepts → row-compute link
Maintenance Nightmare
EBNF delegates WITH_EXPR to row-compute (good); chaining order duplicated lightly
Corporate Speak
Clean, operational prose throughout
Recommend one follow-up doc commit: continuation contract table in plasm-language-definition.md + a Relation-dot terminal bindings subsection in plasm-row-compute.md that generalizes beyond .with.
**Row identity (`RowIdentity`)** — every row-producing plan node carries a canonical identity handle (qualified entity + [`Ref`] + ambient scope slots) in materialization, not only JSON payload. Projection and `.limit(1)` preserve identity when the suffix pipeline folds [`RowSuffix`] segments; [`PlasmInputRef::NodeInput`] holes resolve via identity, not stripped JSON paths.
**Suffix pipeline** — after the path head (Get/Query/label), dot/bracket segments classify as [`RowSuffix`] (relation, limit, project, sort, …) and lower through one fold (`lower_suffix_stream`), including interleaved forms such as `repo.commits.limit(1).author`.
**Suffix pipeline** — after the path head (Get/Query/label), dot/bracket segments classify as [`RowSuffix`] (relation, limit, project, filter, with, sort, dedupe, distinct, group_by, …) and lower through one fold (`lower_suffix_stream`), including interleaved forms such as `repo.commits.limit(1).author` or `issues.with{age_days: (now - updated_at)}.filter{age_days>14}`.
The reason will be displayed to describe this comment to others. Learn more.
Suffix-pipeline update is accurate, but it doesn’t close the spec gap above in Continuation rules (~L541–551). Implementation (binding_contract.rs) marks .filter / .with / .sort / .dedupe / .group_by / .aggregate bindings as ContinuationCapability::Terminal (no label.<relation> on a separate line), while […] projection and .limit inherit the parent anchor.
Please add a small table there (compute op → continuation mode) and link to row-compute for agent pitfalls — otherwise the canonical spec still reads as if only projection / .limit(1) / .singleton() affect continuation.
Add computed columns to each row while preserving upstream **RowIdentity** (qualified entity + ref). Relation hops and further postfix compose in the **same RHS** (`issues.with{…}.filter{…}`, `issues.with{…}.r#`) or via a follow-on binding whose RHS **starts with the prior label** plus postfix (`sorted = filtered.sort(field)`). A binding whose RHS ends on `.with{…}` alone is **terminal** for bare relation-dot (`stale.labels`) — chain on one line or bind from an earlier surface anchor.
The reason will be displayed to describe this comment to others. Learn more.
The relation-dot terminal rule applies to all terminal compute bindings (.filter, .sort, .dedupe, .group_by, .aggregate, not just .with). Consider a dedicated Relation-dot terminal bindings subsection with one table instead of repeating per operator.
Clarify the positive case too: filtered = issues.filter{…} then stale = filtered.with{…} works because the RHS is a full expression referencing filtered, not bare filtered.labels.
`group_by` and `aggregate` change the row schema (terminal for relation-dot continuation on that label). After `group_by`, output columns are the **group keys** plus aggregate names (`n`, `total`, …). A chained `.sort(n, desc)` sorts on those aggregate columns — not on fields of the original catalog entity.
The reason will be displayed to describe this comment to others. Learn more.
Same continuation note: .filter is equally terminal for bare label.<relation> — matrix tests already use items = LangItem.filter{…} followed by items.with{…} on the next line. Cross-link the canonical continuation table once you add it to the language definition.
Legacy opaque `p#` tokens for fields/params are **rejected** at parse.
Expressions compose with pipes and postfix transforms. Multi-line payloads use tagged **heredocs** — see the [Language definition](reference/plasm-language-definition.md).
Expressions compose with pipes and postfix transforms (`.filter`, `.with`, `.sort`, `.group_by`, …). Multi-line payloads use tagged **heredocs** — see the [Language definition](reference/plasm-language-definition.md).
The reason will be displayed to describe this comment to others. Learn more.
Now that row compute is a first-class reference page, link it here alongside the language definition — e.g. “postfix transforms (see Row compute)”. Avoids agents reading only the monolithic language-def for .with / dedupe semantics.
- Document these vars in any **core** onboarding path (README / desktop installer), separate from Helm/object-store guides.
- Operator UIs that list traces expect agent `/v1/traces*`; durable list/detail requires local archive or sink — see [`http_traces.rs`](https://github.com/PlasmTools/plasm-core/blob/main/crates/plasm-agent-core/src/http_traces.rs).
- **OTLP export** (optional): when `OTEL_*` collectors are configured, application spans use stable semantic names (`plasm_agent.*`, `plasm_core.*`, `plasm_runtime.*`) — not Rust module paths. Contract and dashboard guidance: [`plasm-otel` README — Semantic span names](https://github.com/PlasmTools/plasm-core/blob/main/crates/plasm-otel/README.md#semantic-span-names-stable-contract).
The reason will be displayed to describe this comment to others. Learn more.
Pointer to plasm-otel is good, but this PR’s span work adds a much larger stable set (plasm_agent.oauth_link.*, plasm_agent.plan.dry_run, plasm_agent.plan.step_materialize, …) with span_graph_tests parent/child contracts. Consider either (a) a short inventory subsection here, or (b) expanding the README examples table so doc-site readers don’t have to spelunk spans.rs for dashboard wiring.
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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Summary
RowPlanexecuted by Polars inplasm-runtime(engine stays hidden from teaching/grammar)..with{k: expr}with a training-set glossary: fields, literals,+ - * /,now, grouping parens,len,when; string+concatenates; temporal − temporal yields integer days.FillSummaryentities).Test plan
cargo test -p plasm-runtime --lib -- row_computecargo test -p plasm-core --lib -- row_plan:: with_parse with_body peel_withcargo test -p plasm-e2e --test plasm_language_matrix.with{boost: score * 2}and(now - updated_at)programsMade with Cursor