Skip to content

fix: clarify init first-success path - #604

Merged
mk3008 merged 2 commits into
mainfrom
codex/npm-consumer-guards
Mar 17, 2026
Merged

mk3008 merged 2 commits into
mainfrom
codex/npm-consumer-guards

Conversation

@mk3008

@mk3008 mk3008 commented Mar 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • redesign \ztd init\ next steps into a clear first-success path from DDL to first test
  • promote \model-gen\ into the main numbered flow for default, webapi, and local-source scaffolds
  • add a separate fallback block and sync scaffold README/test snapshots with the new guidance

Testing

  • pnpm --filter @rawsql-ts/ztd-cli test -- init.command.test.ts
  • pre-commit: ztd-cli typecheck, full ztd-cli test suite, build, lint

Summary by CodeRabbit

  • New Features

    • Enhanced initialization output with structured next steps plus fallback instructions and app-shape–aware guidance.
  • Documentation

    • README templates refocused to an SQL-asset-first workflow with explicit scaffold, regenerate, and troubleshooting guidance (including “If this fails” flows).
  • Tests

    • Updated test expectations and verification checks to match the revised guidance and README content.

@coderabbitai

coderabbitai Bot commented Mar 17, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f3836be4-b112-4fb1-b95a-5bc2a15f482a

📥 Commits

Reviewing files that changed from the base of the PR and between cb7f2fa and 0fe61c1.

📒 Files selected for processing (1)
  • scripts/verify-published-package-mode.mjs

📝 Walkthrough

Walkthrough

Refactors init command to return structured next steps ({ nextSteps, fallbackSteps }) from buildNextSteps; updates README templates to prioritize SQL-asset-first workflows and adds failure-handling guidance; updates tests and verification script expectations to reference the new model-gen guidance and adjusted command phrasing.

Changes

Cohort / File(s) Summary
Init Command Refactoring
packages/ztd-cli/src/commands/init.ts
Changes buildNextSteps(...) to return { nextSteps: string[]; fallbackSteps: string[] }, adds appShape-aware SQL and wiring step generation, local-source mapping for fallback steps, and updates buildSummaryLines to consume the new structure.
README Templates
packages/ztd-cli/templates/README.md, packages/ztd-cli/templates/README.webapi.md
Reworks "Next steps" to a SQL-asset-first workflow (place SQL under src/sql/, regenerate DDL, scaffold QuerySpec via ztd model-gen, review repository seam, run smoke tests) and adds a detailed "If this fails:" troubleshooting block.
Tests & Verification
packages/ztd-cli/tests/init.command.test.ts, scripts/verify-published-package-mode.mjs
Updates expectations to include npx ztd model-gen guidance, adjusted test-runner phrasing (explicit npm run test / pnpm test or npx vitest run) and presence of the "If this fails:" troubleshooting lines.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through code with a gentle tap,
Next steps split neatly into map and fallback lap.
SQL files first, then scaffold and test—
If troubles arrive, consult the little nest.
Happy scaffolding! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: restructuring the init command's next-steps guidance into a clearer first-success path, which is the primary focus across all modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/npm-consumer-guards
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/ztd-cli/src/commands/init.ts (2)

1993-1996: Redundant conditional for wiringStep.

Both branches produce the same string. This can be simplified.

♻️ Proposed fix
-  const wiringStep =
-    appShape === 'webapi'
-      ? `Review ${sqlClientPath} and ${repositoryTelemetryPath} so the first SQL-backed repository has a seam to plug into`
-      : `Review ${sqlClientPath} and ${repositoryTelemetryPath} so the first SQL-backed repository has a seam to plug into`;
+  const wiringStep = `Review ${sqlClientPath} and ${repositoryTelemetryPath} so the first SQL-backed repository has a seam to plug into`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ztd-cli/src/commands/init.ts` around lines 1993 - 1996, The
conditional assigning wiringStep is redundant because both branches produce the
same string; simplify by assigning wiringStep directly to the shared template
using sqlClientPath and repositoryTelemetryPath (remove the ternary on
appShape). Update any references to wiringStep accordingly and delete the unused
appShape-based branch to keep the code concise.

1978-1979: Redundant ternary for sqlAssetPath.

Both branches of the ternary produce the same value 'src/sql/'. This can be simplified to a direct assignment.

♻️ Proposed fix
-  const sqlAssetPath = appShape === 'webapi' ? 'src/sql/' : 'src/sql/';
+  const sqlAssetPath = 'src/sql/';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ztd-cli/src/commands/init.ts` around lines 1978 - 1979, The ternary
for sqlAssetPath is redundant since both branches return 'src/sql/'; replace the
conditional assignment of sqlAssetPath with a direct assignment const
sqlAssetPath = 'src/sql/'; while leaving the existing sqlClientPath ternary
(const sqlClientPath = appShape === 'webapi' ?
'src/infrastructure/db/sql-client.ts' : 'src/db/sql-client.ts') unchanged so
behavior for sqlClientPath remains the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/ztd-cli/src/commands/init.ts`:
- Around line 1993-1996: The conditional assigning wiringStep is redundant
because both branches produce the same string; simplify by assigning wiringStep
directly to the shared template using sqlClientPath and repositoryTelemetryPath
(remove the ternary on appShape). Update any references to wiringStep
accordingly and delete the unused appShape-based branch to keep the code
concise.
- Around line 1978-1979: The ternary for sqlAssetPath is redundant since both
branches return 'src/sql/'; replace the conditional assignment of sqlAssetPath
with a direct assignment const sqlAssetPath = 'src/sql/'; while leaving the
existing sqlClientPath ternary (const sqlClientPath = appShape === 'webapi' ?
'src/infrastructure/db/sql-client.ts' : 'src/db/sql-client.ts') unchanged so
behavior for sqlClientPath remains the same.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a3b9fc6-df45-4dba-9699-a30faed1d033

📥 Commits

Reviewing files that changed from the base of the PR and between 3c679ec and cb7f2fa.

⛔ Files ignored due to path filters (1)
  • packages/ztd-cli/tests/__snapshots__/init.command.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (4)
  • packages/ztd-cli/src/commands/init.ts
  • packages/ztd-cli/templates/README.md
  • packages/ztd-cli/templates/README.webapi.md
  • packages/ztd-cli/tests/init.command.test.ts

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