Skip to content

cleanup: remove tag system, expose equivalents as CLI flags#63

Merged
kalil0321 merged 3 commits into
cleanup/agent-modefrom
cleanup/remove-tags
May 5, 2026
Merged

cleanup: remove tag system, expose equivalents as CLI flags#63
kalil0321 merged 3 commits into
cleanup/agent-modefrom
cleanup/remove-tags

Conversation

@kalil0321
Copy link
Copy Markdown
Owner

@kalil0321 kalil0321 commented May 5, 2026

Stacked on top of #60 (cleanup/agent-mode). Once #60 lands, the base will rebase to main.

Summary

The inline @-tag syntax inside the REPL was undiscoverable (not in --help), drift-prone, and largely a duplicate of existing CLI surface. This PR removes it entirely.

Tag (was) Replacement
@record-only <prompt> manual --no-engineer (already existed)
@id <run_id> engineer <run_id> (already existed)
@id <run_id> <prompt> engineer <run_id> --prompt "..." (new flag)
@id <run_id> --fresh <prompt> engineer <run_id> --fresh --prompt "..." (new flag)
@codegen removed (low usage; ActionRecorder + playwright_codegen module dropped)
@docs deferred to a follow-up docs subcommand
@help removed — superseded by the existing /help slash command

Also removes the dead --reverse-engineer/--no-engineer flag from the agent command. It was parsed but never propagated to the auto path (the only path remaining after #60), so it lied about its effect.

Net diff

−1325 / +51 across 9 files. Three modules deleted entirely (action_recorder.py, playwright_codegen.py, tests/test_action_recorder.py). Three tag parsers (parse_engineer_prompt, parse_record_only_tag, parse_codegen_tag) and four mode-specific help functions removed.

Test plan

  • uv run pytest — 635 passing on this branch; the 5 remaining failures all reproduce on cleanup/agent-mode (this branch's base), none introduced here
  • uv run ruff check src/reverse_api/cli.py — 1 fewer warning than the base
  • engineer --help shows the new --prompt / --fresh flags
  • agent --help no longer shows the dead --reverse-engineer/--no-engineer flag
  • Manual REPL smoke (engineer mode: <run_id> switches, free text re-engineers latest)

Out of scope (follow-up)

  • Dedicated docs subcommand to replace @docs (tracked alongside agent friendliness #62 ideas)
  • The output_mode="docs" parameter in run_engineer is left in place (dead but harmless) so the follow-up just wires the click command without re-implementing the engine

🤖 Generated with Claude Code


Summary by cubic

Removed the inline @-tag system from the REPL in favor of clear CLI flags and simplified flows. Also cleaned the engineer prompt template to show run context (no tag references), fixed engineer --prompt semantics, and removed low-usage codegen modules plus the dead agent flag.

  • Bug Fixes

    • engineer <run_id> --prompt "..." now adds instructions on top of the captured run’s original goal; --fresh makes --prompt a full replacement.
  • Migration

    • @record-onlymanual --no-engineer
    • @id <run_id>engineer <run_id>
    • @id <run_id> <prompt>engineer <run_id> --prompt "..."
    • @id <run_id> --fresh <prompt>engineer <run_id> --fresh --prompt "..."
    • @codegen → removed (dropped ActionRecorder and playwright_codegen)
    • @docs → removed for now (will return as a docs subcommand)
    • @help → use /help in the REPL
    • agent --reverse-engineer/--no-engineer → removed; use manual --no-engineer for HAR-only runs

Written for commit 7300df8. Summary will update on new commits.

Greptile Summary

This PR replaces the undiscoverable inline @-tag system with first-class CLI flags and standard positional arguments, removing ~1325 lines across three deleted modules (action_recorder.py, playwright_codegen.py, tests/test_action_recorder.py) and simplifying the REPL engineer loop to a clean two-branch flow.

  • engineer CLI gains --prompt and --fresh flags to cover the @id <run_id> [--fresh] <prompt> use-cases that previously only worked in the REPL.
  • agent CLI loses the dead --reverse-engineer/--no-engineer flag, which was parsed but never forwarded to the auto-capture path.
  • REPL engineer mode is simplified: any input is either a known run-id (context switch) or free text (additive instruction on the latest run); --fresh is now only accessible via the engineer subcommand.

Confidence Score: 4/5

Safe to merge; all deletions are self-consistent and the new CLI surface matches the documented replacements.

The changes are a straightforward deletion of tag-parsing infrastructure and the modules that depended on it. The new engineer REPL logic correctly reproduces the pre-existing run-id-switch and free-text-instruction paths. The only finding is a leftover reverse_engineer=False argument in run_agent_capture whose return value is never consumed — harmless but slightly misleading about what the function controls.

src/reverse_api/cli.py — the dead reverse_engineer argument in run_agent_capture is worth a quick look.

Important Files Changed

Filename Overview
src/reverse_api/cli.py Major cleanup: removes tag parsing imports, simplifies engineer REPL to a two-branch run_id-or-freetext flow, adds --prompt/--fresh to the engineer CLI command, and drops --reverse-engineer from agent. One small dead-code remnant: reverse_engineer=False is still passed to prompt_interactive_options in run_agent_capture but the key is never read from the returned dict.
src/reverse_api/utils.py Removes three tag-parsing functions (parse_engineer_prompt, parse_record_only_tag, parse_codegen_tag) and get_actions_path; get_scripts_dir is retained and still used by base_engineer/native_host.
src/reverse_api/browser.py Removes enable_action_recording parameter, _inject_action_recorder method, and all action recording teardown code; ActionRecorder import dropped cleanly.
tests/test_utils.py Removes import and test classes for the three deleted tag-parsing utilities and get_actions_path; remaining test coverage is unaffected.
CHANGELOG.md Accurately documents all removed tags, the new --prompt/--fresh flags, and the rationale for each removal.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/reverse_api/cli.py:1013-1026
The `reverse_engineer=False` argument is passed to `prompt_interactive_options` but `options["reverse_engineer"]` is never read afterwards. The agent path inside `prompt_interactive_options` always returns `"reverse_engineer": False` unconditionally, so the parameter has no effect here. Dropping it keeps the call-site honest.

```suggestion
    if prompt is None:
        options = prompt_interactive_options(
            prompt=prompt,
            url=url,
            model=model,
        )
        if "command" in options:
            return
        prompt = options["prompt"]
        url = options["url"]
        model = options["model"]

    agent_provider = config_manager.get("agent_provider", "auto")
```

Reviews (1): Last reviewed commit: "cleanup: remove tag system, expose equiv..." | Re-trigger Greptile

The inline @-tag syntax (@record-only, @codegen, @id, @docs, @help) was
undiscoverable, drift-prone (e.g. @record-only silently ignored on the
agent path), and a duplicate of existing CLI surface. Removed entirely.

Surviving capabilities now live as first-class flags / arguments:
  @record-only            -> manual --no-engineer (already existed)
  @id <run_id>            -> engineer <run_id>     (already existed)
  @id <run_id> <prompt>   -> engineer <run_id> --prompt "..."  (new)
  @id <run_id> --fresh    -> engineer <run_id> --fresh         (new)

Dropped without replacement:
  @codegen + ActionRecorder + playwright_codegen module (low usage,
    superseded by the standard capture+engineer flow)
  @docs (will return as a dedicated `docs` subcommand in a follow-up)
  @help (the existing /help slash command covers it)

Also removes the dead `--reverse-engineer/--no-engineer` flag from the
agent command — it was parsed but never propagated to the auto path.

Net -1325 / +51. 635 tests passing; the 5 remaining failures all
reproduce on cleanup/agent-mode (this branch's base) — none introduced
here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 9 files

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 13926a9b55

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/reverse_api/cli.py Outdated
Comment on lines +1223 to +1225
def engineer(run_id, prompt, fresh, model, output_dir):
"""Run reverse engineering on a previous run."""
run_engineer(run_id, model=model, output_dir=output_dir)
run_engineer(run_id, prompt=prompt, model=model, output_dir=output_dir, is_fresh=fresh)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve original prompt for non-fresh --prompt

When replacing the old non-fresh @id <run_id> <prompt> workflow, passing the new CLI value as prompt makes run_engineer skip prompt = run_data["prompt"] (lines 1252-1253) and send the user's refinement as the entire original task instead of as additional_instructions. For existing runs, reverse-api-engineer engineer RUN --prompt "add pagination" now loses the capture's original goal/context, whereas the removed path explicitly treated non-fresh text as additive instructions; this can cause re-engineering to optimize for only the small refinement text rather than the API that was captured.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ack — fixed in ec9de62. Confirmed by reading run_engineer (cli.py:1239-1257): when prompt is set the prompt = run_data["prompt"] fallback is skipped, so the user's refinement was indeed replacing the captured run's original goal. New wiring matches the legacy semantics:

  • --fresh + --prompt X → main prompt becomes X (replaces original)
  • (no --fresh) + --prompt X → X becomes additional_instructions (original preserved)
  • --fresh only → fresh re-engineer on the original prompt
  • nothing → vanilla re-engineer on the original prompt

Added tests/test_cli_engineer_command.py with one test per combination.

@kind-agent
Copy link
Copy Markdown

kind-agent Bot commented May 5, 2026

Test Results

🛡️ 4/6

Results

# Test Status Details
1 Automated test suite (uv run pytest) ❌ failed Suite finished with 636 passed / 4 failed. The observed failures were outside the changed tag-removal/CLI-flag surfaces, but the branch is not green overall.
2 Lint changed CLI module ❌ failed uv run ruff check src/reverse_api/cli.py still reports existing issues in cli.py (complexity and raise-from guidance), so the requested lint target is not clean.
3 CLI help surface ✅ passed engineer --help shows the new --prompt and --fresh flags. agent --help no longer exposes --reverse-engineer/--no-engineer, while manual --help still exposes --no-engineer for capture-only behavior.
4 Engineer flag routing ✅ passed With a Click-runner smoke test and run_engineer mocked, engineer <run_id>, engineer <run_id> --prompt "...", and engineer <run_id> --fresh --prompt "..." all parsed successfully and passed the expected run_id, prompt, and is_fresh values into run_engineer.
5 REPL tag-removal behavior ✅ passed In the REPL loop, /help still routes to help, while removed @ commands like @help, @codegen, and @record-only ... are no longer treated specially and instead fall through as ordinary prompts.
6 Docs / repository consistency ❌ failed Current README/help text and deleted-module state match the new behavior, but repo-wide docs are not fully free of removed-tag references because older CHANGELOG sections still document deprecated tag features.

Issues Found

Project health checks: The branch is not clean on the requested validation commands. uv run pytest still has 4 failing tests, and uv run ruff check src/reverse_api/cli.py still fails. Even if these appear unrelated to the PR’s core behavior, they prevent a clean pass on the requested verification path.

Docs consistency: The updated README and top changelog entry reflect the new flag-based UX, and the deleted action_recorder.py / playwright_codegen.py modules are indeed gone. However, older CHANGELOG history still contains references to @docs, @record-only, and Playwright codegen, so the repository is not fully free of removed-tag documentation.

Summary

I verified the core change itself: the explicit engineer --prompt/--fresh replacement works, agent no longer advertises the dead reverse-engineer toggle, /help still works in REPL, and removed @ tags are no longer interpreted as commands. I’m marking this run failed because the requested health checks are not green and because documentation consistency is only partial at the repo level, even though the primary changed CLI behavior looks correct.

⚠️ Note: This is a large PR — the diff was truncated for analysis. Test coverage may not reflect all changes. Consider splitting large PRs for more thorough testing.


View full run details · Tested by Kind

I tested 13926a9 (cleanup: remove tag system, expose equivalents as CLI flags)

📹 View browser recording

kalil0321 and others added 2 commits May 5, 2026 20:01
…fresh

When the user runs `engineer <run_id> --prompt "X"` on an existing capture
*without* --fresh, the previous wiring passed X straight through as the main
prompt. That bypassed the `prompt = run_data["prompt"]` fallback in
run_engineer, so the captured run's original goal/context was lost and the
SDK only saw the small refinement.

Now --prompt is treated like the legacy `@id <run_id> <prompt>` syntax:
  - no --fresh: --prompt becomes additional_instructions, original goal kept
  - --fresh:    --prompt fully replaces the original goal

Adds tests/test_cli_engineer_command.py covering all four flag combinations.

Reported by chatgpt-codex-connector on PR #63 (P2).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Self-audit follow-up to the tag removal: the engineer/user.md prompt
template still framed the session as "Tag-Based Workflows" and printed
"@id <run_id>" / "@docs" labels into the LLM context. These referenced
tags that no longer exist in the CLI, would confuse the SDK's model,
and could surface in user-visible LLM output.

- Renamed the section to "Run Context" and dropped the @id formatting
- Removed the now-unused tag_extra template variable from base_engineer
  and the corresponding test_prompts harness
- Updated test_prompt_includes_tag_context -> test_prompt_includes_run_context
- Sharpened the CHANGELOG to spell out --prompt semantics (additive
  without --fresh, replacement with --fresh) — matches the fix in ec9de62

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kalil0321 kalil0321 mentioned this pull request May 5, 2026
@kalil0321 kalil0321 merged commit 19311d8 into cleanup/agent-mode May 5, 2026
2 checks passed
@kalil0321 kalil0321 mentioned this pull request May 5, 2026
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