Conversation
Greptile SummaryThis PR bumps goldfix to v2026.04.09.23 and includes both a significant refactoring of the goldfix formatter tool internals and widespread formatting fixes (applied via the new formatter) across library files and tests. Key architectural improvements in the goldfix tool include: extraction of Confidence Score: 5/5Safe to merge — all logic changes are well-structured refactorings with no identified correctness regressions. All remaining findings are P2 style/maintainability suggestions. No P0 or P1 issues found. The core logic changes are internally consistent and the widespread formatting fixes are mechanical applications of the updated goldfix tool. tools/goldfix/liii/goldfix-split.scm (normalize-restart-index pass-number coupling) and tools/goldfix/liii/goldfix.scm (stale line-start-states after vector mutation) — both are low-risk P2 notes.
|
| Filename | Overview |
|---|---|
| tools/goldfix/liii/goldfix-split-cond.scm | New file extracting case/cond clause splitting logic into its own module; clean extraction with all helpers included. |
| tools/goldfix/liii/goldfix-split.scm | Major refactoring: replaces 4-round full-pass loop with a 20-step targeted-restart strategy; adds rewrite-split-pass returning (values lines envs changed?), shift-lines-indentation-selectively for env-selective scope, and normalize-restart-index for pass-specific restart routing. |
| tools/goldfix/liii/goldfix.scm | Refactoring of repair-define-header-lines to vector-based line scan; simplification of canonicalize-right-tag-lines; new line-start-state-in-code? helper; widespread closing-paren comment style fixes. |
| tools/goldfix/liii/goldfix-paren-right-tag-plan.scm | Refactors sort-envs-for-insertion to use O(n) parent→max-child-line map, replacing the previous O(n²) find-next-sibling traversal. |
| tools/goldfix/liii/goldfix-env-scan-step.scm | Adds leading-anonymous-followup-open-index and new 'leading-anonymous line-kind handling to correctly process anonymous leading open-forms. |
| tools/goldfix/liii/goldfish-env-scan-state.scm | Introduces 'leading-anonymous as a new line-kind when a new form has an empty tag; enables downstream anonymous-form handling in the scanner. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["normalize-multi-open-lines(lines)"] --> B["remaining-steps=20\ncurrent-envs=#f\nremaining-passes=pass-order"]
B --> C{remaining-steps=0\nor passes empty?}
C -- Yes --> Z["return current-lines"]
C -- No --> D["apply-split-pass\n(current-lines, cached-envs, collect-plan)"]
D --> E["rewrite-split-pass\n→ values(next-lines, changed?)"]
E -- "changed?=true" --> F["clear envs cache\nnext-envs=#f\nrestart from normalize-restart-index"]
F --> G["remaining-steps - 1"]
G --> C
E -- "changed?=false" --> H["keep envs cache\nnext-envs=scanned-envs\nmove to next pass"]
H --> I["remaining-steps - 1"]
I --> C
Reviews (1): Last reviewed commit: "wip" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| (export normalize-multi-open-lines) | ||
|
|
||
| (begin | ||
| (define (normalize-restart-index changed-pass-index) | ||
| (cond | ||
| ;; binding 的拆分会继续暴露新的 binding 行，直接回到 binding 本身 | ||
| ((= changed-pass-index 1) 1) | ||
| ;; map/filter -> lambda 的拆分只会继续暴露后续 nested-lambda | ||
| ((= changed-pass-index 2) 2) | ||
| ;; case 头部拆开后，下一步主要是 clause/body 级别的拆分 | ||
| ((= changed-pass-index 3) 4) | ||
| ;; clause body 被拆到新行后，可能露出 let/map/filter/case 等更早阶段 | ||
| ((= changed-pass-index 4) 1) |
There was a problem hiding this comment.
normalize-restart-index couples to positional pass numbers
normalize-restart-index is keyed on the integer position of the pass in pass-order (1–5). If a pass is ever inserted, removed, or reordered in pass-order, these hardcoded indices silently point to the wrong pass without a compile-time or runtime warning. Consider keying on a symbolic pass name (e.g. 'binding, 'conditional-clause) and deriving the restart index by looking up the name in pass-order, so the mapping stays correct under reordering.
| ;; 若头部净 open 数大于 1，通常表示函数签名缺少了一个 )。 | ||
| (define (repair-define-header-lines lines) | ||
| (let* ((normalized-lines (trim-excess-trailing-rparens lines)) | ||
| (line-start-states (compute-line-start-states normalized-lines)) | ||
| (details (scan-environment-details normalized-lines))) | ||
| (let loop ((remaining details) | ||
| (current-lines normalized-lines)) | ||
| (if (null? remaining) | ||
| current-lines | ||
| (let* ((detail (car remaining)) | ||
| (env (env-detail-env detail)) | ||
| (lparen-line (env-lparen-line env)) | ||
| (rparen-line (env-rparen-line env))) | ||
| (if (and (string=? (env-tag env) "define") | ||
| rparen-line | ||
| (> rparen-line lparen-line)) | ||
| (let* ((line-idx (- lparen-line 1)) | ||
| (line (list-ref current-lines line-idx)) | ||
| (line-state (list-ref line-start-states line-idx)) | ||
| (net-open-count (line-net-open-count-at-state line line-state))) | ||
| (if (and (> net-open-count 1) | ||
| (define-function-header-line? line)) | ||
| (let* ((fixed-line (add-rparens-by-diff line (- net-open-count 1))) | ||
| (new-lines (list-set current-lines line-idx fixed-line))) | ||
| (loop (cdr remaining) new-lines) | ||
| ) ;let* | ||
| (loop (cdr remaining) current-lines) | ||
| (line-start-states (list->vector (compute-line-start-states normalized-lines))) | ||
| (lines-vec (list->vector normalized-lines)) | ||
| (line-count (vector-length lines-vec))) | ||
| (let loop ((line-idx 0)) | ||
| (if (>= line-idx line-count) | ||
| (vector->list lines-vec) | ||
| (let* ((line (vector-ref lines-vec line-idx)) | ||
| (line-state (vector-ref line-start-states line-idx))) | ||
| (if (and (line-start-state-in-code? line-state) | ||
| (define-function-header-line? line)) | ||
| (let ((net-open-count (line-net-open-count-at-state line line-state))) | ||
| (if (> net-open-count 1) | ||
| (vector-set! lines-vec | ||
| line-idx | ||
| (add-rparens-by-diff line (- net-open-count 1)) | ||
| ) ;vector-set! | ||
| #f | ||
| ) ;if | ||
| ) ;let* | ||
| (loop (cdr remaining) current-lines) | ||
| ) ;let | ||
| #f | ||
| ) ;if | ||
| (loop (+ line-idx 1)) | ||
| ) ;let* | ||
| ) ;if | ||
| ) ;let |
There was a problem hiding this comment.
line-start-states not refreshed after mutation
line-start-states is computed once from normalized-lines before the loop begins. When vector-set! patches a define-header line (adding closing parens), the states for all lines after that index become stale — the balance at the start of subsequent lines is now different from what was precomputed. In practice this is likely harmless because (a) the same "stale" issue existed in the previous version and (b) these fixes only add closing parens to toplevel define headers. Worth documenting explicitly if this invariant is intentionally accepted.
1 participant
No description provided.