Skip to content

Feat/eng 2697#723

Merged
DatPham-6996 merged 6 commits into
proj/byterover-tool-modefrom
feat/ENG-2697
May 27, 2026
Merged

Feat/eng 2697#723
DatPham-6996 merged 6 commits into
proj/byterover-tool-modefrom
feat/ENG-2697

Conversation

@DatPham-6996
Copy link
Copy Markdown
Collaborator

Summary

  • Problem:
  • Why it matters:
  • What changed:
  • What did NOT change (scope boundary):

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Test
  • Chore (build, dependencies, CI)

Scope (select all touched areas)

  • TUI / REPL
  • Agent / Tools
  • LLM Providers
  • Server / Daemon
  • Shared (constants, types, transport events)
  • CLI Commands (oclif)
  • Hub / Connectors
  • Cloud Sync
  • CI/CD / Infra

Linked issues

  • Closes #
  • Related #

Root cause (bug fixes only, otherwise write N/A)

  • Root cause:
  • Why this was not caught earlier:

Test plan

  • Coverage added:
    • Unit test
    • Integration test
    • Manual verification only
  • Test file(s):
  • Key scenario(s) covered:

User-visible changes

List user-visible changes (including defaults, config, or CLI output).
If none, write None.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording

Checklist

  • Tests added or updated and passing (npm test)
  • Lint passes (npm run lint)
  • Type check passes (npm run typecheck)
  • Build succeeds (npm run build)
  • Commits follow Conventional Commits format
  • Documentation updated (if applicable)
  • No breaking changes (or clearly documented above)
  • Branch is up to date with main

Risks and mitigations

List real risks for this PR. If none, write None.

  • Risk:
    • Mitigation:

- Update curate examples to include implementation context inline
- Replace detached-curate wording with session completion status guidance
- Clarify history and review template language around recent curates
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

Claude finished @DatPham-6996's task in 3m 3s —— View job


PR Review — Skill template cleanup

  • Read changed files & PR diff
  • Review for correctness, security, code quality
  • Post inline comments for specific issues
  • Post summary review with score

Summary

Pure docs-only PR (3 markdown templates under src/server/templates/skill/, +6/-9 lines). The changes:

  1. curate.md:28 — replace brv curate "..." -f src/retry.ts with an inline file reference. ✅ Correct: -f/--files is in CURATE_REMOVED_FLAGS (src/oclif/lib/removed-flags.ts:80-83), and inlining the path is the documented migration.
  2. curate.md:137 — common-mistakes row pivots from "detached curate" to data.status: "done". ✅ Matches the Session Protocol on lines 46-48 and aligns with the removal of --detach from tool-mode curate.
  3. history.md:11-12 — consolidates two bullets into one and drops the "verify a detached curate completed" framing. ✅ Tool-mode curate runs synchronously over kickoff + continuation RPCs, so "detached" framing is obsolete here.
  4. history.md:43-45 — collapses two paragraphs into one. The dropped "still processing" guidance is no longer applicable; the failure-reporting case is still covered by the "Treating failed history entries as saved memory" mistake row.
  5. history.md:55 — mistake row references completed status, which matches the --status completed filter shown on line 26.
  6. review.md:38 — drops "detached" qualifier and pluralizes "marker"; consistent with the rest of the cleanup but note that the project-level CLAUDE.md still describes review markers as a detached-only behavior. Possible doc drift — see inline comment.

Correctness

  • All wording maps to real flags, real status values, and real session-protocol behavior in the current codebase. No false references introduced.
  • Status terminology is internally consistent: done (session response) vs completed (curate-log filter) are used correctly in their respective contexts.

Security

  • N/A — markdown template content only, no secrets, no code paths touched.

Code quality

  • Tightens example accuracy (-f flag was stale).
  • Removes legacy "detached" framing that no longer matches tool-mode curate.
  • Minor info loss on the dropped history.md paragraph, but adequately covered by the mistake rows.
  • Note the PR description is unfilled — the conventional template (Summary / Type / Scope / Test plan / Risks) is left blank. For a docs-only change this is low risk, but please fill the Summary so the historical record matches git log.

Test plan

  • Docs-only; no unit/integration tests required. Manual verification: ensure no agent skill prompt regresses (these .md files are embedded into the agent's system prompt at runtime).

Score: 4.5 / 5

Strong, targeted cleanup that aligns the skill templates with the post-tool-mode codebase. Half-point deduction is split between the unfilled PR template and the minor CLAUDE.mdreview.md wording drift to clean up in a follow-up.
• Branch: feat/ENG-2697

Comment thread src/server/templates/skill/curate.md
Comment thread src/server/templates/skill/curate.md
Comment thread src/server/templates/skill/history.md
Comment thread src/server/templates/skill/review.md
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

Claude finished @DatPham-6996's task in 1m 26s —— View job


PR Review — Skill template cleanup (re-review on synchronize)

  • Read changed files & PR diff
  • Review for correctness, security, code quality
  • Verify prior inline-comment references still hold
  • Post summary review with score

Re-review note

This synchronize event picked up a merge from proj/byterover-tool-mode (commit b1273fb) but the net diff against the base is unchanged from the prior review: still 3 files, +6/-9 in src/server/templates/skill/{curate,history,review}.md. The previously-posted inline comments remain accurate and have not been re-posted to avoid duplicates. PR is already approved by @cuongdo-byterover.

Summary

Pure docs-only cleanup of three skill-template markdown files embedded into the agent system prompt:

  1. curate.md:28 — replace brv curate "..." -f src/retry.ts with an inline file reference. ✅ -f/--files lives in CURATE_REMOVED_FLAGS (src/oclif/lib/removed-flags.ts:80-83); inlining is the documented migration.
  2. curate.md:137 — common-mistakes row pivots from "detached curate" framing to data.status: "done". ✅ Matches the Session Protocol on lines 46–48 and aligns with --detach removal (removed-flags.ts:85-88).
  3. history.md:11-12 — consolidates two bullets into one and drops the "verify a detached curate completed" framing. ✅ Tool-mode curate is synchronous over kickoff + continuation RPCs, so the "detached" framing is obsolete.
  4. history.md:43-45 — collapses two paragraphs into one. The dropped "still processing" guidance is no longer applicable; the failure-reporting case is still covered by the "Treating failed history entries as saved memory" row (history.md:58).
  5. history.md:55 — mistake row references completed status, consistent with the --status completed filter on line 26.
  6. review.md:38 — drops "detached" qualifier, pluralizes "marker". Consistent with the broader cleanup but creates minor wording drift vs. project-level CLAUDE.md (see prior inline comment).

Correctness

  • All wording maps to real flags, real status values, and real session-protocol behavior in the current codebase. No false references introduced.
  • Status terminology is internally consistent: done (session response, curate.md) vs completed (curate-log filter, history.md) are used correctly in their respective contexts.

Security

  • N/A — markdown template content only, no secrets, no code paths touched.

Code quality

  • Tightens example accuracy (-f flag was stale).
  • Removes legacy "detached" framing that no longer matches tool-mode curate.
  • Minor info loss on the dropped history.md paragraph, but adequately covered by the mistake rows.
  • PR description is still left as the unfilled template. Low risk for docs-only but please fill the Summary so the historical record matches git log.

Test plan

  • Docs-only; no unit/integration tests required. Templates are read at runtime by the agent system-prompt assembly (src/server/templates/sections/).

Score: 4.5 / 5

Strong, targeted cleanup that aligns the skill templates with the post-tool-mode codebase. The half-point deduction is split between the unfilled PR template and the minor CLAUDE.mdreview.md wording drift to clean up in a follow-up.

• Branch: feat/ENG-2697

@DatPham-6996 DatPham-6996 merged commit 930656c into proj/byterover-tool-mode May 27, 2026
7 checks passed
@github-actions github-actions Bot mentioned this pull request May 27, 2026
8 tasks
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.

2 participants