Skip to content

Commit f7c943a

Browse files
committed
fix(app): resolve remaining coderabbit findings
1 parent bd26bfa commit f7c943a

13 files changed

Lines changed: 689 additions & 270 deletions

packages/app/src/docker-git/menu-create-advance.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,15 @@ export const completeCreateDisplaySettingsFlow = (
259259

260260
const resolveNextCreateFlowStep = (
261261
currentStep: CreateStep,
262-
currentStepIndex: number,
263-
nextSteps: ReadonlyArray<CreateStep>
262+
currentStepIndex: number
264263
): number =>
265264
currentStep === "repoUrl"
266265
? firstCreateSettingsStepIndex
267-
: clampCreateSettingsStep(currentStepIndex + 1, nextSteps.length - 1)
266+
: currentStepIndex + 1
267+
268+
const shouldCompleteCreateFlow = (
269+
nextSteps: ReadonlyArray<CreateStep>
270+
): boolean => nextSteps.length <= firstCreateSettingsStepIndex
268271

269272
/**
270273
* Advances create mode by applying the active prompt buffer.
@@ -278,10 +281,10 @@ const resolveNextCreateFlowStep = (
278281
// QUOTE(ТЗ): "after applying a non-repoUrl step it advances to currentStepIndex + 1"
279282
// REF: issue-339
280283
// SOURCE: n/a
281-
// FORMAT THEOREM: step != repoUrl -> nextStep = clamp(stepIndex + 1)
284+
// FORMAT THEOREM: remaining = empty -> Complete, otherwise Continue(next valid setting)
282285
// PURITY: CORE
283286
// EFFECT: n/a
284-
// INVARIANT: next step is always within the current settings range when continuing
287+
// INVARIANT: completion requires no unsatisfied settings after repoUrl
285288
// COMPLEXITY: O(k + s) where s = number of remaining create steps
286289
export const advanceCreateFlow = (
287290
contextOrCwd: string | CreateFlowContext,
@@ -303,10 +306,13 @@ export const advanceCreateFlow = (
303306
}
304307

305308
const nextSteps = resolveCreateFlowSteps(nextValues)
306-
const nextStep = resolveNextCreateFlowStep(step, view.step, nextSteps)
307-
return nextSteps.length > firstCreateSettingsStepIndex && nextStep < nextSteps.length
308-
? continueCreateFlow(nextStep, nextValues)
309-
: completeCreateFlow(context, nextValues)
309+
const nextStep = resolveNextCreateFlowStep(step, view.step)
310+
if (shouldCompleteCreateFlow(nextSteps)) {
311+
return completeCreateFlow(context, nextValues)
312+
}
313+
return nextStep < nextSteps.length
314+
? continueCreateFlow(clampCreateSettingsStep(nextStep, nextSteps.length - 1), nextValues)
315+
: continueCreateFlow(nextSteps.length - 1, nextValues)
310316
}
311317
)
312318
}

packages/app/src/docker-git/menu-create-command-parse.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,28 +68,27 @@ const consumeCreateTokenChar = (state: CreateTokenizeState, char: string): Creat
6868

6969
const consumeCreateTokenInput = (
7070
input: string,
71-
index: number,
7271
state: CreateTokenizeState
7372
): CreateTokenizeState => {
74-
if (index >= input.length) {
75-
return state
76-
}
77-
78-
const codePoint = input.codePointAt(index)
79-
if (codePoint === undefined) {
80-
return state
73+
let index = 0
74+
let next = state
75+
while (index < input.length) {
76+
const codePoint = input.codePointAt(index)
77+
if (codePoint === undefined) {
78+
return next
79+
}
80+
const char = String.fromCodePoint(codePoint)
81+
next = consumeCreateTokenChar(next, char)
82+
index += char.length
8183
}
82-
83-
const char = String.fromCodePoint(codePoint)
84-
return consumeCreateTokenInput(input, index + char.length, consumeCreateTokenChar(state, char))
84+
return next
8585
}
8686

8787
const tokenizeCreateCommandLine = (
8888
input: string
8989
): Either.Either<ReadonlyArray<string>, ParseError> => {
9090
const state = consumeCreateTokenInput(
9191
input.trim(),
92-
0,
9392
{ current: "", escaping: false, quote: null, tokens: [] }
9493
)
9594

packages/app/src/docker-git/menu-create-flow-types.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,15 @@ export const firstCreateSettingsStepIndex = 1
5050
/**
5151
* Narrows a create-flow view to interactive create mode.
5252
*
53+
* @param view - Create-flow view to refine.
54+
* @returns True when `view` is a create-mode view.
5355
* @pure true
54-
* @invariant true iff mode = create
56+
* @effect n/a
57+
* @invariant result <=> view.mode = "create"
58+
* @precondition `view` is a non-null CreateFlowView value.
59+
* @postcondition True narrows `view` to CreateModeFlowView; false leaves it as the remaining union member.
5560
* @complexity O(1)
61+
* @throws Never
5662
*/
5763
// CHANGE: expose a pure predicate for create-mode flow views
5864
// WHY: callers need type-safe mode refinement before create-only transitions
@@ -69,9 +75,15 @@ export const isCreateModeFlowView = (view: CreateFlowView): view is CreateModeFl
6975
/**
7076
* Narrows a create-flow view to browser display-settings mode.
7177
*
78+
* @param view - Create-flow view to refine.
79+
* @returns True when `view` is a display-mode view.
7280
* @pure true
73-
* @invariant true iff mode = display
81+
* @effect n/a
82+
* @invariant result <=> view.mode = "display"
83+
* @precondition `view` is a non-null CreateFlowView value.
84+
* @postcondition True narrows `view` to DisplayModeFlowView; false leaves it as the remaining union member.
7485
* @complexity O(1)
86+
* @throws Never
7587
*/
7688
// CHANGE: expose a pure predicate for display-mode flow views
7789
// WHY: callers need type-safe mode refinement before display-only transitions
@@ -88,9 +100,15 @@ export const isDisplayModeFlowView = (view: CreateFlowView): view is DisplayMode
88100
/**
89101
* Detects the repo-url prompt in create mode.
90102
*
103+
* @param view - Create-flow view to inspect.
104+
* @returns True when `view` is create mode at step zero.
91105
* @pure true
92-
* @invariant true implies create mode and step = 0
106+
* @effect n/a
107+
* @invariant result <=> view.mode = "create" and view.step = 0
108+
* @precondition `view` is a non-null CreateFlowView value.
109+
* @postcondition True implies callers may treat the active row as the repo-url prompt.
93110
* @complexity O(1)
111+
* @throws Never
94112
*/
95113
// CHANGE: expose the repo-step predicate for shared create-flow input handling
96114
// WHY: navigation and input logic treat repo URL entry differently from settings rows

packages/app/src/docker-git/menu-create-navigation.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,16 @@ import { resolveCreateDisplaySteps, resolveCreateFlowSteps } from "./menu-create
1212
/**
1313
* Clamps a create settings index into the editable settings range.
1414
*
15+
* @param step - Candidate step index.
16+
* @param lastStep - Inclusive upper bound of the current settings range.
17+
* @returns `step` bounded to `[firstCreateSettingsStepIndex, lastStep]`.
1518
* @pure true
19+
* @effect n/a
1620
* @invariant result is between first settings step and lastStep when range is valid
21+
* @precondition `lastStep >= firstCreateSettingsStepIndex` for a non-empty settings range.
22+
* @postcondition result >= firstCreateSettingsStepIndex and result <= lastStep when the precondition holds.
1723
* @complexity O(1)
24+
* @throws Never
1825
*/
1926
// CHANGE: centralize create settings index clamping
2027
// WHY: advancement and navigation must share the same range invariant
@@ -31,6 +38,31 @@ export const clampCreateSettingsStep = (
3138
lastStep: number
3239
): number => Math.min(Math.max(step, firstCreateSettingsStepIndex), lastStep)
3340

41+
/**
42+
* Computes a wrapped adjacent settings index.
43+
*
44+
* @param step - Current valid settings step index.
45+
* @param lastStep - Inclusive upper bound for settings.
46+
* @param direction - Vertical navigation direction.
47+
* @returns The previous or next settings index with wraparound.
48+
* @pure true
49+
* @effect n/a
50+
* @invariant result is inside `[firstCreateSettingsStepIndex, lastStep]`.
51+
* @precondition `step` is inside `[firstCreateSettingsStepIndex, lastStep]`.
52+
* @postcondition `up` decrements or wraps to `lastStep`; `down` increments or wraps to first settings step.
53+
* @complexity O(1)
54+
* @throws Never
55+
*/
56+
// CHANGE: isolate wrapped create-settings index arithmetic
57+
// WHY: both create and display modes share the same vertical navigation law
58+
// QUOTE(ТЗ): "Add concise but compliant TSDoc + functional comments"
59+
// REF: issue-339
60+
// SOURCE: n/a
61+
// FORMAT THEOREM: forall s in [first,last], d: next(s,d) in [first,last]
62+
// PURITY: CORE
63+
// EFFECT: n/a
64+
// INVARIANT: navigation never returns the repo URL step
65+
// COMPLEXITY: O(1)
3466
const nextCreateSettingsStep = (
3567
step: number,
3668
lastStep: number,
@@ -42,6 +74,31 @@ const nextCreateSettingsStep = (
4274
Match.exhaustive
4375
)
4476

77+
/**
78+
* Moves any settings-capable create-flow view within a bounded range.
79+
*
80+
* @param view - Create or display view to move.
81+
* @param lastStep - Inclusive upper bound for the active settings list.
82+
* @param direction - Vertical navigation direction.
83+
* @returns A moved view, the original view if unchanged, or null when no settings range is active.
84+
* @pure true
85+
* @effect n/a
86+
* @invariant Returned views preserve mode and committed values.
87+
* @precondition `view` is non-null and `lastStep` belongs to the step list used by the caller.
88+
* @postcondition Non-null moved views have empty `buffer` and null `inputError`.
89+
* @complexity O(1)
90+
* @throws Never
91+
*/
92+
// CHANGE: share immutable settings movement across create and display modes
93+
// WHY: the two views differ only in the step list that defines `lastStep`
94+
// QUOTE(ТЗ): "Add concise but compliant TSDoc + functional comments"
95+
// REF: issue-339
96+
// SOURCE: n/a
97+
// FORMAT THEOREM: valid(v,last) -> move(v).values = v.values
98+
// PURITY: CORE
99+
// EFFECT: n/a
100+
// INVARIANT: repo URL or empty settings ranges return null
101+
// COMPLEXITY: O(1)
45102
const moveCreateSettingsWithin = <
46103
A extends CreateModeFlowView | DisplayModeFlowView
47104
>(
@@ -65,13 +122,59 @@ const moveCreateSettingsWithin = <
65122
}
66123
}
67124

125+
/**
126+
* Resolves a horizontal boolean choice to the create-flow buffer token.
127+
*
128+
* @param direction - Horizontal choice direction.
129+
* @returns `"n"` for left and `"y"` for right.
130+
* @pure true
131+
* @effect n/a
132+
* @invariant result is always a valid yes/no buffer token.
133+
* @precondition `direction` is a valid CreateSettingsChoiceDirection.
134+
* @postcondition Left maps to false-preview token; right maps to true-preview token.
135+
* @complexity O(1)
136+
* @throws Never
137+
*/
138+
// CHANGE: encode boolean left/right choices as create input buffer values
139+
// WHY: preview buffers reuse the same parser path as typed settings input
140+
// QUOTE(ТЗ): "Add concise but compliant TSDoc + functional comments"
141+
// REF: issue-339
142+
// SOURCE: n/a
143+
// FORMAT THEOREM: direction in {left,right} -> token in {n,y}
144+
// PURITY: CORE
145+
// EFFECT: n/a
146+
// INVARIANT: output token set is finite and parser-compatible
147+
// COMPLEXITY: O(1)
68148
const booleanChoiceBuffer = (direction: CreateSettingsChoiceDirection): string =>
69149
Match.value(direction).pipe(
70150
Match.when("left", () => "n"),
71151
Match.when("right", () => "y"),
72152
Match.exhaustive
73153
)
74154

155+
/**
156+
* Resolves a horizontal GPU choice to the create-flow buffer token.
157+
*
158+
* @param direction - Horizontal choice direction.
159+
* @returns `"none"` for left and `"all"` for right.
160+
* @pure true
161+
* @effect n/a
162+
* @invariant result is always a valid GPU buffer token.
163+
* @precondition `direction` is a valid CreateSettingsChoiceDirection.
164+
* @postcondition Left maps to `none`; right maps to `all`.
165+
* @complexity O(1)
166+
* @throws Never
167+
*/
168+
// CHANGE: encode GPU left/right choices as create input buffer values
169+
// WHY: display controls should use the same parser-compatible token space as manual input
170+
// QUOTE(ТЗ): "Add concise but compliant TSDoc + functional comments"
171+
// REF: issue-339
172+
// SOURCE: n/a
173+
// FORMAT THEOREM: direction in {left,right} -> token in {none,all}
174+
// PURITY: CORE
175+
// EFFECT: n/a
176+
// INVARIANT: output token set is finite and parser-compatible
177+
// COMPLEXITY: O(1)
75178
const gpuChoiceBuffer = (direction: CreateSettingsChoiceDirection): string =>
76179
Match.value(direction).pipe(
77180
Match.when("left", () => "none"),
@@ -82,9 +185,16 @@ const gpuChoiceBuffer = (direction: CreateSettingsChoiceDirection): string =>
82185
/**
83186
* Resolves horizontal browser setting controls into preview buffer tokens.
84187
*
188+
* @param view - Browser display-settings view that provides the active row.
189+
* @param direction - Horizontal choice direction.
190+
* @returns A parser-compatible buffer token for discrete rows, otherwise null.
85191
* @pure true
192+
* @effect n/a
86193
* @invariant free-text rows return null
194+
* @precondition `view.step` may be inside or outside the display step range.
195+
* @postcondition Returned non-null tokens do not mutate `view.values`.
87196
* @complexity O(1)
197+
* @throws Never
88198
*/
89199
// CHANGE: map display-mode left/right choices to create setting buffer values
90200
// WHY: browser controls should preview discrete settings without committing values
@@ -122,9 +232,16 @@ export const resolveCreateSettingsChoiceBuffer = (
122232
/**
123233
* Moves the create-mode settings selection with wraparound.
124234
*
235+
* @param view - Create-mode view to move.
236+
* @param direction - Vertical navigation direction.
237+
* @returns Moved create-mode view or null when the repo URL step is active.
125238
* @pure true
239+
* @effect n/a
126240
* @invariant returned view has an empty buffer
241+
* @precondition `view` is a non-null CreateModeFlowView.
242+
* @postcondition Non-null result preserves mode and values while clearing transient input state.
127243
* @complexity O(s) where s = number of remaining create steps
244+
* @throws Never
128245
*/
129246
// CHANGE: expose pure create-mode settings navigation
130247
// WHY: arrow-key handling must not mutate the current view
@@ -145,9 +262,16 @@ export const moveCreateSettingsStep = (
145262
/**
146263
* Moves the browser display-settings selection with wraparound.
147264
*
265+
* @param view - Display-mode view to move.
266+
* @param direction - Vertical navigation direction.
267+
* @returns Moved display-mode view or null when no settings range is active.
148268
* @pure true
269+
* @effect n/a
149270
* @invariant returned view has an empty buffer
271+
* @precondition `view` is a non-null DisplayModeFlowView.
272+
* @postcondition Non-null result preserves mode and values while clearing transient input state.
150273
* @complexity O(s) where s = number of display steps
274+
* @throws Never
151275
*/
152276
// CHANGE: expose pure display-mode settings navigation
153277
// WHY: browser settings keep all rows navigable regardless of committed values

0 commit comments

Comments
 (0)