remove: factory command and internal/factory package#46
Conversation
The factory feature (supermodel factory health|run|improve) is removed. The health analysis logic it contained is moved into a new internal/audit/ vertical slice so the existing 'supermodel audit' command continues to work. - delete cmd/factory.go - delete internal/factory/ (health.go, render.go, types.go, zip.go, tests) - add internal/audit/ with the health analysis, rendering, and zip logic - update cmd/audit.go to import internal/audit instead of internal/factory Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughThe Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/audit/render.go (1)
214-214:⚠️ Potential issue | 🟠 MajorReplace removed command references in generated prompts.
Line 214 and Line 362 still tell users to run
supermodel factory health, but that subcommand is removed in this PR. Generated instructions will fail as written.Proposed fix
- fmt.Fprintln(w, "Run `supermodel factory health` and confirm status is HEALTHY before closing the improvement pass.") + fmt.Fprintln(w, "Run `supermodel audit` and confirm status is HEALTHY before closing the improvement pass.")-Run ` + "`supermodel factory health`" + ` and verify: +Run ` + "`supermodel audit`" + ` and verify:Also applies to: 362-362
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/audit/render.go` at line 214, The generated prompt still references the removed subcommand `supermodel factory health` (in the fmt.Fprintln(w, ...) calls at the locations shown); update those two strings (lines around the fmt.Fprintln calls at the occurrences in internal/audit/render.go) to either remove the obsolete command or replace it with the correct existing command/verification steps used in this PR (e.g., the current factory status check), so generated instructions no longer point to the removed `supermodel factory health`.
🧹 Nitpick comments (3)
cmd/audit.go (1)
53-66: Consider reusing one archive/hash for both analyze + impact.Right now the command zips and hashes twice (Line 94-103 and Line 120-129). It works, but doubles disk I/O for large repositories.
Refactor direction
-func auditAnalyze(cmd *cobra.Command, rootDir, projectName string) (*api.SupermodelIR, error) { +func auditAnalyze(cmd *cobra.Command, zipPath, projectName, runSuffix string) (*api.SupermodelIR, error) { @@ - fmt.Fprintln(cmd.ErrOrStderr(), "Creating repository archive…") - zipPath, err := audit.CreateZip(rootDir) - if err != nil { - return nil, fmt.Errorf("create archive: %w", err) - } - defer func() { _ = os.Remove(zipPath) }() - - hash, err := cache.HashFile(zipPath) - if err != nil { - return nil, fmt.Errorf("hash archive: %w", err) - } @@ - return client.AnalyzeDomains(ctx, zipPath, "audit-"+hash[:16]) + return client.AnalyzeDomains(ctx, zipPath, "audit-"+runSuffix) }Then in
runAudit, build archive/hash once, defer delete once, and pass the same values to both analyze and impact calls.Also applies to: 84-111, 114-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/audit.go` around lines 53 - 66, The code currently creates the repository archive and hash twice (once for analyze and once for impact), causing duplicated I/O; update runAudit to build the archive and hash once, defer a single cleanup, and pass those same archive/hash values into auditAnalyze (or refactor auditAnalyze to accept precomputed archive/hash) and runImpactForAudit (or change runImpactForAudit signature to accept them) so both Analyze (audit.Analyze) and EnrichWithImpact use the same archive/hash without re-zipping or re-hashing; ensure only one defer delete is used and adjust calls to functions like auditAnalyze, runImpactForAudit, and any helpers that currently rebuild the archive/hash.internal/audit/types.go (1)
17-17: Update stale comments that still say “factory”.Line 17 and Line 93 still describe factory-specific behavior even though the package is now
audit. Small thing, but worth aligning for clarity.Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/audit/types.go` at line 17, The comments in types.go reference "factory" but the package is now audit; update the doc comment for HealthReport and any other comments (e.g., the comment at the second occurrence around line 93) to remove "factory"-specific wording and replace it with correct audit/package-neutral language (for example: "HealthReport is the output of a health analysis" or "audit health analysis"), ensuring the symbol name HealthReport remains documented and the intent matches the audit package.internal/audit/zip.go (1)
47-47: Rename the temp archive prefix to match theauditflow.Line 47 still creates
supermodel-factory-*.zip. This won’t break behavior, but it’s confusing now that factory was removed.Proposed cleanup
- f, err := os.CreateTemp("", "supermodel-factory-*.zip") + f, err := os.CreateTemp("", "supermodel-audit-*.zip")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/audit/zip.go` at line 47, The temp archive filename still uses the old "supermodel-factory-*.zip" prefix; update the os.CreateTemp call that assigns to f (the temp zip file creation) to use an audit-appropriate prefix (e.g., "audit-*.zip") so the temporary file name reflects the audit flow and removes the outdated "factory" reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/audit/render.go`:
- Line 214: The generated prompt still references the removed subcommand
`supermodel factory health` (in the fmt.Fprintln(w, ...) calls at the locations
shown); update those two strings (lines around the fmt.Fprintln calls at the
occurrences in internal/audit/render.go) to either remove the obsolete command
or replace it with the correct existing command/verification steps used in this
PR (e.g., the current factory status check), so generated instructions no longer
point to the removed `supermodel factory health`.
---
Nitpick comments:
In `@cmd/audit.go`:
- Around line 53-66: The code currently creates the repository archive and hash
twice (once for analyze and once for impact), causing duplicated I/O; update
runAudit to build the archive and hash once, defer a single cleanup, and pass
those same archive/hash values into auditAnalyze (or refactor auditAnalyze to
accept precomputed archive/hash) and runImpactForAudit (or change
runImpactForAudit signature to accept them) so both Analyze (audit.Analyze) and
EnrichWithImpact use the same archive/hash without re-zipping or re-hashing;
ensure only one defer delete is used and adjust calls to functions like
auditAnalyze, runImpactForAudit, and any helpers that currently rebuild the
archive/hash.
In `@internal/audit/types.go`:
- Line 17: The comments in types.go reference "factory" but the package is now
audit; update the doc comment for HealthReport and any other comments (e.g., the
comment at the second occurrence around line 93) to remove "factory"-specific
wording and replace it with correct audit/package-neutral language (for example:
"HealthReport is the output of a health analysis" or "audit health analysis"),
ensuring the symbol name HealthReport remains documented and the intent matches
the audit package.
In `@internal/audit/zip.go`:
- Line 47: The temp archive filename still uses the old
"supermodel-factory-*.zip" prefix; update the os.CreateTemp call that assigns to
f (the temp zip file creation) to use an audit-appropriate prefix (e.g.,
"audit-*.zip") so the temporary file name reflects the audit flow and removes
the outdated "factory" reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81acf387-ae12-420b-9981-b5fd1afc0874
📒 Files selected for processing (9)
cmd/audit.gocmd/factory.gointernal/audit/health.gointernal/audit/render.gointernal/audit/types.gointernal/audit/zip.gointernal/factory/doc.gointernal/factory/factory_test.gointernal/factory/zip_test.go
💤 Files with no reviewable changes (4)
- internal/factory/doc.go
- internal/factory/factory_test.go
- internal/factory/zip_test.go
- cmd/factory.go
Summary
supermodel factory health|run|improveand all code ininternal/factory/internal/audit/vertical slice sosupermodel auditcontinues to work unchangedfactorysubcommandWhat was removed
cmd/factory.go— thefactoryCobra command groupinternal/factory/— health analysis, SDLC prompt rendering, types, zip helper, testsWhat was added
internal/audit/— the health analysis, rendering, and zip code thatauditdepended on, repackaged as a proper vertical slice (health.go,render.go,types.go,zip.go)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Removed Features
factorycommand and all associated subcommands (health,run,improve).Chores