Skip to content

fix: rootCmd args parsed as directory path — breaks watch cache loading#109

Merged
jonathanpopham merged 2 commits intosupermodeltools:mainfrom
jonathanpopham:progressive-updates-e2e-integration
Apr 10, 2026
Merged

fix: rootCmd args parsed as directory path — breaks watch cache loading#109
jonathanpopham merged 2 commits intosupermodeltools:mainfrom
jonathanpopham:progressive-updates-e2e-integration

Conversation

@jonathanpopham
Copy link
Copy Markdown
Contributor

@jonathanpopham jonathanpopham commented Apr 9, 2026

Problem

rootCmd accepted MaximumNArgs(1) and used args[0] as the working directory. Since watch is the default behavior on rootCmd (not a subcommand), any trailing token would be used as the directory path. The daemon would create and read from a wrong directory (e.g. ./watch/.supermodel/shards.json instead of ./.supermodel/shards.json), loading a stale 1-node cache instead of the 2000-node cache from analyze.

Fix

Switch to cobra.NoArgs + explicit --dir flag. 4 lines changed.

Production E2E results

Tested against production API v0.12.0 on the supermodeltools/cli repo:

Step Result
analyze --force 1993 nodes, 4471 rels, 126 shards
Daemon cache load 1993 nodes (was 1 before fix)
Incremental (hook → UDP → API → merge) 1 file → 15 shards re-rendered
Node churn 99% kept (10 removed, 26 added)
Domains Preserved (identical before/after)
Dangling edges 0

Fixes #108

Summary by CodeRabbit

  • Breaking Changes
    • Project directory must now be specified with the --dir flag (default: current directory) instead of as a positional argument.
    • CLI no longer accepts a positional path; update invocations to use, for example, supermodel --dir /path/to/project (or omit to use the current directory).

rootCmd accepted MaximumNArgs(1) and used args[0] as the working
directory. Since watch is the default behavior on rootCmd (not a
subcommand), running `supermodel` with any trailing token would use
that token as the directory. This caused the daemon to create and
read from a wrong directory (e.g. ./watch/.supermodel/shards.json
instead of ./.supermodel/shards.json).

Fix: switch to NoArgs and add an explicit --dir flag. This matches
the pattern used by all other subcommands that accept a directory.

Fixes supermodeltools#108
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

Walkthrough

Root CLI command no longer accepts a positional path. A new persistent --dir flag (default ".") bound to watchDir is added; command usage changed from supermodel [path] with MaximumNArgs(1) to supermodel with NoArgs. Watch directory is read from watchDir only.

Changes

Cohort / File(s) Summary
Command argument handling
cmd/root.go
Replaced positional path argument with a persistent --dir (watchDir) flag (default "."). Updated Use/Args from supermodel [path] + cobra.MaximumNArgs(1) to supermodel + cobra.NoArgs. RunE now uses watchDir instead of args[0].

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • PR #60 — Also changes how the watch path is provided via cmd/root.go, touching flag/positional handling for the watch/daemon CLI.

Poem

A flag now points where watches steer,
--dir whispers, "Start from here."
No more args to fret or pry,
The root command says clear and shy,
Direct the path and let it fly.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: switching from positional args to an explicit --dir flag to fix cache path resolution issues in the watch daemon.
Description check ✅ Passed The description covers the problem, fix, and production E2E test results, but deviates from the template structure (Problem/Fix instead of What/Why/Test plan).
Linked Issues check ✅ Passed The PR directly addresses issue #108 by implementing explicit --dir flag to ensure consistent cache path resolution between analyze and watch commands.
Out of Scope Changes check ✅ Passed All changes are narrowly scoped to fixing the root command's argument handling and adding the --dir flag, directly addressing the cache path resolution bug.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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)
cmd/root.go (1)

34-43: ⚠️ Potential issue | 🟡 Minor

Use still advertises a positional path that is now rejected.

Line 34 says supermodel [path], but Line 43 enforces cobra.NoArgs. So --help currently tells users to do something that fails.

Suggested fix
-	Use:   "supermodel [path]",
+	Use:   "supermodel",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/root.go` around lines 34 - 43, The command metadata is inconsistent: Use
is "supermodel [path]" but Args is set to cobra.NoArgs, causing HELP to
advertise a positional that will be rejected; either remove the positional from
Use or allow one arg. Fix by updating the command definition in root.go: if the
command should accept an optional path, replace Args: cobra.NoArgs with Args:
cobra.MaximumNArgs(1) (or cobra.ArbitraryArgs if multiple allowed) and adjust
any code that reads the arg (refer to the root command initialization and any
handlers that use os.Args or cmd.Args), otherwise change Use to "supermodel" to
reflect no positional argument. Ensure the chosen change keeps help text and
argument parsing consistent.
🤖 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 `@cmd/root.go`:
- Around line 34-43: The command metadata is inconsistent: Use is "supermodel
[path]" but Args is set to cobra.NoArgs, causing HELP to advertise a positional
that will be rejected; either remove the positional from Use or allow one arg.
Fix by updating the command definition in root.go: if the command should accept
an optional path, replace Args: cobra.NoArgs with Args: cobra.MaximumNArgs(1)
(or cobra.ArbitraryArgs if multiple allowed) and adjust any code that reads the
arg (refer to the root command initialization and any handlers that use os.Args
or cmd.Args), otherwise change Use to "supermodel" to reflect no positional
argument. Ensure the chosen change keeps help text and argument parsing
consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f4f58c1b-56d8-4cb1-bb26-41a5e077d81c

📥 Commits

Reviewing files that changed from the base of the PR and between 161c66e and ceb5ec3.

📒 Files selected for processing (1)
  • cmd/root.go

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.

🧹 Nitpick comments (1)
cmd/root.go (1)

43-43: Optional: make the positional-arg error message migration-friendly.

Right now users coming from supermodel <path> will get a generic arg error. A targeted message like “use --dir” makes this transition smoother (example: supermodel watch typo).

💡 Suggested tweak
-	Args:         cobra.NoArgs,
+	Args: func(cmd *cobra.Command, args []string) error {
+		if len(args) == 0 {
+			return nil
+		}
+		return fmt.Errorf("unexpected argument %q; use --dir %q", args[0], args[0])
+	},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/root.go` at line 43, Replace the hard-coded cobra.NoArgs with a custom
Args validation that detects when a positional argument is provided and returns
a migration-friendly error directing users to use --dir; e.g., change the Args
assignment (currently cobra.NoArgs in cmd/root.go) to a function that checks
len(args)>0 and returns an error like "positional paths are not accepted; use
--dir <path> instead" (keep this logic on the root command's Args field so
callers like `supermodel <path>` get the targeted message).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cmd/root.go`:
- Line 43: Replace the hard-coded cobra.NoArgs with a custom Args validation
that detects when a positional argument is provided and returns a
migration-friendly error directing users to use --dir; e.g., change the Args
assignment (currently cobra.NoArgs in cmd/root.go) to a function that checks
len(args)>0 and returns an error like "positional paths are not accepted; use
--dir <path> instead" (keep this logic on the root command's Args field so
callers like `supermodel <path>` get the targeted message).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1b1b887d-8e12-4825-af70-627a3b450971

📥 Commits

Reviewing files that changed from the base of the PR and between ceb5ec3 and b8b642a.

📒 Files selected for processing (1)
  • cmd/root.go

@jonathanpopham jonathanpopham marked this pull request as ready for review April 10, 2026 01:14
@jonathanpopham jonathanpopham merged commit d66d0dc into supermodeltools:main Apr 10, 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.

bug: watch daemon resolves cache path differently than analyze — loads wrong cache

1 participant