Skip to content

feat: add preview review reference and update interactive iterate step#441

Merged
johnnygreco merged 14 commits intomainfrom
feat/skill-preview-review-reference
Mar 23, 2026
Merged

feat: add preview review reference and update interactive iterate step#441
johnnygreco merged 14 commits intomainfrom
feat/skill-preview-review-reference

Conversation

@johnnygreco
Copy link
Copy Markdown
Contributor

Summary

  • Add references/preview-review.md with structured guidance for reviewing dataset previews (diversity, data quality, design choices)
  • Update interactive workflow iterate step to offer self-review and reference the new guide
  • Remove stale "and serve again" wording from iterate step

@johnnygreco johnnygreco requested a review from a team as a code owner March 20, 2026 00:23
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 20, 2026

Greptile Summary

This PR adds a new references/preview-review.md guide that provides structured criteria for self-reviewing dataset previews (diversity, data quality, design choices), and wires it into the interactive workflow's iterate step. It also generalizes the record-count warning in both workflow files by removing the hard-coded 50-record threshold.

Key points:

  • The review guide is well-structured and fills a real gap in the self-review loop.
  • Several issues flagged in prior review threads remain unresolved: the references/preview-review.md path in interactive.md may not resolve correctly relative to the file's directory; dataset.parquet is referenced in the guide but its existence as a --save-results artefact is undocumented; the self-review capability is not surfaced in the autopilot iterate loop; and autopilot.md step 7 still has a contradictory instruction order (run command first, then warn-before-running second).
  • A minor new ambiguity: the fallback artifacts/preview_results_*/ glob in preview-review.md can match multiple directories and does not explain how an agent should determine which is "most recent".

Confidence Score: 3/5

  • Safe to merge for documentation value, but four previously-flagged issues (path resolution, parquet existence, autopilot parity, instruction ordering) remain unaddressed and could cause agent failures in the field.
  • Prior threads identified four concrete issues — none were resolved in this revision. The new guide provides real value, but two of the prior concerns (missing parquet artefact, broken relative path) can cause outright agent errors rather than just degraded behavior, which prevents a higher confidence score.
  • skills/data-designer/references/preview-review.md (dataset.parquet availability, fallback directory resolution) and skills/data-designer/workflows/autopilot.md (contradictory create-step ordering).

Important Files Changed

Filename Overview
skills/data-designer/references/preview-review.md New guide providing structured review criteria (diversity, data quality, design choices). Instructs agents to load dataset.parquet, but the file's existence from --save-results is undocumented; fallback glob artifacts/preview_results_*/ also lacks guidance on how to resolve "most recent".
skills/data-designer/workflows/interactive.md Iterate step expanded to offer self-review via references/preview-review.md; stale "serve again" wording removed; finalize caution generalized from hard-coded 50-record threshold. Path references/preview-review.md may not resolve correctly relative to this file's location.
skills/data-designer/workflows/autopilot.md Create step reworded to remove the 50-record hard threshold in favor of context-sensitive guidance; however, the instruction still directs the agent to run the command first and warn/confirm "before running" second — a contradictory ordering that was flagged in a prior review thread and remains unresolved.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant A as Agent
    participant DD as data-designer CLI
    participant PR as preview-review.md

    Note over A,DD: Interactive Workflow — Iterate Step (updated)
    DD-->>A: Results path: artifacts/preview_results_*/
    A->>U: Here is your preview link
    A->>U: Would you like me to review the records?
    alt User accepts self-review
        A->>PR: Read references/preview-review.md
        PR-->>A: Diversity / Quality / Design criteria
        A->>DD: Load dataset.parquet (pandas)
        DD-->>A: Sample records
        A->>A: Evaluate diversity, quality, design
        A->>U: Suggested improvements
    else User provides own feedback
        U->>A: Feedback
    end
    A->>DD: data-designer validate + preview
    DD-->>A: Updated preview
    A->>U: Repeat until satisfied
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: skills/data-designer/references/preview-review.md
Line: 9

Comment:
**"Most recent" directory is ambiguous for an agent**

The fallback path `artifacts/preview_results_*/` is a glob that can match multiple directories. The guide says to use "the most recent" one, but does not explain how to determine recency — by modification time, by directory name suffix, or by listing and sorting. An agent may arbitrarily pick any match.

Consider making this explicit, e.g.:

```suggestion
Load `dataset.parquet` from the preview results directory (printed as `Results path:` by the preview command, or the most recent `artifacts/preview_results_*/` directory — sort by name or modification time to find it).
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (9): Last reviewed commit: "Merge branch 'main' into feat/skill-prev..." | Re-trigger Greptile

Comment thread skills/data-designer/workflows/interactive.md
Comment thread skills/data-designer/workflows/autopilot.md
Comment thread skills/data-designer/workflows/interactive.md
Comment thread skills/data-designer/references/preview-review.md
@johnnygreco johnnygreco merged commit 6de3032 into main Mar 23, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants