Skip to content

docs: correct stale &+N relative-offset parsing TODO claims (#287)#306

Merged
unclesp1d3r merged 1 commit into
mainfrom
docs/issue-287-relative-offset-parsing
May 29, 2026
Merged

docs: correct stale &+N relative-offset parsing TODO claims (#287)#306
unclesp1d3r merged 1 commit into
mainfrom
docs/issue-287-relative-offset-parsing

Conversation

@unclesp1d3r
Copy link
Copy Markdown
Member

Summary

  • Updates AGENTS.md "Offset Specifications" to reflect that magic-file &N / &+N / &-N parsing is implemented (decimal and 0x hex forms; bare & rejected) and points at parse_offset_relative in src/parser/grammar/tests/mod.rs for parse-side coverage.
  • Updates GOTCHAS.md S3.11 (parse_text_magic_file is Fail-Fast) to use an unquoted $VAR string value on a non-string-family type as the canonical fail-fast example, since &+N is no longer an unsupported-syntax case per S3.6.
  • Pure documentation. No code changes.

Test Plan

  • rg -n '&\+N' AGENTS.md GOTCHAS.md returns only the corrected line on AGENTS.md:245 and the still-accurate evaluator-semantics line on AGENTS.md:216.
  • Existing parse-side coverage parse_offset_relative in src/parser/grammar/tests/mod.rs already verifies the claim — no new tests needed.

Closes #287

AGENTS.md and GOTCHAS.md both stated that magic-file &+N/&-N
relative-offset parsing was "still TODO". Parsing has been
implemented for some time and is exercised by parse_offset_relative
tests in src/parser/grammar/tests/mod.rs (covers &0, &4, &+4, &-4,
&0x10, &-0x10; bare & rejected).

- AGENTS.md Offset Specifications: replace the TODO clause with a
  statement that &N/&+N/&-N parsing is implemented, including hex
  forms, and point to the parse-side test module.
- GOTCHAS.md S3.11: replace the canonical fail-fast example (which
  used &+N as the unsupported syntax) with an unquoted $VAR string
  value on a non-string-family type, which remains a real parse
  failure per S3.6.

Pure documentation. No code changes.

Closes #287

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Copilot AI review requested due to automatic review settings May 29, 2026 22:26
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. documentation Improvements or additions to documentation labels May 29, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 29, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Require conventional commit format per https://www.conventionalcommits.org/en/v1.0.0/. Skipped for bots.

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?!?:

🟢 Full CI must pass

Wonderful, this rule succeeded.

All CI checks must pass. Release-plz PRs are exempt because they only bump versions and changelogs (code was already tested on main), and GITHUB_TOKEN-triggered force-pushes suppress CI.

  • check-success = coverage
  • check-success = quality
  • check-success = test
  • check-success = test-cross-platform (macos-latest, macOS)
  • check-success = test-cross-platform (ubuntu-22.04, Linux)
  • check-success = test-cross-platform (ubuntu-latest, Linux)
  • check-success = test-cross-platform (windows-latest, Windows)

🟢 Do not merge outdated PRs

Wonderful, this rule succeeded.

Make sure PRs are within 10 commits of the base branch before merging

  • #commits-behind <= 10

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 508b9a4c-e848-4c43-ad0f-33cdbeee5127

📥 Commits

Reviewing files that changed from the base of the PR and between d1f6167 and 88189e1.

📒 Files selected for processing (2)
  • AGENTS.md
  • GOTCHAS.md

Summary by CodeRabbit

Documentation

  • Clarified that relative offset syntax for magic files (&N, &+N, &-N in decimal and hexadecimal formats) is fully supported
  • Updated fail-fast parsing documentation examples

Walkthrough

This PR updates documentation in AGENTS.md and GOTCHAS.md to reflect that relative offset parsing for &N, &+N, and &-N syntax is fully implemented and tested. Two stale TODO references are removed and replaced with accurate descriptions of the current implementation state and a still-valid fail-fast example.

Changes

Documentation: Relative offset parsing completion

Layer / File(s) Summary
Relative offset parsing documentation updates
AGENTS.md, GOTCHAS.md
AGENTS.md line 245 asserts completion of &N/&+N/&-N parsing (decimal/hex) with test coverage reference; GOTCHAS.md line 153 removes the outdated unsupported &+N syntax example and replaces it with unquoted $VAR substitution on non-string types.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested labels

parser

Poem

Offsets once marked TODO,
Now fully parsed and tested bright;
Docs reflect what's done,
Stale claims take their flight. 📚✨

🚥 Pre-merge checks | ✅ 10
✅ Passed checks (10 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits specification with 'docs' type and 'offsets' scope, clearly describes the change (correcting stale documentation claims about &+N relative-offset parsing).
Description check ✅ Passed Description is directly related to the changeset, providing clear context about documentation updates to AGENTS.md and GOTCHAS.md regarding relative-offset parsing implementation and test coverage.
Linked Issues check ✅ Passed Changes fully satisfy #287 requirements: AGENTS.md updated to reflect implemented &+N/&-N parsing with test reference, GOTCHAS.md example replaced with unquoted $VAR syntax, documentation-only edits as specified.
Out of Scope Changes check ✅ Passed All changes are documentation-only edits to AGENTS.md and GOTCHAS.md directly addressing #287 objectives; no code modifications or unrelated changes present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Memory-Safety-Check ✅ Passed PR modifies only documentation (AGENTS.md, GOTCHAS.md). No new unsafe code introduced; existing unsafe block in src/io/mod.rs is pre-existing, properly allowed, and wraps vetted memmap2 dependency.
Libmagic-Compatibility-Check ✅ Passed Docs correctly reflect implemented &N/&+N/&-N parsing with hex support (verified by test_parse_offset_relative); GOTCHAS.md updated with valid unsupported example; maintains libmagic compatibility.
Performance-Regression-Check ✅ Passed This is a documentation-only PR with zero code changes. No parser, evaluator, I/O, or performance-sensitive code was modified. No allocations, unsafe code, or hot paths were touched.
Test-Coverage-Check ✅ Passed Documentation-only PR with no new functionality. Existing parse and integration tests adequately cover &+N/&-N offset parsing and evaluation with edge cases.
Error-Handling-Check ✅ Passed PR is documentation-only (AGENTS.md, GOTCHAS.md); no library code changes. Error-handling check is inapplicable to documentation-only PRs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

These MCP integrations need to be re-authenticated in the Integrations settings: Linear, Notion


Comment @coderabbitai help to get the list of available commands and usage tips.

@unclesp1d3r unclesp1d3r enabled auto-merge (squash) May 29, 2026 22:27
@coderabbitai coderabbitai Bot added the parser Magic file parsing components and grammar label May 29, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Documentation-only cleanup that removes stale TODO claims about magic-file &+N / &-N relative-offset parsing, which has been implemented and tested. Updates the canonical fail-fast example in GOTCHAS.md to a still-unsupported syntax case.

Changes:

  • AGENTS.md: corrects "Offset Specifications" to state &N/&+N/&-N parsing is implemented (decimal + hex; bare & rejected), pointing at parse_offset_relative for coverage.
  • GOTCHAS.md S3.11: replaces the stale &+N example with an unquoted $VAR value on a non-string-family type as the canonical fail-fast trigger.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
AGENTS.md Replaces TODO claim with accurate description of implemented relative-offset parsing.
GOTCHAS.md Updates fail-fast example to a still-valid unsupported-syntax case.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@unclesp1d3r unclesp1d3r merged commit f1aad8d into main May 29, 2026
29 checks passed
@unclesp1d3r unclesp1d3r deleted the docs/issue-287-relative-offset-parsing branch May 29, 2026 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation parser Magic file parsing components and grammar size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs: AGENTS.md/GOTCHAS.md claim &+N parsing is TODO but it's implemented

2 participants