Skip to content

docs: refresh README feature overview - #906

Open
andreatnvidia wants to merge 1 commit into
mainfrom
andreatnvidia/docs/readme-outdated-content
Open

docs: refresh README feature overview#906
andreatnvidia wants to merge 1 commit into
mainfrom
andreatnvidia/docs/readme-outdated-content

Conversation

@andreatnvidia

Copy link
Copy Markdown
Contributor

📋 Summary

Refresh the README overview to reflect current stable Data Designer capabilities and remove transition-era guidance. This keeps the repository front page aligned with shipped multimodal, tool-use, extensibility, resumability, observability, and agent-skill support.

🔗 Related Issue

N/A

🔄 Changes

  • Expand the README feature overview with multimodal generation, MCP tools and traces, plugins, resume, and monitoring.
  • Remove the async-engine heads-up now that async execution is the established engine path.
  • Replace migration-era documentation wording with evergreen copy.
  • Update agent-skill guidance to reflect Claude Code and Codex testing and agent-neutral invocation.

🧪 Testing

  • .venv/bin/ruff check --fix .
  • .venv/bin/ruff format .
  • git diff --check
  • Changed external links verified
  • Unit tests N/A - README-only change
  • E2E tests N/A - README-only change

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • Architecture docs updated (N/A - README-only change)

Signed-off-by: Andre Manoel <amanoel@nvidia.com>
@andreatnvidia
andreatnvidia requested a review from a team as a code owner August 31, 2026 23:22
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Refreshes the repository overview to describe current multimodal generation, MCP tooling, plugins, resumability, monitoring, and agent-skill support.

  • Consolidates validation and scoring capabilities in the feature overview.
  • Removes transition-era async-engine and documentation wording.
  • Makes agent-skill installation and invocation guidance agent-neutral.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking documentation inconsistency in the agent-skill section.

The README’s capability refresh is supported by current implementation, but its new Claude Code and Codex statement conflicts with the linked guide’s remaining Claude Code-only language.

Files Needing Attention: README.md

Important Files Changed

Filename Overview
README.md Updates the public feature overview and agent guidance, with a non-blocking inconsistency between the new dual-agent claim and the linked Claude Code-focused guide.
Prompt To Fix All With AI
### Issue 1
README.md:123
**Agent support guidance conflicts**

The README now presents Claude Code and Codex as tested agents, but the linked guide still says testing focused on Claude Code and provides Claude Code-specific installation guidance. Readers following that link receive contradictory support and setup instructions.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "docs: refresh README feature overview" | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for tidying up the front page, @andreatnvidia — the new feature list is a much better snapshot of where Data Designer actually is today.

Summary

This PR rewrites the README's "What can you do" bullets to cover multimodal generation, MCP tool use with traces, plugins, resume, and monitoring; drops the async-engine transition notice; and refreshes the docs/agent-skill wording. The implementation matches the stated intent — it's a README-only change, and I verified each new capability claim against the code rather than taking it on faith. Every one holds up (details in What Looks Good).

Findings

Warnings — Worth addressing

README.md:123-131 — Agent-skill section now contradicts the page it links to

  • What: The new copy says the skill "is tested with [Claude Code] and [Codex]" and reduces the install step to a bare "Install via skills.sh". The dev note linked on that very line (fern/versions/latest/pages/devnotes/posts/data-designer-got-skills.mdx:1432,1441,1443) still asserts the opposite and retains the step this PR removed:
    • line 1432: "our development and testing has focused on Claude Code at this stage"
    • line 1441: <Tip>When prompted, make sure to select **Claude Code** as an additional agent.</Tip>
    • line 1443: "open Claude Code and type /data-designer"
  • Why: Two things worth separating here. First, a reader who clicks the link gets a direct contradiction on which agents are supported — and the README is the correct one, since skills/data-designer/skill-card.md:36 and BENCHMARK.md:21 both report codex evaluation results. So the dev note is simply stale, and this PR makes that visible without fixing it. Second, and more concretely: if npx skills add still prompts for which agents to install for, a user following only the README won't know to select theirs and may end up with nothing installed. That's a silent onboarding failure rather than a wording nit.
  • Suggestion: Could we carry the same two-line refresh into the dev note in this PR? It's a natural extension of "remove migration-era wording", and it keeps the two pages telling one story:
    Next, install the skill. It is tested with Claude Code and Codex.
    
    plus dropping or generalizing the Tip. If you'd rather keep this PR strictly README-scoped, a tracked follow-up issue works too — I'd just like the drift not to go unrecorded. And on the install prompt specifically: if it does still appear, it's worth keeping one agent-neutral sentence in the README ("select your agent when prompted") so the step isn't lost.

Suggestions — Take it or leave it

README.md:131 — Losing the concrete invocation may cost more than the agent-neutrality gains

  • What: type /data-designer became invoke the data-designer skill.
  • Why: I get the motivation — /data-designer is Claude-Code-specific syntax. But "invoke the skill" isn't actionable for someone who's just run npx skills add and is staring at a prompt, and it's the one line in this section that tells them what to actually do next.
  • Suggestion: Keep the neutral phrasing and add one concrete example, e.g. "invoke the data-designer skill (/data-designer in Claude Code) or describe the dataset you want…". Best of both.

README.md:100-105 — No README pointer to Architecture & Performance anymore

  • What: Removing the async-engine heads-up was the right call — I confirmed the operational guidance survives at fern/versions/latest/pages/concepts/architecture-and-performance.mdx:363-380, including the inference_parameters.timeout advice for slow self-hosted endpoints. But that heads-up was the README's only link to that page, and the "Learn more" list doesn't include it.
  • Why: The timeout knob is the single most common fix for self-hosted-endpoint complaints. It's still documented, just no longer reachable in one hop from the front page.
  • Suggestion: Add a bullet to "Learn more":
    - **[Architecture & Performance](https://docs.nvidia.com/nemo/datadesigner/concepts/architecture-performance)** – Async engine behavior, concurrency, and timeout tuning

README.md:23 — Preview gets less prominence than it probably deserves

  • What: "Iterate quickly with preview mode before full-scale generation" was folded into "Preview, resume, and monitor generation from small experiments to large runs".
  • Why: Preview is the feature the quickstart actually demonstrates — the snippet at line 90 ends on data_designer.preview(...). Compressing it into a three-verb bullet makes the front page slightly less coherent with its own example. monitor is also the vaguest of the three verbs relative to what a reader can go find (there's ProgressTracker and the JSONL scheduler event sink, but no single "monitoring" concept page).
  • Suggestion: Optional split, if you want preview to keep its own line:
    - **Iterate quickly** with preview mode before full-scale generation
    - **Scale confidently** with resumable runs and progress monitoring

What Looks Good

  • The claims are real. I checked each new bullet against the source, and they're accurate — not aspirational: ImageColumnConfig for image generation; ImageContext/AudioContext/VideoContext with adapter support in openai_compatible.py; both local stdio and remote SSE/streamable-HTTP MCP providers in config/mcp.py; TraceType for interaction capture; ResumeMode wired through create/acreate; and Python/SQL/local-callable/remote validators plus LLM judges. That's a good hit rate for a marketing-adjacent section.
  • The plugin bullet is exactly right. "custom columns, seed readers, and processors" maps one-to-one onto PluginType's COLUMN_GENERATOR, SEED_READER, PROCESSOR. Easy thing to get vague about; you didn't.
  • Retiring the async-engine notice was well-judged. A "heads-up" banner has a shelf life, and you confirmed the substance still lives in the architecture docs before deleting it — which is the right order of operations.
  • Clean mechanics. Single-purpose commit, DCO signed off, git diff --check clean, no stray whitespace, and the changed external links resolve.

Residual Risk

  • https://chatgpt.com/codex returned 403 to my automated fetch, including with a browser user-agent. That's almost certainly Cloudflare-style bot protection on chatgpt.com rather than a broken link, so I'm not filing it as a finding — but I couldn't positively confirm it, and you may prefer a docs-canonical Codex URL that's friendlier to link checkers if CI ever gains one.
  • I could not verify whether npx skills add still prompts for agent selection; the Warning above is conditional on that behavior.
  • No /tmp/structural-impact-906.md was produced for this PR, which is expected for a README-only change — no structural analysis section is included below.

Verdict

Needs changes — one item to resolve before merge:

  • Reconcile the agent-skill section with the linked dev note (data-designer-got-skills.mdx:1432,1441,1443), and confirm whether the removed "select your agent when prompted" install step is still needed. Fixing it here is a two-line edit; an explicit follow-up issue is an acceptable alternative if you'd rather keep this PR README-only.

Everything else is optional polish. The substance of the refresh is solid and verified.


This review was generated by an AI assistant.

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