From b02ea99e40d7585353db72a4bb115f2922d1bb44 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:44:45 +0000 Subject: [PATCH] chore: stop tracking local project guidance Amp-Thread-ID: https://ampcode.com/threads/T-01a19864-d61b-726e-9bda-cd9b953985fc --- AGENTS.md | 313 ------------------------------------------------------ 1 file changed, 313 deletions(-) delete mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md deleted file mode 100644 index 84c2a95b6..000000000 --- a/AGENTS.md +++ /dev/null @@ -1,313 +0,0 @@ -# Agent Instructions - -## Testing - -- Actively look for opportunities to add sensible integration and E2E tests, not only unit tests. - DuckDB makes real end-to-end coverage cheap in this repository: prefer exercising the real CLI, - compiler, executor, and state paths over mocking them. Unit-only coverage of new behavior gives a - false sense of completion and lets the product rot; every new feature or behavior change should - ship with at least one integration or E2E test proving the complete path works. - -- Run targeted pytest commands with xdist by default: `uv run pytest -n auto --dist loadfile`. -- Do not run pytest serially unless diagnosing an xdist-specific failure. -- `make test` is the sanctioned broader local unit and integration check. It is normally acceptable - when the change warrants broader regression coverage because the target owns the required paths, - marker exclusions, xdist settings, and logging. -- Do not replace `make test` with a raw broad pytest command. Do not run `make test-all`, E2E, dbt, - real-warehouse, or the full CI suite locally unless reproducing a specific failure or the user - explicitly requests it; rely on CI for those suites. -- Before pushing, run the exact static CI target with all optional dependencies available: `uv sync --all-extras` followed by `make check-ci`. - -## DuckShop Edge Cases - -- DuckShop (`benchmarks/duckshop`) is the all-round stress test for the compiler, Rules, unit - tests, scenarios, and translation from Snowflake SQL. Edge cases found anywhere must end up in it. -- A fix for a bug, review finding, or recorded hazard that involves a project shape (a model, macro - or test pattern), a data value, or a lifecycle sequence (for example a view/table switch or a - microbatch retry) adds a matching synthetic case to DuckShop's edge-case area in the same change. - If it does not apply, such as command-line behavior or output formatting, the pull request says - why. -- The edge-case area stays selectable on its own, and benchmark comparisons exclude it, until the - stress version replaces the current DuckShop and the benchmark is re-baselined on it. - -## Real-Project Verification and Performance Guards - -- Synthetic fixtures and clean benchmark projects do not exercise every path. Before releasing - changes to compilation, SQL analysis, diagnostics, formatting, Rules, or planning, run the built - branch against at least one large real project available locally outside this repository, and - compare it with the latest release. Compare diagnostic counts by code, cold compile wall time and - peak memory, and, for formatter changes, that formatting preserves compiled dependencies, lineage, - and query semantics. -- Treat every new diagnostic on a real project as a false positive until warehouse evidence proves - otherwise, for example a model that has built successfully since its SQL last changed. -- A material slowdown or new false positive is a release blocker, not a follow-up. -- Real-project names, SQL, identifiers, file paths, and results never enter this repository, commits, - pull requests, or CI output. Record only neutral aggregate conclusions, and rebuild every - reproduction synthetically. -- Performance guards must cover worst-case paths, not only clean projects: very large models, - diagnostic-heavy compiles with many errors and warnings, wide queries, and deep macro expansion. - Any per-diagnostic, per-reference, or per-token work must be bounded by a test with a strict time - limit. A path that is only slow when something is wrong is still a regression. - -## Performance Work - -- Never compare timings across different machines, runners, or sessions. Every performance claim - must come from a same-machine comparison: base and candidate built from the same toolchain, run - alternately at least three times each, reporting medians of both wall time and CPU time - (user+sys) plus the per-phase `compile_timings`. -- On shared local machines, confirm no concurrent builds or benchmarks are running and record the - load average. If load exceeds the core count, treat wall times as unreliable and report CPU time. -- Hosted CI runners vary by up to ~60%. Do not diagnose regressions from absolute CI budgets or - from runs on different runners; use the same-runner ratio guard or a same-job paired comparison. -- Before attributing a regression to a cause, prove it with an on/off experiment (a temporary - kill switch or a revert) on the same machine. Profiles identify suspects; experiments convict. - Remove experiment switches before committing. -- Profile native code with a native sampling profiler and call graphs (for example `perf` with frame - pointers). Python-level samplers misattribute time inside the extension. -- Before building an optimisation, confirm from the profile or code that the target path actually - runs on the benchmark workload under its dialect and configuration. -- Estimate the ceiling first from a per-second CPU timeline. Removing CPU from a phase that already - saturates every core saves little wall time; single-core phases bound what parallelism can save. - Do not build changes whose ceiling is small. -- Check output identity (compiled files and compile JSON excluding timings) before timing a change, - and force the path under test: for example, edit one model so per-model caches are consulted - instead of whole-project reuse. -- Measure each change against the current integrated baseline; independent savings do not simply - add up. Report only measured results or profile-derived ceilings, never projected savings. -- Measure the modes users run: cold without cache, cold writing the cache, and warm. A change that - affects analysis, validation, or caching must report all three. -- Performance fixes must not reduce coverage. Show that diagnostics are unchanged or a strict, - validated superset on the dense benchmark and a real-project run. -- When load makes wall times unreliable, compare `perf stat -e instructions:u` (repeat runs agree - to about 0.05%). Instruction counts cannot show overlap, so judge parallelism and background-work - changes by paired wall time from quiet runs. -- Before timing a candidate, prove its extension binary contains the change (for example, a new - symbol or string in the built `.so`); a stale build silently measures the base. -- Read the timeline for main-thread/pool alternation: serial native loops on the main thread while - the pool idles cost more wall time than CPU-heavy phases that already use every core. -- Work that runs concurrently with another phase saves wall time only while it is the longer of the - two; identify the critical path before optimising one side. Report the overlapped phase's own - timing too, because background work can slow the phase it overlaps by as much as it saves. -- Speculative early work must verify or discard: compare a digest of the real request at the normal - call site, hold cache writes until adoption, never run user code early, and treat start, panic - or evaluation failures as misses that fall back to the normal path. -- Do not raise performance budgets or tolerances to make a regression pass. -- Temporary user-approved exception (2026-10-10), removable after post-conversion memory - optimisation: only the 10,000-model case in `test_fresh_process_compile_cache_performance.py` - may use 3 GiB peak RSS instead of 2.5 GiB; release-performance `lineage column trace` may use - a 2.0 peak RSS ratio and `dag --json` a 1.5 ratio instead of 1.25. Smaller cache cases, - cold-only guards, all other commands, and all speed/correctness limits retain their budgets. - The prohibition above applies to every other budget or tolerance increase. -- Additional temporary user-approved exception (2026-10-10): the dense 3,000-model - base/head edit comparison may use 1.20x wall and 1.75x CPU; release-performance - `compile (one-model edit)` may use 1.35x peak RSS. Remove after post-conversion - optimisation. Cold/warm modes, other workloads and correctness gates remain unchanged. - -## Native SQL (Polyglot) Changes - -- Treat every Polyglot change as a compiler change: before adopting a new Polyglot version, run the - same-machine dense and fresh 3,000-model comparisons against the current version and report the - ratio and per-phase timings. -- Polyglot diagnostics adopted by SQLBuild are enforceable errors or dropped. Never surface a new - native finding as a non-blocking warning; map it to a SQLBuild error code with a mechanical remedy. -- Coverage conditions must not depend on incidental query shape (for example, only checking - statements with a set operation). Checks apply uniformly or the gap is documented and ticketed. -- Run the Polyglot feature-gate checks (`make test-rust-feature-gates`) before pushing Polyglot - changes; cross-module helpers must respect feature gates. - -## Public Repository Hygiene - -- Treat every tracked file, generated artifact, fixture, benchmark, filename, commit, branch, pull - request, review comment, screenshot, and CI log as publicly visible. -- Use only neutral synthetic examples such as orders, customers, products, inventory, fulfillment, - support tickets, and product-owned identifiers. Never copy organization-, client-, provider-, - environment-, infrastructure-, or production-specific names, data, query output, identifiers, or - links into public artifacts, even when the source operation was read-only. -- Keep real-system validation evidence in its approved private tracking location. Do not paste that - evidence into source, tests, documentation, generated skills, benchmarks, commit metadata, pull - requests, or CI output. -- Never put private issue-tracker identifiers (for example `ABC-123`) in commits, branch names, pull - request titles or bodies, code, comments, tests, or documentation. Describe the change instead. -- Load and follow the `public-repository-hygiene` skill before editing or publishing this repository. - Run its private scanner over the working tree and introduced history before every push, and - rescan after generation, formatting, rebasing, or merging. -- Never bypass the configured public-repository Git hooks. A missing or failing private scanner is - a publication blocker. -- Do not copy the private restricted-vocabulary list or scanner implementation into this public - repository. - -## Command-Line User Experience - -- Keep lifecycle feedback consistent across commands. Potentially slow discovery, connection, - inspection, planning, and execution phases must emit a concise start message and an explicit - success or failure completion message; do not leave users watching a blank terminal with no - indication that work is active. -- Preserve machine-readable stdout. When JSON or another machine-output mode is active, send - lifecycle progress to stderr so redirected output remains valid while interactive users still see - meaningful progress. -- Prefer a few meaningful phase transitions over noisy per-resource narration. Long-running TTY - phases should use the shared transient progress facilities where practical. -- Maintenance notices and unrelated warnings must not be the final visible output of an otherwise - successful command. Report them before command execution or ensure the command still prints an - unambiguous terminal success state afterward. -- Use `sqb debug` as the first diagnostic when distinguishing authentication, connection, - configuration, and command-specific failures. - -## User Documentation - -- Every change to the docs site (`website/src/content/docs/`) must follow `website/STYLE.md`: - user-facing content only, task-first structure, tables for per-adapter facts, and no history - wording or internal mechanisms. -- When a change affects a docs page, edit the page into shape: update the section the fact belongs - to and remove what is no longer true. Never append a paragraph to a page or section. - -## Subagent Verification - -- Read-only review subagents are standing-authorized at meaningful review boundaries and do not - require a task-specific user request. This standing authorization does not extend to implementation - or open-ended research delegation. The bounded review cycle below remains the required limit. -- The primary agent owns the overall verification plan. Do not ask multiple subagents to run the same broad test suites. -- Implementation subagents should run only tests directly covering changed behavior plus targeted lint, type, and architecture checks for touched files. -- Review subagents are read-only by default. They should inspect the diff and relevant call flow and run only focused tests needed to validate a concrete suspected finding. -- Follow-up reviews should verify only previously reported findings and affected boundaries rather than repeating the complete review or full suite. -- `make test` may be used for broader local unit and integration verification; use repository CI for - `test-all`, end-to-end, dbt, real-warehouse, and other full suites unless the user explicitly - requests local verification or CI is unavailable. -- State the expected verification scope in subagent prompts and explicitly prohibit unnecessary full-suite runs. -- Do not delay committing and pushing a focused fix solely to repeat checks already completed successfully by another agent or CI. -- Subagents reporting performance results must include wall time, CPU time, run count, machine - load, and whether base and candidate ran on the same machine. Results lacking these are - hypotheses, not evidence. - -## State and Source of Truth - -- Do not store what can be calculated. If a value can be derived from the project or the live - warehouse at the point of use, derive it there and then instead of persisting it and reading it - back later. -- Stored state is for history, auditing, logging, and facts that genuinely cannot be recalculated: - what was built, when, at which version, and whether a migration or batch completed. -- The warehouse catalog and planner snapshot are authoritative for live facts such as relation - existence, relation type, columns and types, table type, and retention. Never infer a live fact - from fingerprints, events, bindings, or a plan reason when the warehouse can answer; built-up - state drifts through external drops and edits and then needs reconciliation. -- Consult stored state only after the live facts it describes are confirmed. For example, a model - whose relation is missing plans as a first run regardless of any recorded fingerprint. - -## Direct-Mode Warehouse State - -- NEVER implement a mutable lifecycle state machine in raw warehouse state tables in direct mode. Do not represent progress by repeatedly updating one row through statuses such as `PLANNED`, `RUNNING`, and `COMPLETE`. -- Model lifecycle state as immutable, append-only events or facts with deterministic event IDs and idempotent writes. Derive current status by projecting event history, following the existing microbatch requirement/completion pattern. Retention pruning is cleanup, not a lifecycle update. -- Design every warehouse-DML/state-publication failure window for reconciliation from durable events and physical warehouse evidence. If append-only state cannot represent a proposed direct-mode feature safely, stop and resolve the architecture explicitly rather than adding mutable transitions. -- Sequential microbatch execution (`batch_concurrency = 1`) is fully stateless: zero reads and zero writes of `_sqlbuild_microbatches`. The state table is strictly a concurrency-coordination feature. Reintroducing state reads or writes into the sequential path requires an explicit user-approved design decision, never incidental wiring. -- State-table schemas evolve additively only: add new columns and deprecate old columns in place; never repurpose a column or change a stored format in place. Readers must tolerate absent new columns. Do not build version-reset or migration machinery for state tables; a breaking state change requires an explicit user decision and a one-time operator-performed table drop. - -## Delivery Workflow - -- Consolidate related work targeting the same release into one delivery branch and one pull request. Use separate pull requests only for independently releasable changes, intentionally different delivery timing, concrete risk isolation, or explicit user instruction. -- Local commits are checkpoints and do not trigger CI. Complete related implementation and review before the first push. -- Run targeted regressions and fast local static checks before pushing. Do not delay a ready commit or push solely to run or wait for long full integration or end-to-end suites that CI already executes; run those locally only to reproduce or diagnose the change, or when the user explicitly requests them. CI remains the required broad-suite gate. -- Review the complete local diff against the target branch and resolve findings before pushing. Do not push partial or overlapping branches merely to start CI. -- Push once and open one ready pull request, then immediately enable squash auto-merge unless the - user explicitly asks to leave that pull request unmerged. The delivery agent owns enabling - auto-merge; do not assume a separate GitHub workflow or bot will enable it. Verify the pull - request reports an active auto-merge request, or confirm it completed as a squash merge - immediately after the enable command when all required checks had already passed. -- Do not interpret "do not manually merge" as permission to leave auto-merge disabled. It means - enable auto-merge and let GitHub merge after required checks pass. Do not invoke a manual merge - while the automation is healthy. Watch the pull request through merge in the foreground when no - other useful work remains, or keep a background watch running while continuing independent work. - Use a manual merge only with concrete evidence that auto-merge is broken or unavailable, and - document that evidence and the fallback reason first. -- PR titles must follow Conventional Commits. Descriptions must be no longer than 2,000 characters and contain non-empty `## Why`, `## Changes`, and `## Verification` sections in that order. -- Before creating or editing a PR, validate its metadata with `make check-pr-metadata PR_TITLE='type: summary' PR_BODY_FILE=/path/to/body.md`. -- Monitor CI after every push and follow it through completion. Address failures before considering delivery complete. -- When asked to babysit or watch a pull request, keep exactly one active background CI watcher for - the surviving pull request until its latest commit reaches a terminal state. Do not stop at the - initial push, PR creation, or an intermediate green run superseded by a later push. If pull - requests are consolidated, stop obsolete watchers and retain one watcher for the combined PR. -- Babysitting includes inspecting failures, applying and pushing required fixes, restarting the - watcher after each push, and reporting the final green state. It does not authorize merging when - the user has asked for the pull request to remain unmerged. -- Push follow-up commits only for CI failures or correctness findings that could not reasonably have been found before the first push. -- For deployable changes, continue through auto-merge, release workflow completion, package publication, and published-version verification. Do not stop at PR creation unless the user explicitly asks. - -## Review Discipline - -- Treat review findings as hypotheses to validate against the supported product contract, ownership boundary, and realistic execution paths before changing code. -- Prioritize concrete correctness, authorization, data-loss, and mutation risks within systems the project manages. Distinguish those from unsupported external misuse or purely theoretical states. -- Keep fixes within the requested scope. Do not expand supported behavior, permissions, operational cost, or system ownership without a short product decision from the user. -- Do not add safeguards solely for impossible or unsupported states. Record residual risks when a concern is real but outside the current contract. - -## Duplicated Code - -- Run `uv run fensu dupes --since origin/main` before review, and `uv run fensu dupes --path ''` before adding helpers or logic to an area that may already implement them. The report is advisory: unlike `fensu check`, it never gates and never needs to reach zero. -- Treat a genuine duplicate as evidence of possible wider drift, not only a cleanup task: a missing shared owner, parallel implementations of one concept, logic on the wrong side of a boundary, or copies that have already diverged (use `--diff`; a fix present in only one copy may be a bug in the others). Diagnose and record that wider smell before consolidating, because removing the copies erases the only deterministic signal of it. -- Consolidate genuine duplication the change introduces or touches. Report unrelated findings as follow-ups instead of refactoring them opportunistically. -- Members marked `[forced]` are adapter contract overrides required by `test_strict_adapter.py`. Record other intentional mirrors in `[dupes]` in `fensu.toml` with a reason. -- Public adapter contract methods are duplicated per adapter on purpose: `test_strict_adapter.py` requires every first-class adapter to define them, and `[dupes]` exempts them. The private helpers behind them must not be copied. Keep them in shared builders, such as the snapshot SQL builders in `src/sqlbuild/adapter/contract/classes/`, and let each contract method delegate. Express real dialect differences as explicit parameters, such as a `SnapshotSqlDialect` field, not as a forked copy of the helper. - -## Rust and Native Builds - -- Python imports the native extension built from `crates/sqlbuild-python`. After changing any - Rust, rebuild it incrementally with `make fast-native` (release optimisation, incremental) - before running Python tests. Use one profile consistently within a worktree. -- Always run Python commands with `uv run --no-sync`. A syncing `uv run` or `uv sync` can replace - the locally built extension with a packaged one; if that happens, rebuild with maturin rather - than debugging stale native behaviour. -- Keep one persistent `CARGO_TARGET_DIR` per worktree, outside the repository, and reuse it for - the whole task. Never run `cargo clean`, delete a target directory, or switch target - directories mid-task. Do not override `RUSTC_WRAPPER`; the compiler cache is shared. -- An incremental rebuild after a focused change takes minutes. A build that is unexpectedly cold - or much slower than that is a signal to stop and investigate, not to wait or start over. -- Get fast feedback first: `cargo check -p ` while editing, `cargo test -p ` for - the crate you touched, then one maturin build and the focused Python tests. CI lints with the - latest stable toolchain, so keep the local `stable` toolchain current (`rustup update stable`); - `make check-ci` includes `make rust-lint` (workspace clippy with `-D warnings`) before a push. -- Fix compiler errors by reading them and making the smallest correct change. Never delete and - rewrite a working module, or restart a port from scratch, because a build failed. -- Do not run two native builds against the same target directory at once, and do not run - benchmarks while a build or test suite is running. -- Every new or changed binding updates `src/sqlbuild/_native.pyi` in the same change, and Rust - follows the `fensu.toml` layout rules (`uv run --no-sync fensu check`). - -## Implementation Subagents - -- Implementation subagents work only with explicit user approval, each on a bounded assignment in - its own worktree and branch. The primary agent owns the plan, integration, verification gates, - pushes and pull requests. -- A port's specification is the Python it replaces. Behaviour must match exactly, including error - types, messages, ordering and Python string semantics; check the native result against the - Python on realistic inputs before deleting the Python. -- Stop and hand back to the primary agent with a short report (what you tried, what blocks you, - the options you see) instead of continuing alone when: - - the next step will take an unusually long time, such as a cold rebuild, a broad or full test - suite, or any single command expected to run longer than about ten minutes; - - you are about to restart, rewrite from scratch, or abandon an approach you already built; - - the same build or test has failed twice with the same approach; - - the change needs files, crates, dependencies, `stays` entries or behaviour outside the - assignment; - - the Python specification is ambiguous, or native and Python results differ and the cause is - not obvious. -- Never run `cargo clean`, delete target directories, reset, rebase or force-push shared branches, - commit outside the assigned branch, run full suites, or edit `AGENTS.md` or - `python-boundary.toml` `stays` entries. -- Finish with local commits on the assigned branch and a report listing the files changed, the - Python deleted, the checks run with their results, and any residual risk. - -## Rust-Hosted Migration - -- Rust is moving to host compile, planning and non-warehouse CLI work. Python stays only for the modules listed in `python-boundary.toml`: user Python code, adapters, the executor and public Python APIs. -- A migration change deletes the Python it replaces in the same pull request and lowers `remaining_lines`. Never keep a parallel Python implementation, and never fall back to Python after a native failure; surface the failure as an error. -- Adding a `stays` or `handled_native_errors` entry, or raising `remaining_lines`, requires a stated reason in the pull request and explicit user approval. Difficulty or a small measured speed gain is not a reason. -- Moving code into a path listed in `stays`, or behind a reviewed handler, is not migration progress; only code that now runs in Rust lowers `remaining_lines`. - -## Bounded Review Process - -Orchestrating agents must run reviews as a single bounded cycle, not an open-ended loop: - -1. The orchestrator first performs its own targeted diff scan of dangerous seams (boundaries, gates, exit codes, state writes, deleted-symbol references) — not a line-by-line style review. -2. Use exactly one independent read-only review subagent on the complete combined diff, scoped to concrete correctness, data-loss, authorization, and behavioral-regression risks, with file:line evidence required for every finding. The reviewer must be explicitly told not to propose architecture expansions or run broad test suites. -3. Findings are hypotheses, never mandates. A validated in-scope correctness bug is fixed; any finding whose fix would expand behavior, state, permissions, or ownership is escalated to the user as a product decision — accept the reviewer's finding, never its solution, without validation. This rule exists because converting a correct review finding directly into an unapproved stateful implementation caused a production defect. -4. One follow-up pass verifies only the fixed findings and their affected boundaries — never a fresh full re-review, never broad suites. -5. Do not run overlapping review agents, issue "look for more things to harden" prompts, or enter review→fix→review loops. Valid out-of-scope findings become tickets, not opportunistic fixes. -6. After the single review cycle, the mechanical gates (`make test` where warranted, `make check-ci`, repository CI) are the final arbiter.