Skip to content

change default from ask to agent#13

Closed
piqoni wants to merge 1 commit intomainfrom
test_reviwer
Closed

change default from ask to agent#13
piqoni wants to merge 1 commit intomainfrom
test_reviwer

Conversation

@piqoni
Copy link
Owner

@piqoni piqoni commented Oct 5, 2025

No description provided.

@github-actions
Copy link

github-actions bot commented Oct 5, 2025

∇ Vogte Code Review

Summary:

  • The change flips the default mode to AGENT, but the conditional now inverts the flag behavior. This likely introduces a logic bug and breaks expected semantics of the --agent flag. Risk: High.

Findings:

  • [Severity: High] main.go:~38–44 — Inverted flag behavior
    Explanation: With initialMode := "AGENT" and if *agentPtr { initialMode = "ASK" }, passing --agent now sets the mode to ASK, the opposite of what the flag implies. This is almost certainly unintended based on the change description.
    Suggestion: If the goal is simply to make AGENT the default while keeping --agent as a no-op (backward compatible), simplify to:

    • initialMode := "AGENT"
    • Remove or leave the if block as a no-op:
      if agentPtr { / no-op for backward compatibility */ }
      Alternatively, if you want users to be able to opt out to ASK, invert the control via the flag default:
    • Set the flag default to true: agentPtr := flag.Bool("agent", true, "Start in agent mode")
    • Then use:
      initialMode := "AGENT"
      if !*agentPtr { initialMode = "ASK" }
  • [Severity: Medium] main.go: flags definition (not shown) — Backward compatibility and UX of flags
    Explanation: Changing the default to AGENT without providing a way to explicitly select ASK removes user control unless they pass a new flag. If the flag remains named --agent and defaults to false, it becomes misleading or redundant.
    Suggestion: Either:

    • Introduce a new --ask flag (bool) and define precedence (e.g., --ask overrides --agent).
    • Or change --agent’s default to true so users can do --agent=false to get ASK.
    • Update help text and docs accordingly.
  • [Severity: Low] Tests missing for mode selection
    Explanation: There are no tests verifying mode selection behavior under different flag combinations.
    Suggestion: Add tests covering:

    • No flags => AGENT.
    • --agent => AGENT.
    • If adding --ask: --ask => ASK; both flags => define and test precedence.
    • If changing --agent default to true: --agent=false => ASK.

Verdict: Needs attention


This review was automatically generated by vogte

@piqoni
Copy link
Owner Author

piqoni commented Oct 5, 2025

was just testing reviewer

@piqoni piqoni closed this Oct 5, 2025
@piqoni
Copy link
Owner Author

piqoni commented Oct 6, 2025

/vogte-review

@piqoni piqoni reopened this Oct 6, 2025
@piqoni
Copy link
Owner Author

piqoni commented Oct 6, 2025

/vogte-review

@piqoni piqoni closed this Oct 6, 2025
@github-actions
Copy link

github-actions bot commented Oct 6, 2025

∇ Vogte Code Review

Summary:

  • Changes flip the default mode to AGENT, refactor the GH Action to rely solely on pull_request, and update README flags. Risk is high: the -agent flag semantics are inverted and README appears out of sync with the code.

Findings:

  • [Severity: High] main.go:~line 37–44 — Agent flag inverted and unsafe default
    Explanation: initialMode now defaults to "AGENT" and passing -agent sets it to "ASK". This inverts the meaning of -agent and silently enables AGENT by default, which is a destructive mode per your README. This is a breaking behavior change and a safety risk. It also contradicts README text stating -agent enables agent mode.
    Suggestion: Either:

    • Keep -agent semantics and introduce a new -mode flag to change default safely:
      modePtr := flag.String("mode", "ASK", "initial mode: ASK or AGENT")
      agentPtr := flag.Bool("agent", false, "DEPRECATED: run in AGENT mode")
      // ...
      initialMode := strings.ToUpper(*modePtr)
      if *agentPtr { // deprecated; override for backward compatibility
          initialMode = "AGENT"
      }
      If you truly want AGENT as the default, set the default of -mode to "AGENT" and communicate clearly in CHANGELOG. Do not invert -agent.
    • Or revert to previous logic and keep default ASK to maintain safety and backwards compatibility:
      initialMode := "ASK"
      if *agentPtr {
          initialMode = "AGENT"
      }

    Add tests around flag parsing to ensure mode selection remains correct.

  • [Severity: Medium] README.md — CLI flags/docs drift from implementation
    Explanation: README replaces -generate-context with -output and changes the default output file name to "vogte-output.txt". The code still references contextFile := "vogte-context.txt" and there’s no evidence of an -output flag in this diff. Examples like vogte -output project_structure.txt likely won’t work and will confuse users.
    Suggestion: Align docs with code or implement the new flag:

    • If you intend to support -output:
      outputPtr := flag.String("output", "vogte-context.txt", "The output file")
      // ...
      contextFile := *outputPtr
      Maintain -generate-context for backward compatibility (deprecated), or provide a migration note. Ensure help text matches README.
    • Otherwise, revert README changes related to -output and keep -generate-context examples.
      Also fix the options block spacing and ensure defaults match exactly.
  • [Severity: Medium] .github/workflows/code-review.yml:~steps — Checkout ref may switch to merge commit
    Explanation: Previously, you explicitly checked out the PR head ref. Now you rely on the pull_request default, which checks out the merge commit (refs/pull//merge). This can change what vogte diffs against and lead to incorrect reviews.
    Suggestion: Pin to the PR head:

    - name: Checkout code
      uses: actions/checkout@v4
      with:
        ref: ${{ github.event.pull_request.head.sha }}
        fetch-depth: 0
  • [Severity: Low/Medium] .github/workflows/code-review.yml:~on — Removed issue_comment trigger
    Explanation: You removed the manual “/vogte-review” trigger path. If this was intentional, great; if not, you lost a useful manual trigger.
    Suggestion: If you want to keep it, reintroduce:

    on:
      pull_request:
        types: [opened]
      issue_comment:
        types: [created]
    jobs:
      code-review:
        if: |
          github.event_name == 'pull_request' ||
          (github.event.issue.pull_request && contains(github.event.comment.body, '/vogte-review'))

    Consider also handling pull_request types: [opened, synchronize, reopened] if you want reviews on updates.

  • [Severity: Low] .github/workflows/code-review.yml:~Run vogte review — Error handling swallows failures
    Explanation: Using command || echo "Review failed" ensures the step always succeeds and posts a comment even when vogte fails.
    Suggestion: If you want the job to fail on errors:

    set -euo pipefail
    REVIEW_OUTPUT=$(vogte -review "$PR_DESCRIPTION" 2>&1)

    Or explicitly capture and check exit code to post a clear comment and then fail the job:

    if ! REVIEW_OUTPUT=$(vogte -review "$PR_DESCRIPTION" 2>&1); then
      echo "REVIEW_OUTPUT<<EOF" >> $GITHUB_OUTPUT
      echo "$REVIEW_OUTPUT" >> $GITHUB_OUTPUT
      echo "EOF" >> $GITHUB_OUTPUT
      echo "::error::vogte review failed"
      exit 1
    fi
  • [Severity: Low] README.md — Minor formatting/consistency issues
    Explanation: The options block includes a stray tab and changed default filename that doesn’t match code.
    Suggestion: Normalize spacing and ensure defaults and flag names match the actual binary help output (consider adding a CI check comparing README options versus -h output).

Verdict: Needs attention

This review was automatically generated by vogte

@github-actions
Copy link

github-actions bot commented Oct 6, 2025

∇ Vogte Code Review

Summary:

  • This change flips the default mode from ASK to AGENT, simplifies the PR review workflow, and updates docs. There are high-risk issues: the mode flag logic is inverted, and README flags don’t match the code. The workflow now only runs on PR opened and may checkout the merge ref, which can affect diffs.

Findings:

  • [Severity: High] main.go:~line 36 — Inverted -agent logic
    Explanation: initialMode now defaults to "AGENT" but when -agent is provided, it switches to "ASK". This contradicts expected semantics (“-agent starts agent mode”) and README text. It also likely breaks backward compatibility for users expecting -agent to enable AGENT mode.
    Suggestion: Either:

    • Preserve flag semantics and default ASK:
      initialMode := "ASK"
      if *agentPtr {
      initialMode = "AGENT"
      }
    • If you truly want AGENT as default and keep the -agent flag name, change the flag default to true and invert check:
      // define flag default to true
      agentPtr := flag.Bool("agent", true, "run in agent mode (use -agent=false for ASK)")
      initialMode := "AGENT"
      if !*agentPtr {
      initialMode = "ASK"
      }
      Update README to reflect any change in default and usage (e.g., -agent=false to opt-out).
  • [Severity: High] README.md: Multiple lines — CLI flags/documentation mismatch
    Explanation: Docs replace -generate-context with -output and state default output is “vogte-output.txt”, but code still uses contextFile := "vogte-context.txt" and there’s no evidence of -output-only mode in code. The “No LLM API” example shows vogte -output project_structure.txt without a mode flag that would generate context.
    Suggestion:

    • Revert docs to the current code behavior (retain -generate-context and default file vogte-context.txt), or implement the new flags in code:
      • Add -output string flag and wire it to replace contextFile.
      • Support generating context when only -output is provided, or introduce a clear flag like -generate-context/-context-only.
    • Fix examples:
      • If old behavior: vogte -generate-context [-output project_structure.txt]
      • Ensure default filename in README matches the code.
    • Add/update tests for flag parsing and filename behavior.
  • [Severity: Medium] .github/workflows/code-review.yml: triggers — Workflow only runs on PR opened
    Explanation: You removed issue_comment trigger and limited pull_request to types: [opened], so reviews won’t run on subsequent commits (synchronize) or reopened PRs.
    Suggestion: Expand triggers or clearly document the limitation:
    on:
    pull_request:
    types: [opened, synchronize, reopened, edited]
    workflow_dispatch:

  • [Severity: Medium] .github/workflows/code-review.yml: Checkout ref and diff base
    Explanation: With actions/checkout@v4 and no ref, on pull_request it checks out the PR merge commit (github.sha). Tools diffing against base may produce unexpected results versus diffing the PR head. Previously you explicitly checked out the PR head ref.
    Suggestion:

    • Check out the PR head SHA:
      • uses: actions/checkout@v4
        with:
        ref: ${{ github.event.pull_request.head.sha }}
        fetch-depth: 0
    • If vogte supports specifying base, pass it:
      REVIEW_OUTPUT=$(vogte -review "$PR_DESCRIPTION" -base "${{ github.event.pull_request.base.ref }}" 2>&1 || echo "Review failed")
    • If not, consider aligning a local branch named “main” to the real base ref when base != main to satisfy vogte’s default (stopgap, not ideal).
  • [Severity: Low] .github/workflows/code-review.yml: Removed manual trigger by comment
    Explanation: Feature regression: can’t trigger via /vogte-review comment anymore.
    Suggestion: If intentional, fine; otherwise reintroduce issue_comment with a contains() guard, or provide workflow_dispatch to allow on-demand runs.

  • [Severity: Low] README.md: Minor formatting/accuracy
    Explanation: “Agent Mode” section still says “starting vogte with -agent option” which conflicts with current main.go change (sets ASK when -agent is present).
    Suggestion: Fix wording after correcting the flag logic and defaults.

Verdict: Needs attention

This review was automatically generated by vogte

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