Public: anyone can read this; the crew can change it.

Automate a review finding as a lint or test: Merge the second import from perf/live.ts into the existing one. #11650

Parked opened this
Closed without merging

The gate now refuses a second import of the same module in one file, so a repeat of the `perf/live.ts` finding fails the gate instead of waiting for a reviewer. The commit was already on the branch and pushed, so this run changed nothing. The gate, `tsc --noEmit` and `test/review-rules.test.ts` (8 tests) all pass; I did not run the rest of the suite.

stopped parked by the crew deploy d0acba1

opened this

The gate now refuses a second import of the same module in one file, so a repeat of the perf/live.ts finding fails the gate instead of waiting for a reviewer. The commit was already on the branch and pushed, so this run changed nothing. The gate, tsc --noEmit and test/review-rules.test.ts (8 tests) all pass; I did not run the rest of the suite.

  • The rule: it lives in src/kernel/review_rules.ts and is wired into scripts/gate.ts. A second import {…} from "x" of the same module in the same kind (value or import type) is a violation in src/ and test/.
  • Baseline: offenders that predate the rule are listed in scripts/duplicate-imports-allow.json. A new duplicate fails, and so does an entry whose duplicate was fixed, so the list only shrinks. No perf/live.ts duplicates remain, so none are in it.
  • Tests: test/review-rules.test.ts covers the rule.
  • Left out: namespace and side-effect imports are skipped because they can't merge. I did not clean up the existing duplicates in the baseline (../chrome.ts, ../core/runs.ts and the rest); each is a small follow-up that deletes its baseline entry.
  • Try it: add a second import { x } from "../perf/live.ts" to any src/ file and run bun scripts/gate.ts.

Closes #11517

commented

The gate now refuses a second import of the same module in one file, so a repeat of the perf/live.ts finding fails the gate instead of waiting for a reviewer. The commit was already on the branch and pushed, so this run changed nothing. The gate, tsc --noEmit and test/review-rules.test.ts (8 tests) all pass; I did not run the rest of the suite.

  • The rule: it lives in src/kernel/review_rules.ts and is wired into scripts/gate.ts. A second import {…} from "x" of the same module in the same kind (value or import type) is a violation in src/ and test/.
  • Baseline: offenders that predate the rule are listed in scripts/duplicate-imports-allow.json. A new duplicate fails, and so does an entry whose duplicate was fixed, so the list only shrinks. No perf/live.ts duplicates remain, so none are in it.
  • Tests: test/review-rules.test.ts covers the rule.
  • Left out: namespace and side-effect imports are skipped because they can't merge. I did not clean up the existing duplicates in the baseline (../chrome.ts, ../core/runs.ts and the rest); each is a small follow-up that deletes its baseline entry.
  • Try it: add a second import { x } from "../perf/live.ts" to any src/ file and run bun scripts/gate.ts.
commented

The check failed because main had gained a duplicate import that the new rule rightly rejects. test/crew-stopped-twice.test.ts imported ../src/core/runs.ts twice. I merged origin/main into the branch, folded noteRunProgress into the first import and deleted the second line, then pushed.

After the fix, bun scripts/gate.ts, bun x tsc --noEmit and the two test files I ran (test/crew-stopped-twice.test.ts and test/review-rules.test.ts, 11 tests) are all green here. I have not seen the forge's check on the new push, and I did not run the full suite.

What the pull does: the gate now refuses a second import of the same module in one file in src/ and test/, which is the perf/live.ts review finding from #11517. The rule is in src/kernel/review_rules.ts and scripts/gate.ts, with tests in test/review-rules.test.ts.

Baseline: offenders that predate the rule are listed in scripts/duplicate-imports-allow.json. A new duplicate fails, and so does a stale entry, so the list only shrinks.

Left out: namespace and side-effect imports are skipped, since they can't merge. I did not clean up the existing baselined duplicates; each is a small follow-up that deletes its baseline entry.

Try it: add a second import { x } from "../perf/live.ts" to any src/ file and run bun scripts/gate.ts.

🤖 Generated with Claude Code

commented

The failing timers test came from main, not from this pull: src/spawn/host.ts has a setTimeout that wasn't wrapped in during. I wrapped it (named spawn-wake, same error handling) and pushed. The forge's check has not run on the new push yet.

Locally, test/timers-named.test.ts (8 tests), bun scripts/gate.ts and bun x tsc --noEmit all pass. I tried the spawn wake tests as well, but their network calls are denied in this sandbox, so they hung and I stopped them. The behaviour change is small: the wake callback now runs inside during, with errors caught as before. I did not run those spawn tests, so the wake path itself is unverified here.

What the pull does: the gate now refuses a second import of the same module in one file in src/ and test/, which is the perf/live.ts review finding from #11517. The rule is in src/kernel/review_rules.ts and scripts/gate.ts, with tests in test/review-rules.test.ts.

Baseline: duplicates that predate the rule are listed in scripts/duplicate-imports-allow.json. A new duplicate fails, and so does a stale entry, so the list only shrinks.

Left out: namespace and side-effect imports are skipped because they can't merge. I did not clean up the existing baselined duplicates; each is a small follow-up that deletes its baseline entry.

Also in this pull: I folded a duplicate core/runs.ts import in test/crew-stopped-twice.test.ts and wrapped that timer in src/spawn/host.ts. Both came in through merges from main.

Try it: add a second import { x } from "../perf/live.ts" to any src/ file and run bun scripts/gate.ts.

🤖 Generated with Claude Code

commented

Parked by the crew: the check stayed red through 2 automatic tries

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs