yakoshi

Какие agent skills я реально использую, и почему ревью стало дорогим

Агентами пользуюсь каждый день. Дорого обычно не «сломанный» код, а код, который выглядит законченным - пока не садишься ревьюить. Выложил небольшой пак skills, которые подталкивают вывод к diff'ам, которые потом ещё можно читать.

Skills for intentional code — yakoshiq/skills
Skills for intentional code — yakoshiq/skills

Проблема

Агенты у меня в ежедневной работе: скелет фичи, рефакторинг, тесты, скучная половина задачи. Я не против AI и не собираюсь писать ещё один пост «AI-код нельзя сопровождать».

Сбой уже и раздражающе. Вывод выглядит готовым. Типы сходятся, тесты зелёные, summary уверенный. Через неделю открываешь diff - и не хватает как раз сложного: почему четыре разных отказа схлопнулись в один false, кто владеет этим состоянием при конкуренции, зачем комментарий, если имя и так всё говорит, поймал бы тест тот баг, от которого он якобы защищает.

На одном файле нормально. На неделе agent-assisted работы в живом проекте это уже налог. Надоело его платить, поэтому собрал skills for intentional code.

Что я называю review cost

Не «насколько код правильный в абстрактном смысле», а сколько времени человеку реально стоит ему доверять: прочитать имена, восстановить failure modes, проверить concurrency-историю, понять, доказывают ли тесты хоть что-то. Эта цена растёт, когда автор оптимизирует за «выглядит готовым».

Тот же инстинкт, что и у security-maintenance cost для фреймворков. Конкретный вопрос простой: если отдать этот diff другому разработчику - или себе через три месяца - можно ли его проревьюить, не восстанавливая замысел с нуля?

Примеры, которые всплывают снова и снова

Ниже - укороченные куски из before/after пака. Полные версии в репозитории; здесь важна форма проблемы.

Один false на все ошибки

Путь списания с кошелька, который «работает», но прячет все исходы за одним возвратом:

ts
export function process(d: any, flag = true, mode = 0): any {
  try {
    if (mode !== 1) return false;
    if (
      typeof d.uid !== "string" ||
      typeof d.amt !== "number" ||
      d.amt <= 0 ||
      !(d.uid in wallets)
    ) {
      return false;
    }

    const bal = wallets[d.uid] - d.amt;
    wallets[d.uid] = bal;
    if (flag) notifyProvider(d.uid, -d.amt);
    return bal;
  } catch {
    return false;
  }
}

Невалидная сумма, нет кошелька, неверный mode, notify упал после успешного списания - снаружи всё одно и то же. Переименовать в updateWallet почти бесполезно, если модель ошибок осталась схлопнутой.

От агента я хочу другое: явная domain-операция для новых callers, разные failures, а старый process - тонкий adapter, если нужна совместимость:

ts
export function debitWallet(
  userId: string,
  amount: number,
  opts: { notify?: boolean } = {},
): number {
  if (amount <= 0) throw new InvalidDebit(amount);
  if (!(userId in wallets)) throw new WalletNotFound(userId);

  const balance = wallets[userId] - amount;
  wallets[userId] = balance;

  if (opts.notify ?? true) {
    try {
      notifyProvider(userId, -amount);
    } catch (cause) {
      // Списание уже закоммичено - не прячем это за общим false.
      throw new NotifyFailedAfterDebit(userId, balance, cause);
    }
  }

  return balance;
}

Вот для чего jane-street-style: не «красивее имена», а API, по которому ревьюер может рассуждать.

Комментарии, которые пересказывают код

Агенты обожают такой паттерн:

ts
// Validate the request.
if (!req.productId || req.qty <= 0) return false;

// Get the stock.
const stock = inventory[req.productId] ?? 0;

// Charge the user.
charge(req.userId, req.unitPrice * req.qty);

От этого толку ноль. Помогает то, чего имена не несут - policy, порядок, внешние ограничения:

ts
// force is the operations override for VIP oversell.
if (stock < req.qty && !opts.force) return false;

// Reserve before charge so concurrent checkouts cannot both pass the
// stock check. Charge failure restores the accepted brief over-hold.
inventory[req.productId] = stock - req.qty;

// Stripe webhook delivery can race the read model; see #1842.
scheduleConfirm(req.orderId, { afterMs: 2000 });

Это essential-comments: оставлять why, выкидывать narration.

Зелёные тесты почти без доказательств

ts
it("processTransfer calls its dependencies", async () => {
  wallets.get.mockResolvedValue({ id: "w1", balance: 100 });
  await processTransfer({ from: "w1", to: "w2", amount: 40 }, { wallets, notify });
  expect(wallets.get).toHaveBeenCalledWith("w1");
  expect(wallets.save).toHaveBeenCalled();
  expect(notify).toHaveBeenCalled();
});

Такой тест может остаться зелёным, пока уезжает неверная сумма, сохраняется только одна сторона, или failure notify внезапно откатывает уже закоммиченные балансы. Мне нужно скорее такое:

ts
it("keeps committed balances when notification fails", async () => {
  const wallets = memoryWallets({ w1: 100, w2: 10 });
  const notify = vi.fn().mockRejectedValue(new Error("provider down"));

  const result = await processTransfer(
    { from: "w1", to: "w2", amount: 40 },
    { wallets, notify },
  );

  expect(result).toEqual({
    ok: false,
    error: "notify_failed",
    committed: true,
  });
  expect(wallets.snapshot()).toEqual({ w1: 60, w2: 50 });
});

Это tests-that-matter. Зелёный цвет - ещё не confidence.

Что в паке

Репозиторий публичный: github.com/yakoshiq/skills. Установка - одна команда:

npx skills add yakoshiq/skills

Работает с Pi, Claude Code, Cursor, Codex и другими агентами со skills. В picker полный набор - Intentional Code. Skills ровно пять, файлы короткие специально:

  1. jane-street-style - semantic clarity, когда это явная цель: domain-имена, честные failures, полезные типы. Следуй языку и репозиторию, не тащи functional religion.
  2. essential-comments - оставлять why / invariants / tradeoffs / external constraints. Удалять narration и AI-residue.
  3. concurrency-invariants - назвать гарантию до выбора мьютекса, retry, транзакции или idempotency key. Ownership, commit points, replay, interruption. Timeout обычно unknown outcome, пока API не доказывает обратное.
  4. surgical-changes - наименьшее coherent-изменение, которое полностью закрывает запрос. Только required и coupled правки; adjacent cleanup - не в diff.
  5. tests-that-matter - доказывать observable behavior, failure semantics, invariants, boundaries.

Before/after лежат отдельно от skill-папок, чтобы установленные skills оставались model-facing. Ещё прогонял пак на нескольких актуальных моделях - DeepSeek, MiMo, GLM, Qwen, MiniMax, Kimi, GPT среди них - и выкидывал то, что «работало» только на одной любимой. Универсальности это не даёт. Просто меньше привязки к defaults одной лаборатории.

Как я ими реально пользуюсь

Auto-trigger по description зависит от модели. Кто-то подхватывает skill сам, кто-то нет. Когда задача ровно про одно, предпочитаю явный invoke: /skill:essential-comments или «use surgical-changes» в запросе.

Грубо:

  • размытый domain API, схлопнутые ошибки, primitive soup → jane-street-style
  • шумные комментарии, AI-residue, нет why → essential-comments
  • очереди, воркеры, retries, websockets, гонки → concurrency-invariants
  • маленький фикс, соседний код не трогать → surgical-changes
  • coverage без confidence, mock-heavy suite → tests-that-matter

Diff я всё равно читаю. Skills в основном меняют, на чём спотыкаюсь: меньше narration-комментариев, меньше API вида success: boolean, меньше тестов, которые проверяют только что мок вызвали. Иногда сложный вопрос только назван и всё ещё неверен - это уже лучше тихой каши и дешевле чинить.

Skill не выдаст вкус и не создаст культуру ревью. Кривая задача останется кривой. Если код никто не будет читать - пак не нужен, просто шипь. Если модель skill полуигнорирует, считай это bias, не контракт.

Зачем выложил

Не потому что боюсь, будто агенты «убьют craft». Генерация дешёвая. Дорогими остаются ревью, доверие и возможность потом менять код без археологии.

Если полезно:

skills.yakoshi.dev · github.com/yakoshiq/skills

npx skills add yakoshiq/skills

yakoshi.dev