Add AI-disclosure and quality requirements to the contribution guidelines#2143
Merged
Add AI-disclosure and quality requirements to the contribution guidelines#2143
Conversation
…ines. Co-authored-by: GPT 5.5 <[email protected]>
Split the quality-expectations section into two paragraphs (the warning about low-quality contributions being declined was visually merged with the preceding paragraph). Replace "and the pull request closed without warning" with a note that maintainers may not always be able to provide detailed feedback, which conveys the same practical reality. Co-authored-by: Claude Opus 4.6 <[email protected]>
EliahKagan
reviewed
May 6, 2026
Member
There was a problem hiding this comment.
I've pushed a commit (92ff6df). How do you feel about this wording? I worry this may soften the point you are making too much, which I would not want to happen. But this also explains the reasoning why feedback may be absent -- and I am hoping it may also avoid discouraging people from contributing, while at the same time hopefully making the situation at least as clear, and setting the exact same expectations, as in the original wording.
EliahKagan
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s contribution guidelines to set explicit quality expectations for contributions and to require disclosure/identification for AI-assisted work when it meaningfully affects a PR, commits, or GitHub interactions.
Changes:
- Add a “Quality expectations” section defining baseline standards (readability, maintainability, tests where practical, documentation, consistency).
- Add an “AI-assisted contributions” section describing disclosure requirements and agent identification expectations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
Thanks a lot! Let's use this. |
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
May 7, 2026
Remove the Cygwin xfail decorations from test_submodules in
test_docs.py and test_repo.py, and from test_root_module in
test_submodule.py, so the tests surface the underlying failure
directly. Add 256 reproduce-safe-dir matrix jobs to cygwin-test.yml,
each running these three tests under the current safe.directory
configuration. All or most of these reproduce-safe-dir jobs must be
removed before this work is integrated.
The existing test job's env, defaults, and setup steps gain YAML
anchors so the new job can reference them without duplication.
Root cause hypothesis
---------------------
The CI's safe.directory list (set in the "Special configuration for
Cygwin git" step) covers the main repo and its .git directory but
not the gitdb submodule's working tree at git/ext/gitdb. Git matches
safe.directory exactly, not by prefix, so when GitPython spawns
"git cat-file --batch-check" in the gitdb submodule (via
Repo(submodule_path).odb.info(sha)), git rejects the repository for
dubious ownership and exits.
Three observed failure modes, all from the same root cause
----------------------------------------------------------
The git subprocess exits soon after starting. What Python sees
depends on a race between Python writing to the subprocess's stdin
and the subprocess exiting and closing its stdout pipe.
1. ValueError ("SHA is empty, possible dubious ownership..."):
Python's write/flush completes before git has finished exiting.
The buffered write succeeds, then stdout.readline() returns b""
(EOF). _parse_object_header(b"") in git/cmd.py raises this
ValueError. The error message names the rejected directory and
even suggests the safe.directory fix. This propagates uncaught
from Object.new_from_sha through name_to_object (line 229 of
git/repo/fun.py is outside the try/except loop) through
repo.commit("HEAD") to iter_items, where the
"except (IOError, BadName)" clause does not catch ValueError.
2. IndexError ("list index out of range"):
Git exits before Python's write/flush runs. cmd.stdin.write or
cmd.stdin.flush raises BrokenPipeError, a subclass of OSError
(IOError). This time iter_items's "except (IOError, BadName)"
catches it, returning an empty iterator. children() therefore
returns an empty list, and "[0]" in test_submodules raises
IndexError.
3. AssertionError ("1 not greater than or equal to 2"):
Same BrokenPipeError-caught-as-IOError mechanism as case 2, but
manifesting in test_repo.py::test_submodules, which does
"assertGreaterEqual(len(list(self.rorepo.iter_submodules())), 2)".
The recursive traversal finds gitdb (via the main repo, which is
in safe.directory) but cannot enumerate gitdb's children, so
only one submodule is yielded.
Evidence
--------
Recent main-branch CI runs show all three xfailed tests consistently
matching their raises=ValueError xfail, e.g. job 74546730688 in run
25415738988. Verified across the five most recent main runs.
PR gitpython-developers#2143 attempt 1 (job 74630986491, run 25440735020 attempt 1) had
test_docs.py FAILED with IndexError while test_repo.py XFAILed with
ValueError in the same job. PR gitpython-developers#2143 attempt 2 (job 74633063805) had
both XFAIL with ValueError. Same commit, same workflow, same runner
image -- a flaky race.
The 3-job trial of these reproduce-safe-dir jobs (run 25454533092,
commit baf3526) produced 8 ValueError and 1 IndexError out of 9
test runs.
The 30-job run (run 25454836713) produced 90 ValueError and 0
IndexError in the reproduce jobs, but the same run's test (fast)
job exhibited the AssertionError variant in
test_repo.py::test_submodules. That AssertionError is the third
manifestation of the BrokenPipeError race.
Plan
----
1. (this commit) Reproduce the failure under the current
safe.directory configuration, with xfails removed so failures
surface directly. The 256-job matrix is intended to characterize
the rate of each variant.
2. Apply the fix: extend the safe.directory configuration to cover
the gitdb submodule's working tree (and smmap's, recursively).
Verify that the tests now pass.
3. Remove the reproduce-safe-dir jobs and the YAML anchors that
only the reproduce job needed. The existing test job retains the
permanent fix.
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
May 7, 2026
Remove the Cygwin xfail decorations from test_submodules in
test_docs.py and test_repo.py, and from test_root_module in
test_submodule.py, so the tests surface the underlying failure
directly. Add 256 reproduce-safe-dir matrix jobs to cygwin-test.yml,
each running these three tests under the current safe.directory
configuration. All or most of these reproduce-safe-dir jobs must be
removed before this work is integrated.
The existing test job's env, defaults, and setup steps gain YAML
anchors so the new job can reference them without duplication.
Root cause hypothesis
---------------------
The CI's safe.directory list (set in the "Special configuration for
Cygwin git" step) covers the main repo and its .git directory but
not the gitdb submodule's working tree at git/ext/gitdb. Git matches
safe.directory exactly, not by prefix, so when GitPython spawns
"git cat-file --batch-check" in the gitdb submodule (via
Repo(submodule_path).odb.info(sha)), git rejects the repository for
dubious ownership and exits.
Three observed failure modes, all from the same root cause
----------------------------------------------------------
The git subprocess exits soon after starting. What Python sees
depends on a race between Python writing to the subprocess's stdin
and the subprocess exiting and closing its stdout pipe.
1. ValueError ("SHA is empty, possible dubious ownership..."):
Python's write/flush completes before git has finished exiting.
The buffered write succeeds, then stdout.readline() returns b""
(EOF). _parse_object_header(b"") in git/cmd.py raises this
ValueError. The error message names the rejected directory and
even suggests the safe.directory fix. This propagates uncaught
from Object.new_from_sha through name_to_object (line 229 of
git/repo/fun.py is outside the try/except loop) through
repo.commit("HEAD") to iter_items, where the
"except (IOError, BadName)" clause does not catch ValueError.
2. IndexError ("list index out of range"):
Git exits before Python's write/flush runs. cmd.stdin.write or
cmd.stdin.flush raises BrokenPipeError, a subclass of OSError
(IOError). This time iter_items's "except (IOError, BadName)"
catches it, returning an empty iterator. children() therefore
returns an empty list, and "[0]" in test_submodules raises
IndexError.
3. AssertionError ("1 not greater than or equal to 2"):
Same BrokenPipeError-caught-as-IOError mechanism as case 2, but
manifesting in test_repo.py::test_submodules, which does
"assertGreaterEqual(len(list(self.rorepo.iter_submodules())), 2)".
The recursive traversal finds gitdb (via the main repo, which is
in safe.directory) but cannot enumerate gitdb's children, so
only one submodule is yielded.
Evidence
--------
Recent main-branch CI runs show all three xfailed tests consistently
matching their raises=ValueError xfail, e.g. job 74546730688 in run
25415738988. Verified across the five most recent main runs.
PR gitpython-developers#2143 attempt 1 (job 74630986491, run 25440735020 attempt 1) had
test_docs.py FAILED with IndexError while test_repo.py XFAILed with
ValueError in the same job. PR gitpython-developers#2143 attempt 2 (job 74633063805) had
both XFAIL with ValueError. Same commit, same workflow, same runner
image -- a flaky race.
The 3-job trial of these reproduce-safe-dir jobs (run 25454533092,
commit baf3526) produced 8 ValueError and 1 IndexError out of 9
test runs.
The 30-job run (run 25454836713) produced 90 ValueError and 0
IndexError in the reproduce jobs, but the same run's test (fast)
job exhibited the AssertionError variant in
test_repo.py::test_submodules. That AssertionError is the third
manifestation of the BrokenPipeError race.
Plan
----
1. (this commit) Reproduce the failure under the current
safe.directory configuration, with xfails removed so failures
surface directly. The 256-job matrix is intended to characterize
the rate of each variant.
2. Apply the fix: extend the safe.directory configuration to cover
the gitdb submodule's working tree (and smmap's, recursively).
Verify that the tests now pass.
3. Remove the reproduce-safe-dir jobs and the YAML anchors that
only the reproduce job needed. The existing test job retains the
permanent fix.
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tasks