Skip to content

refactor: split compile-check input key writers into their own file - #3346

Merged
hatayama merged 1 commit into
feature/compile-check-changed-files-checkfrom
refactor/compile-check-split-input-key
Oct 16, 2026
Merged

hatayama merged 1 commit into
feature/compile-check-changed-files-checkfrom
refactor/compile-check-split-input-key

Conversation

@hatayama

@hatayama hatayama commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Summary

User Impact

None. The integration branch's build-cli (and the jobs that depend on it) failed in TestProductionGoFilesStayFocused because input_key.go had 523 lines. This check counts raw lines and runs only after a merge into the integration branch, so the pull requests that grew the file did not see it.

Changes

  • cli/dispatcher/internal/compilecheck/input_key_identity.go (new, 304 lines): the functions that write each identity into the digest, from writeCompilerIdentity to projectPath, in their original order with their comments.
  • cli/dispatcher/internal/compilecheck/input_key.go (now 230 lines): how the key is assembled, with its types and flag lists. Only the imports the moved functions used are removed.

Verification

  • With the import blocks and blank lines removed, the two files concatenated are identical to the original file.
  • go test -run TestProductionGoFilesStayFocused ./internal/architecture/... in cli/release-automation passes.
  • go test -race ./internal/compilecheck/... in cli/dispatcher passes.
  • For each of the four Go modules: gofmt -l is empty, go vet ./... passes, golangci-lint run ./... reports 0 issues, and go test ./... passes in cli/dispatcher and cli/release-automation. In cli/common and cli/project-runner, one test each fails only because the local sandbox refuses a directory under /tmp and a Unix socket bind; neither module is touched here.
  • scripts/check-code-complexity.sh and scripts/check-file-length.sh pass.

View guided diff

input_key.go had grown to 523 lines, over the 500-line limit the
architecture test enforces on production Go files. The functions that
write each identity into the digest move to input_key_identity.go
unchanged; input_key.go keeps how the key is assembled.
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2aeebe4d-25a2-4e4d-a42a-667c28ad655d
📥 Commits

Reviewing files that changed from the base of the PR and between 8b218a3 and d87e3c4.

📒 Files selected for processing: 2
  • cli/dispatcher/internal/compilecheck/input_key.go
  • cli/dispatcher/internal/compilecheck/input_key_identity.go
💤 Files with no reviewable changes: 1
  • cli/dispatcher/internal/compilecheck/input_key.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The change moves compile-check input identity helpers from input_key.go to input_key_identity.go. The helpers cover compiler and response files, sources, references, analyzer inputs, outputs, response-flag parsing, file hashing, and path resolution. Input-key computation remains in input_key.go.

Changes: Compile-check input identity

Layer / File(s) Summary
Flag parsing and file identity helpers
cli/dispatcher/internal/compilecheck/input_key_identity.go
The new file defines response-flag parsing, path resolution, and file metadata and content identity writers.
Compile-input identity entries
cli/dispatcher/internal/compilecheck/input_key.go, cli/dispatcher/internal/compilecheck/input_key_identity.go
input_key.go retains input-key computation and removes the identity helper implementations. input_key_identity.go defines identity entries for compiler and response files, sources, references, analyzer inputs, and outputs.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to d87e3

This refactor moves compile-check identity helpers into their own file to satisfy the file-length check, with no intended behavior change. No concrete merge risk was identified.

### Architecture Summary

Architecture risk: 🟠 High · up to d87e3

The change affects 1 system.

Changed systems: cli

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — cli (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in cli/dispatcher/internal/compilecheck/input_key.go: The import block drops bytes, os, filepath, sort, and strings; it retains the hashing, encoding, formatting, and I/O imports.
  • observed — Modified behavior in cli/dispatcher/internal/compilecheck/input_key.go: Removed the remaining identity and hashing helpers: compiler and response-file identity, source and reference handling, analyzer and output identity, flag parsing, path resolution, and file stat/content hashing. Their validation and error paths were removed with their implementations.
  • observed — Modified behavior in cli/dispatcher/internal/compilecheck/input_key_identity.go: Adds compiler identity entries for the compiler DLL and .NET host using file metadata. Response identity hashes the required primary response file and optional companion file; other companion read errors and any primary read error are returned.
  • observed — Modified behavior in cli/dispatcher/internal/compilecheck/input_key_identity.go: Adds deterministic source identity: source paths are sorted, each file is content-hashed, and the resulting hashes are stored by source name. A separate writer emits those hashes or records sorted source names without content.

Reliability and maintainability

  • inferred — Risk-relevant change factors for cli: blast_radius_1; blast_radius_2; blast_radius_3; blast_radius_5; blast_radius_6; direct_dependents_1; direct_dependents_2; direct_dependents_3
Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: moving compile-check input-key writers into a separate file.
Description check Passed The description directly explains the file split, the line-limit objective, the intended lack of behavior change, and the verification performed.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 1 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@hatayama
hatayama merged commit 869c812 into feature/compile-check-changed-files-check Oct 16, 2026
4 checks passed
@hatayama
hatayama deleted the refactor/compile-check-split-input-key branch October 16, 2026 01:28
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