Docs: contributing, code of conduct, security policy - #5
Conversation
A first-time contributor had no way to learn the rules except by reading the Makefile and guessing. These three documents close that, and GitHub links them from the pull request and issue screens on its own. CONTRIBUTING points rather than repeats. The gate is `make all`, the extension path is already written in the README, the review questions are already in the pull request template, and DECISIONS.md already says why the expensive choices went the way they did — restating any of that would create a second copy to keep true. No SLA is promised anywhere. One maintainer cannot keep a response-time promise, and CODE_OF_CONDUCT says plainly that a report about the sole maintainer has to go to GitHub instead of to him.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d63da1d422
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s run SECURITY.md claimed embeddings arrive unsanitised and that validation was the caller's business. It is not: Index.Add rejects non-finite components and disagreeing vector widths, the vector scorer rejects a non-finite query norm, and the decoder checks again on the way back in. As written, a report that got past one of those guarantees could have been waved off as out of scope. Size is the thing genuinely unchecked, and that is a documented limitation. CONTRIBUTING said `make arch` decides the three assertions, which read as though they run nowhere else. They are ordinary tests in pkg/engine, so `make all` and CI already run them; `make arch` only reruns the same set by name. A reviewer misread it, which is enough evidence that the sentence was ambiguous.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6ac6e1f9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CONTRIBUTING listed three assertions, said tests decide all of them, and said CI reports a break. Two of the three are true: fusion's invariance to scorer count and fusion's ignorance of scorer packages hold for every scorer, present and future. The 100-line budget does not. TestFourthScorerIsUnderOneHundred Lines reads pkg/scorer/recency by a hardcoded path, so it measures one package that already exists rather than inspecting whatever a contributor adds; a fifth scorer at any size passes it. Advertising it as a gate is worse than not mentioning it. A contributor sizes their work against a limit nobody enforces, and CI never says the word. Making the test generic would be the other repair, and it would be wrong here: milestone 3 is segment merge and ANN, and an ANN scorer does not fit in 100 lines. That gate would fail the project's own roadmap first. So the number stays what FINDINGS already treats it as — a recorded result about the fourth scorer — and the text now says which of the three CI will actually catch.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…y deletes A review of the governance docs against the code found four places where the prose was not true of the repository it describes. The 100-line figure is not enforced for a new scorer, which the last commit said, but it is enforced hard for the one it measures: the budget is a t.Fatalf at 100, countGoLines counts raw newlines including comments and blank lines, and recency.go sits at 99. One comment added there turns make all red, and nothing warned about that. "Two assertions are decided by tests" was too generous to one of them. The import-graph check generalises on its own; TestAddingAFourthScorer and TestAnyNumberOfScorersFuses name the four scorers in the tree, so a fifth runs in no test until it is added to those two slices by hand. Saying otherwise promises coverage a contributor will not get. SECURITY listed no privileged operation. Commit recursively removes every seg-* entry and MANIFEST.tmp from the directory it is handed, and refuseForeignEntries exists precisely so a first commit into a home directory does not delete what weft never wrote. That is the most destructive thing here and it was missing from the table. The memory carve-out was also too wide: it covers a corpus you built, not a file you did not, because Open reads each section whole before it can bound anything. Private vulnerability reporting is off on this repository, so the Security tab has no Report a vulnerability button. The policy now says email, and says why, rather than linking a page that 404s. Also: make runs test -race, which README omitted, and the two documents pointed at each other for the same explanation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77003cf588
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Surface | Why | | ||
| |---|---| | ||
| | The persisted segment format | `Open` parses a block-structured file with varint headers ([docs/FORMAT.md](docs/FORMAT.md)). A malformed or hostile file reaching the decoder is the most likely source of a panic, an out-of-range read, or an allocation driven by a length the file chose. | | ||
| | What `Commit` deletes | `Commit` creates the directory you name and recursively removes every `seg-*` entry and `MANIFEST.tmp` it finds there. `refuseForeignEntries` is what stops a first commit into, say, a home directory from deleting data weft never wrote. A path that gets past that guard, or a link that escapes the `os.Root` both readers and the writer work through, is in scope. | |
There was a problem hiding this comment.
When an existing index directory contains a regular file such as seg-junk or a nonempty directory named MANIFEST.tmp, a successful Commit leaves it in place: prune applies RemoveAll only to seg-* directories and uses non-recursive Remove for the temporary manifest. The first-commit guard also refuses foreign entries rather than deleting them. Saying that every matching entry is recursively removed therefore gives callers assessing the destructive scope of Commit the wrong contract; document the next-generation entry, stale segment directories, and removable manifest temp separately.
Useful? React with 👍 / 👎.
1 participant
Summary
Until now a first-time contributor could only learn this project's rules by reading the
Makefileand guessing. This PR adds the three documents that close that gap; GitHub links them from the pull request and issue screens on its own.The organising rule: CONTRIBUTING points, it does not repeat.
docs/DECISIONS.mdalready declares "Anything readable from the code is not recorded here," and the same applies to process. The gate ismake all, the extension path is already in the README, and the review questions are already in the pull request template — a second copy of any of those is a second copy to keep true.Changes
CONTRIBUTING.md— the gate (make all), the three architecture assertions and the fact thatmake archdecides them rather than a reviewer, links to the README extension path and the PR template, no CLA or DCO, and the v0.x stability positionCODE_OF_CONDUCT.md— Contributor Covenant 2.1, unmodified apart from the contact address and one added noteSECURITY.md— a single reporting route, supported versions (mainonly; there is nothing released to backport to), and where the risk actually is: the segment-format decoder and caller-supplied inputREADME.md— a Contributing section above LicenseValidation
#adding-a-scorerand#limitationsanchors match real headings (README.md:62,127)make allpassescommunity/profile: 57% → 86%. Not 100% —issue_templateis the remaining item and belongs to milestone 3.SECURITY.mdis not part of GitHub's health percentage at all, though the Community Standards page lists itReview Focus
Two judgement calls worth a second opinion:
skyoo2003@gmail.com, already public as the commit author. Private vulnerability reporting is currently disabled on this repository; enabling it would allow a reporting route with no address in the file at allCODE_OF_CONDUCT.mdadds a note the standard text does not have: with one maintainer, a report about that maintainer would go to the person it concerns, so it points at GitHub's abuse form instead. Stating the gap seemed better than a procedure nobody could trust — but it is a deviation from the covenant's normal wordingRisks / Notes
pkg/and the build are untouched