feat(quickstart): split quickstart into router + task-specific topic subcommands (lit-c5519f81-93c728d5.2.1)#201
Conversation
e90e4ce to
ed9ad7e
Compare
There was a problem hiding this comment.
Z.ai Coding Agent Review
One must-change: [LAW:one-source-of-truth] — the set of quickstart topic templates is enumerated in three separate places (quickstartTopics slice, QuickstartGuidanceNames() function, and the quickstartUsage const). Adding a new topic requires editing all three; the refresh path and usage string should derive from quickstartTopics, the single authoritative mapping.
Pattern-level note: the PR is otherwise well-structured. The topic dispatch is data-driven (value lookup, single render path), the refresh logic was cleanly generalized from one template to many, and the tests verify behavior (output content, error messages, override precedence) rather than implementation details. The quickstartTopics data table is a good [LAW:one-type-per-behavior] application — the consolidation request above is about completing that design by making it the sole source of truth.
❌ Request Changes
There was a problem hiding this comment.
Z.ai Coding Agent Review
Clean PR adding lit quickstart <topic> subcommands (ready, new, update, done, doctor) with a data-driven dispatch table. No must-change issues found.
Strengths: The quickstartTopics table in quickstart_topics.go is a well-executed single source of truth — the usage string, topic validation, refresh set, and router display all derive from it, satisfying [LAW:one-source-of-truth] and [LAW:one-type-per-behavior]. Every topic shares one render path (value lookup, not per-topic branching), good [LAW:dataflow-not-control-flow]. Error messages for unknown topics and aliases are dynamically constructed from the table/map, never hand-maintained strings. Test coverage is thorough: router contract, topic content, unknown-topic errors, flag rejection, soil-section exclusion, and project-override precedence.
Minor observations (not blocking):
- Line 1409 (
if *refresh || ejectChanged || *force): the*forcecheck is redundant — if*forceis true withoutejectChanged, the pre-existing guard at line 1404 already returned. Harmless but unnecessary. - Line 76 of the test (
strings.Contains(output, "lit quickstart ready—")) couples the topic tests to a specific string in the router template. If the router wording changes, this sentinel breaks even though topic functionality is unaffected — a minor[LAW:behavior-not-structure]` coupling.
✅ Approved
…subcommands (lit-c5519f81-93c728d5.2.1) Bare 'lit quickstart' becomes a short router: the critical agent-instructions preamble, the topic list with imperative trigger descriptions, and the 'lit ready' -> 'lit start <id>' fastpath. Per-workflow guidance moves into callable topics — lit quickstart ready|new|update|done|doctor — each carrying the material the former monolithic output covered in that section. The router keeps the quickstart.md filename so existing project/global overrides, --eject, and --refresh drift detection keep working unchanged. Topic templates are five new instances of the same managed-template type, registered in the templates package, so override precedence and eject extend to them with no new code paths. Topic dispatch is a value table (token -> template) sharing one render path. SOIL is routed, not dropped: soil_mode appends to the bare router output only (session-start read), never to topic output. --refresh now reports override drift for all six guidance templates instead of just quickstart.md. Breadcrumb pointers in command success output are sibling ticket lit-c5519f81-93c728d5.2.2.
…ng and refresh set derive from the topic table The token→template table in quickstart_topics.go is now the sole declaration of quickstart topic membership and display order. QuickstartGuidanceNames (a second ordered copy in the templates package) is removed; the --refresh template set and the usage string both derive from the table, so adding a topic is a one-line edit. [LAW:one-source-of-truth]
2799ef9 to
356ecd6
Compare
There was a problem hiding this comment.
Z.ai Coding Agent Review
No must-change violations found. The PR is well-designed: the quickstartTopics table is the single source of truth for topic membership, the usage string and refresh template set both derive from it, and every topic shares one render path — clean [LAW:one-source-of-truth] and [LAW:one-type-per-behavior] compliance.
Two pre-existing / non-blocking observations worth the author's attention:
-
Dual-table maintenance surface (
quickstartTopicsvsshortAliases). Adding a new topic requires editing bothquickstart_topics.go(the topic table) andtemplates.go(theshortAliasesmap). The keys differ by design (CLI subcommand tokens vs--ejectaliases), so this isn't a strict[LAW:one-source-of-truth]violation, but the template-name constants are the only shared reference. A future refactor could haveshortAliasesderive its quickstart entries from the topic table. -
TestQuickstartTopicRejectsFlagsgap. The test atquickstart_topics_test.go:94-105covers--refreshand--ejectbut omits--force. The production guard (cli.go:1409) checks all three flags. Worth adding a{"quickstart", "ready", "--force"}case for completeness.
✅ Approved
Finding addressed in 2799ef9 (single-source topic table); thread resolved and re-review on current head is clean.
Summary
lit quickstartbecomes a short router: the critical agent-instructions preamble, the topic list with imperative trigger descriptions ("use when..."), the "cheap to call, re-run any time" line, and thelit ready→lit start <id>fastpath.lit quickstart ready|new|update|done|doctor— each carrying the material the former monolithic output covered in that section, with breathing room.quickstart.mdfilename, so existing project/global overrides,--eject, and--refreshdrift detection keep working unchanged. The five topic templates register as instances of the same managed-template type, so override precedence and eject extend to them with zero new resolution code paths.soil_modeappends to the bare router output only (the session-start read — it's a session-wide convention, not task guidance), never to topic output.--refreshnow reports override drift for all six guidance templates instead of justquickstart.md.Breadcrumb pointers in command success output are sibling ticket
lit-c5519f81-93c728d5.2.2(next in the epic).Verification
go test ./...green.--eject(all 8 templates),--refreshdrift report flagging a customized topic override.Ticket: lit-c5519f81-93c728d5.2.1