feat(tck): node-value rendering + first whole-node un-skip + matrix v2 (#889 slice 2)#899
Conversation
Parked slice-2 render work for #889: render_cell Struct/List arms → `(:Label {k: v})` and the `And having executed:` step. Verified correct (Return4 [2] `(:Singleton)` passes when un-skipped), but corpus un-skipping is gated on engine features (WITH/#577, multi-pattern MATCH, builtins) — so this lands with the un-skip PR once those exist. Not for merge yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#889 slice 2 / #598) Brings the parked render arms back now that the engine runs the needed shapes (WITH forwarding, multi-pattern, RETURN *, node values), and lands the first whole-node TCK un-skip with per-scenario granularity. - Render (tests/bdd/tck_steps.rs, from the cherry-picked commit): render_cell gains node Struct → `(:Label {k: v})` (sorted keys, node_uuid omitted) and List arms; new `And having executed:` step for setup CREATEs. - Un-skip (per-scenario): the corpus interleaves supported/unsupported scenarios, so a feature is un-skipped by removing its feature-level @skip-rust and tagging only the still-unsupported scenarios. clauses/with/With1: [1][2][5] (node + null forwarding) now run; the relationship/path/optional-match scenarios stay @skip-rust. Corpus passing 6 → 9. - Matrix v2 (coverage_matrix.json + tck_coverage.rs): each feature now records `scenarios_passing` (the runnable subset) and a rust_status of passing/partial/skip; `_meta.scenarios_passing` is the corpus total. tck_coverage recomputes per-scenario (feature OR scenario @skip-rust) and asserts the matrix matches — the regression floor is now at scenario granularity. Docs updated. Validated: BDD corpus 9/9 (Literals1 6 + With1 3); tck_coverage green (With1 partial 3/6, total passing 9); e2e + clippy gate clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughAdds a ChangesTCK Step Enhancements and Per-Scenario Coverage Tracking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/gf-api/tests/tck_coverage.rs`:
- Around line 61-116: The inner scanning loop for Examples blocks advances past
all non-scenario lines including tag blocks like `@skip-rust` that belong to the
next scenario. When the outer loop resumes at the next `Scenario:` line,
`pending_skip` has already been reset to false, causing scenario-scoped tags to
be ignored. To fix this, the inner loop (around lines 70-98 where it walks
through Examples and the trailing `j += 1`) needs to detect and stop before
consuming lines that start with `@` (tag lines). Add a condition to check if a
line starts with `@` and break the inner loop when such a line is encountered,
allowing the outer loop to properly re-process those tags for the following
scenario.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 791bcc3e-d8e1-466f-b87d-086a7b3ae675
⛔ Files ignored due to path filters (1)
docs/reference/tck-compliance.mdis excluded by!**/*.md,!**/docs/**
📒 Files selected for processing (4)
crates/gf-api/tests/bdd/tck_steps.rscrates/gf-api/tests/tck_coverage.rstests/tck/coverage_matrix.jsontests/tck/features/clauses/with/With1.feature
… CodeRabbit) feature_breakdown's Scenario-Outline scan walked past every non-scenario line including a following scenario's `@…` tag block (via the trailing `j += 1`), so a scenario-scoped `@skip-rust` placed right after an outline's Examples was never seen — that scenario would be wrongly counted as passing (a latent false-positive that could mask a regression; not triggered by the current corpus). Fix: on a tag line, look past the contiguous `@` block — if it introduces the next Scenario/Feature, stop so the outer loop sees the tags; if it tags an Examples block, keep scanning (those rows still count). Two regression unit tests cover both directions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/gf-api/tests/tck_coverage.rs (1)
84-117: 🎯 Functional Correctness | 🔵 TrivialOutline fix is correct; consider future-proofing for Examples-level
@skip-rusttags.The fix for the previously-reported outline tag-consumption bug is correct: when the contiguous
@…block precedes the nextScenario/Feature, the inner loop breaks andi = jso the outer loop re-reads those tags (lines 84-96). The alternative path (j = k; continue) correctly retains Examples-block tags inside the outline.However, that path also silently skips checking
@skip-ruston anExamples:block itself. When a tag block precedesExamples:(not a scenario/feature), it is consumed at lines 97-98 and subsequent rows are unconditionally tallied at lines 100-117, even if@skip-rustwas present. The current corpus carries no@skip-rusttags on Examples blocks, so counts remain correct today; however, for robustness, consider tracking a flag (e.g.,pending_skip) within the outline scan to exclude such rows frompassingif the corpus evolves.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gf-api/tests/tck_coverage.rs` around lines 84 - 117, The tag block preceding an Examples or Scenarios block is being consumed without checking for `@skip-rust` tags, and the subsequent row counting in the inner while loop (lines 100-117) happens unconditionally. To fix this, introduce a flag variable (e.g., pending_skip) to track whether `@skip-rust` was present in the tag block that precedes the Examples block. Check this flag when processing tags before the Examples block, and then only increment the rows counter if the skip flag is not set. This ensures that rows from Examples blocks with `@skip-rust` tags are not unconditionally counted toward the passing rows tally.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/gf-api/tests/tck_coverage.rs`:
- Around line 84-117: The tag block preceding an Examples or Scenarios block is
being consumed without checking for `@skip-rust` tags, and the subsequent row
counting in the inner while loop (lines 100-117) happens unconditionally. To fix
this, introduce a flag variable (e.g., pending_skip) to track whether `@skip-rust`
was present in the tag block that precedes the Examples block. Check this flag
when processing tags before the Examples block, and then only increment the rows
counter if the skip flag is not set. This ensures that rows from Examples blocks
with `@skip-rust` tags are not unconditionally counted toward the passing rows
tally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25f4668b-4551-435c-a244-4e626fe82546
📒 Files selected for processing (1)
crates/gf-api/tests/tck_coverage.rs
Part of #889 (slice 2) / advances #598. The payoff of the engine work this cycle: the render arms finally have a runnable corpus consumer.
What lands
tests/bdd/tck_steps.rs):render_cellgains a nodeStructarm →(:Label {k: v})(sorted keys,node_uuidomitted as non-deterministic) and aListarm; plus theAnd having executed:setup step.@skip-rust, tag only the still-unsupported scenarios.clauses/with/With1:[1][2][5](node + null forwarding through WITH) now run and pass;[3]/[4]/[6](relationship/path/optional-match) stay@skip-rust. Corpus passing 6 → 9.coverage_matrix.json+tck_coverage.rs): each feature now recordsscenarios_passing(runnable subset) and arust_statusofpassing/partial/skip;_meta.scenarios_passingis the corpus total.tck_coveragerecomputes per-scenario (feature or scenario@skip-rust) and asserts the matrix matches — the regression floor is now at scenario granularity.Why per-scenario (vs whole-feature)
Empirically, zero corpus features are entirely runnable today (verified by running, not the over-optimistic static classifier) — each interleaves scenarios needing features still to come. Per-scenario un-skipping is the mechanism all of #598–#601 will grow through.
Validation
With1 [1][2]exercise the render arms ((:A),(:B)).tck_coveragegreen (With1partial3/6;_meta.scenarios_passing= 9); e2e (88) +cargo clippy --workspace -- -D warningsclean.Engine prerequisites that made this possible (all merged this cycle): node values (#785/#893), WITH read-path + whole-node forwarding (#894/#896), multi-pattern + anon endpoints (#895),
RETURN *(#897), CREATE anonymous paths (#898).🤖 Generated with Claude Code
Note
Add node-value rendering and un-skip first whole-node scenarios in TCK with partial-status matrix support
render_cellin tck_steps.rs to handle ArrowStruct(node) andListarray types, formatting nodes as(Label1:Label2 {k: v, ...})via a newrender_node_structhelper.given_having_executedstep handler so setup queries can be run before assertions in BDD scenarios.@skip-rustfrom With1.feature, adding per-scenario@skip-rustonly to the 3 scenarios involving relationships, paths, or nullable nodes — making 3 additional scenarios runnable.(total, passing)per feature, support apartialmatrix status, and enforce a corpus-widescenarios_passingfloor.scenarios_passingper feature and bumps_meta.scenarios_passingfrom 6 to 9.Macroscope summarized 7b09645.
Summary by CodeRabbit
scenarios_passingvisibility for more granular outcomes@skip-rustbehavior