flyingImer commented on code in PR #4454:
URL: https://github.com/apache/polaris/pull/4454#discussion_r3270083359
##########
AGENTS.md:
##########
@@ -229,6 +253,11 @@ Write the minimum code that solves the problem.
Start with the simplest correct implementation. Optimize only when the task
requires it, and only after the simple version works and has tests.
+When reading from a database, object store, filesystem, archive, or other
external
+data source, do not materialize unbounded inputs just to perform existence
checks.
+Prefer streaming, short-circuiting, indexed lookups, or existing APIs. If
collecting
+is necessary, the bound must be obvious from the code or explained in the PR.
Review Comment:
IMO it's a structural mismatch. The principle ("watch unbounded
materialization") is correctness/resource bounds, not simplicity. 2 cents: pull
this out of the PR and refile once there are enough resource patterns to form a
section, or fold into a future Performance/Design Patterns expansion.
##########
AGENTS.md:
##########
@@ -66,6 +66,27 @@ assumptions and running with them without checking.
---
+## Do Not Treat Issues As Specifications [DISCIPLINE]
Review Comment:
+1 to Russell. IMO the five-step workflow restates "verify before claiming"
with issue-specific detail. A subsection under "Understand Before Coding" would
carry the same weight without two top-level sections framed around the same
thing.
##########
AGENTS.md:
##########
@@ -66,6 +66,27 @@ assumptions and running with them without checking.
---
+## Do Not Treat Issues As Specifications [DISCIPLINE]
+
+A GitHub issue is a starting point for investigation, not an implementation
plan.
+Before making an issue-driven change, verify that the requested behavior is
still
+missing and that the proposed fix actually addresses the stated problem.
+
+Required workflow:
+
+1. Trace the current code path related to the issue.
+2. Search for existing validation, tests, build tasks, or runtime behavior
that may
+ already cover the requested case.
+3. Compare the issue's requested failure modes with the change you intend to
make.
+ If the change only addresses part of the issue, use `Related to #NNN`, not
+ `Fixes #NNN`.
Review Comment:
IMO this is a PR-author convention rather than an agent-coding discipline.
AGENTS.md isn't quite the right surface for it. 2 cents: move to
CONTRIBUTING.md or the PR template, where the rest of the project's commit/PR
conventions live.
##########
AGENTS.md:
##########
@@ -195,6 +216,9 @@ Use the PR template (`.github/pull_request_template.md`).
At minimum:
6. If your change affects user-facing behavior or configuration, check whether
`site/content/in-dev/unreleased/` needs updates.
+For issue-driven work, the PR description must explain the current behavior
that
+was verified in the codebase and why existing checks do not already cover it.
Review Comment:
I'm skeptical about adding issue-specific PR-description boilerplate here.
The general "explain what you verified" expectation is already in this section.
Issue-specific boilerplate gets copy-pasted without actually doing the
verification, and reviewers can read its presence as proxy for verification. 2
cents: drop and rely on the workflow rule above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
