Skip to content

remove: factory command and internal/factory package#46

Merged
greynewell merged 1 commit intomainfrom
remove/factory
Apr 7, 2026
Merged

remove: factory command and internal/factory package#46
greynewell merged 1 commit intomainfrom
remove/factory

Conversation

@greynewell
Copy link
Copy Markdown
Contributor

@greynewell greynewell commented Apr 7, 2026

Summary

  • Removes supermodel factory health|run|improve and all code in internal/factory/
  • Moves the health analysis logic into a new internal/audit/ vertical slice so supermodel audit continues to work unchanged
  • No user-facing behaviour changes outside the removed factory subcommand

What was removed

  • cmd/factory.go — the factory Cobra command group
  • internal/factory/ — health analysis, SDLC prompt rendering, types, zip helper, tests

What was added

  • internal/audit/ — the health analysis, rendering, and zip code that audit depended 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

    • Removed the factory command and all associated subcommands (health, run, improve).
  • Chores

    • Refactored audit command to use updated internal infrastructure.
    • Updated audit command help text to remove outdated Phase 8 gate reference.

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>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

Walkthrough

The factory command is being consolidated into the audit command via a package reorganization. The internal/factory package is being renamed to internal/audit, with factory command logic migrated into cmd/audit.go. The original cmd/factory.go file and factory-specific tests are removed.

Changes

Cohort / File(s) Summary
Command Layer Refactor
cmd/audit.go, cmd/factory.go
cmd/audit.go refactored to incorporate factory logic: new resolveAuditDir, auditAnalyze, and related helpers replace factory command flow. Adds API key validation, zip creation/hashing, and run ID generation. cmd/factory.go entirely removed (215 lines deleted), including runFactoryRun, runFactoryImprove, resolveFactoryDir, factoryAnalyze, and factoryPromptData.
Package Reorganization
internal/audit/health.go, internal/audit/render.go, internal/audit/types.go, internal/audit/zip.go
Package declarations changed from factory to audit across all files, reflecting the package rename from internal/factory to internal/audit. No functional logic modifications.
Removed Factory Package Artifacts
internal/factory/doc.go, internal/factory/factory_test.go, internal/factory/zip_test.go
Package documentation, comprehensive test suites (1108 lines of factory tests, 198 lines of zip tests), and helper tests removed. Tests covered utility helpers, graph computations, health scoring, circular dependency detection, and impact enrichment behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • jonathanpopham

Poem

🏭 ➜ 📊 the factory becomes audit,

Commands consolidated into one strong product,

Tests removed but logic stays,

Packages reorganized—cleaner ways. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: removing the factory command and internal/factory package.
Description check ✅ Passed The description covers what was removed and added, provides context, and explains that there are no user-facing behavior changes outside the removed command.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove/factory

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟠 Major

Replace 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 the audit flow.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 98887ec and bb0b1c7.

📒 Files selected for processing (9)
  • cmd/audit.go
  • cmd/factory.go
  • internal/audit/health.go
  • internal/audit/render.go
  • internal/audit/types.go
  • internal/audit/zip.go
  • internal/factory/doc.go
  • internal/factory/factory_test.go
  • internal/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

@greynewell greynewell merged commit 9651f86 into main Apr 7, 2026
7 checks passed
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