Skip to content

Docs: add LLM-assisted migration prompt callout#1394

Open
justin808 wants to merge 3 commits intomainfrom
jg-codex/llm-migration-homepage-copy
Open

Docs: add LLM-assisted migration prompt callout#1394
justin808 wants to merge 3 commits intomainfrom
jg-codex/llm-migration-homepage-copy

Conversation

@justin808
Copy link
Copy Markdown
Collaborator

@justin808 justin808 commented Mar 31, 2026

Summary

  • add a homepage/README callout pointing to an LLM-assisted migration path
  • add an LLM-assisted migration prompt section to the migration guide
  • provide a copy/paste prompt template oriented around safe, phased migration

Why

This makes the migration path to react_on_rails feel faster and more actionable, while still encouraging review and test validation.

Testing

  • docs-only change
  • rendered diff review

@claude
Copy link
Copy Markdown

claude bot commented Mar 31, 2026

PR Review: Docs — LLM-assisted migration prompt

This is a docs-only change, so no code risk. The idea is useful and the structure is clear. A few issues worth addressing before merging:


Bugs / Rendering Issues

README.md — missing blank line

The new sentence is appended directly after the previous line with no blank line separator:

You can find [migration to React on Rails steps here](...).
For the fastest path, try the [LLM-assisted migration prompt](...).

In CommonMark/GitHub Markdown, lines without a blank line between them render as a single paragraph — these two sentences will run together. Add a blank line (or an explicit <br>) between them if they're meant to be read as separate callouts.


Prompt Accuracy

"Run tests/lint after each phase" (point 5)

An LLM can't execute commands — it can only suggest them. As written, this instruction implies the assistant will run tests itself, which may confuse users about what to expect. Consider rephrasing to something like:

  1. After each phase, tell me what commands to run to verify the changes. Pause and wait for me to share the output before continuing to the next phase.

This also makes the migration more interactive and safer, since the LLM waits for real feedback rather than proceeding blindly.


Missing Context Instruction

The prompt jumps straight to "migrate this app" without instructing the LLM to first understand the user's codebase. Most LLMs won't ask for files unless prompted. Consider adding an opener like:

Before starting, ask me to share the relevant files: Gemfile, package.json, application.js (or equivalent entrypoint), any react_component view helper usages, and any existing webpacker/jsbundling config.

Without this, the LLM may produce a generic migration rather than one tailored to the app.


Minor: Hardcoded main branch URL in the prompt

The prompt points the LLM to:

https://github.com/reactjs/react-rails/blob/main/docs/migrating-from-react-rails-to-react_on_rails.md

This is fine for now, but if the guide ever moves or is updated in a breaking way, the prompt will silently point to a changed doc. Not a blocker, just worth noting.


What's Good

  • The phased structure (dependencies → installer → entrypoints → helpers → cleanup) mirrors best practice for this kind of migration and gives the LLM a clear scaffold.
  • Requiring a rollback plan and PR risk summary in the output is a nice touch.
  • "Review all generated diffs before merging." is an important disclaimer — though making it bold or a > **⚠ Warning:** blockquote would make it harder to overlook.

Overall this is a helpful addition. Addressing the rendering issue and the test-running phrasing would make it production-ready.

@claude
Copy link
Copy Markdown

claude bot commented Mar 31, 2026

PR Review: Docs — LLM-assisted migration prompt

Overall this is a clean, docs-only addition that helps users take action on the migration advice already in the repo. A few observations:


Strengths

  • Well-structured prompt with clear phases (deps → installer → entry points → helpers → cleanup) — this mirrors how a careful human engineer would approach it.
  • Good safety rails: requiring test/lint between phases, rollback steps, and a risk-level PR summary are all genuinely useful constraints to put on an LLM.
  • Known migration footguns (ReactRailsUJS references, stale server_rendering.js) are explicitly called out.
  • The "Review all generated diffs before merging" reminder is the right note to end on.

Concerns / Suggestions

1. The prompt URL assumes web browsing capability

Use this guide as the source of truth:
https://github.com/reactjs/react-rails/blob/main/docs/migrating-from-react-rails-to-react_on_rails.md

Many coding assistants (offline IDEs, restricted environments, older API integrations) can't fetch URLs at inference time. A user following this literally with a non-browsing assistant will get an LLM that silently invents migration steps.

Suggestion: add a brief note before the code block, e.g.:

Note: If your assistant does not support web browsing, paste the contents of the migration guide directly into your prompt before this template.


2. No instruction to provide codebase context

The prompt says "Migrate this app" but doesn't tell the user how to give the LLM visibility into their files. Without that context the model has to guess at the app's structure. A one-liner like:

Before pasting this prompt, share your relevant files (e.g. Gemfile, package.json, entry points, initializers) with your assistant.

would make the template significantly more actionable.


3. bundle exec rails shakapacker:compile is Shakapacker-specific

Not every react-rails app uses Shakapacker — some still use Webpacker or plain importmap/jsbundling-rails. If a user follows the prompt and their app doesn't have Shakapacker, this will fail and might confuse them mid-migration.

Consider qualifying it:

- if using Shakapacker: run `bundle exec rails shakapacker:compile` and fix compile errors before finalizing

Minor nits

  • The new README sentence sits on the same line (no blank line) as the preceding one. In rendered Markdown they'll flow as one paragraph, which reads fine — just flagging it's intentional.
  • The TOC entry addition is correct.

Summary: Docs are correct and the intent is solid. The main actionable items are the web-browsing caveat (#1) and the Shakapacker qualifier (#3). #2 would noticeably improve usability. Happy to see this merged with or without those tweaks — they're suggestions, not blockers.

@claude
Copy link
Copy Markdown

claude bot commented Mar 31, 2026

PR Review: Docs — LLM-assisted migration prompt

This is a docs-only change and is generally well-structured. Here are a few observations:

Positive

  • The prompt template is well-organized with clear phases (dependencies → installer → entrypoints → view helpers → cleanup), which mirrors a safe, incremental migration strategy.
  • Requiring the LLM to pause after each phase and wait for test/lint output before continuing is a good guard against blind bulk migrations.
  • The checklist, rollback steps, and risk-level summary at the end are practical additions.
  • The TOC entry anchor (#llm-assisted-migration-prompt) matches the heading exactly and will render correctly on GitHub.

Issues / Suggestions

1. Floating note before the code block (minor UX issue)

The sentence:

If your assistant cannot browse URLs, paste this migration guide content directly into the prompt context before running the template.

…appears as plain paragraph text between the intro and the code fence, making it easy to overlook. Consider formatting it as a callout so it stands out:

> **Note:** If your assistant cannot browse URLs, paste this migration guide content directly into the prompt context before running the template.

2. Hardcoded main-branch URL inside the prompt

The prompt includes:

https://github.com/reactjs/react-rails/blob/main/docs/migrating-from-react-rails-to-react_on_rails.md

This is intentional (so users copy a stable, resolvable URL), but it means the in-prompt URL and the actual guide can silently diverge. A short comment alongside the code block noting "update this URL to a tagged release when using a pinned version" would help maintainers remember to keep it in sync during future releases.

3. Final safety reminder could be more visible

Review all generated diffs before merging. is the most important piece of advice in this section, but it appears at the very end of the code block as a trailing line, where it blends in. Moving it outside the code fence as a bold or warning callout would make it harder to skip:

> **Warning:** Always review all generated diffs before merging. LLM output can introduce subtle regressions.

4. README link placement (very minor)

The new README line:

For the fastest path, try the [LLM-assisted migration prompt](...).

sits between the existing migration-guide sentence and the --- separator. The two consecutive migration-related lines read slightly awkwardly. Combining them into a single sentence or paragraph would improve flow:

You can find [migration to React on Rails steps here](...), including an [LLM-assisted migration prompt](...) for a faster first pass.

Summary

This is a useful, low-risk addition to the docs. The structural issues above are all minor polish items — none are blockers. The prompt template itself is solid and follows good LLM-prompting practices (phased work, explicit pause points, rollback instructions).

@justin808
Copy link
Copy Markdown
Collaborator Author

Addressed the review suggestions in commit 6b80d55:\n\n- added spacing in README so the migration + LLM callout render as intended\n- added a non-browsing fallback note before the prompt\n- added an explicit instruction to gather app context files before migration\n- changed phase verification to ask for commands + wait for user output\n- qualified the compile step to Shakapacker-only contexts\n\nCI is running on the updated branch.

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.

1 participant