Skip to content

Added streaming reader for items and quantities - #17

Draft
HassanAkbar wants to merge 6 commits into
mainfrom
feat/streaming-parser
Draft

Added streaming reader for items and quantities#17
HassanAkbar wants to merge 6 commits into
mainfrom
feat/streaming-parser

Conversation

@HassanAkbar

Copy link
Copy Markdown
Member

Adds Dcc::Streaming.each_item and each_quantity for reading large DCC documents
without loading the whole tree.

SAX-driven. Buffers one matched subtree at a time, reconstructs it as a fragment, and
parses that into a model object. Peak memory tracks the largest single subtree.

Known and accepted:

  • The subtree is serialized and reparsed rather than handed over directly.
    Nokogiri::XML::Reader#outer_xml would avoid it, but CONTRIBUTING.adoc bans
    Nokogiri outside validate/xsd.rb. Two parses per subtree is the cost of that ban.
  • Escaping calls Moxml::C14n.escape_text. C14N mandates exactly the character
    references we need, so the table is not restated here.
  • Fidelity is at the infoset level, not byte level. Entities come back expanded, CDATA
    as text, PI spacing normalized. Eight limits are listed in the class doc.
  • each_quantity yields the outermost match only. Nesting is not yielded separately.
  • The memory gate proves retained Ruby heap does not grow with document length. It does
    not prove a native libxml2 peak or any absolute MB ceiling.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Dcc::Streaming API that uses SAX to iterate large DCC XML documents and yield parsed dcc:item / (outermost) dcc:quantity subtrees without building the full object tree in memory.

Changes:

  • Introduce Dcc::Streaming.each_item / each_quantity and a SAX-backed streaming reader that reconstructs and reparses matched subtrees.
  • Add a comprehensive spec suite covering version detection, namespace handling, opaque XML payload fidelity, error handling, and early-stopping behavior.
  • Add a child-process memory probe to assert heap retention does not grow with document length.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/support/streaming_memory_probe.rb Standalone probe program to measure retained heap/object counts while streaming items.
spec/dcc/streaming_spec.rb New tests for streaming behavior, fidelity, error handling, and a memory gate using the probe.
lib/dcc/streaming/reader.rb SAX handler that buffers one matched subtree, serializes to fragment XML, reparses into the appropriate model, and yields it.
lib/dcc/streaming.rb Public Dcc::Streaming entry points (each_item, each_quantity) and shared stream setup/validation.
lib/dcc.rb Adds Dcc::Streaming autoload and refactors version detection by introducing major_version_from.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread spec/dcc/streaming_spec.rb

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (2)

lib/dcc/streaming/reader.rb:57

  • @scopes.push(@scopes.last.merge(namespaces)) allocates a new Hash for every element, even when namespaces is empty. In large documents this creates avoidable per-element allocations and GC churn, which works against the goal of low-memory streaming.
      def on_start_element(name, attributes = {}, namespaces = {})
        @scopes.push(@scopes.last.merge(namespaces))
        @major ||= ::Dcc.major_version_from(attributes["schemaVersion"])

lib/dcc.rb:115

  • The YARD example for schema_version includes backticks and quotes ("2.3.0"), which reads like the string literal includes quote characters. This is likely meant to be just "2.3.0".
    # @param schema_version [String, nil] e.g. `"2.3.0"`.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (1)

lib/dcc/streaming/reader.rb:251

  • decode_ampersands uses String#gsub, which always allocates a new String even when the attribute contains no "&" sequence. Since this runs for every attribute in every buffered subtree, it adds avoidable allocations to the streaming hot path. Consider fast-pathing the common case by checking include? first and returning the original string unchanged when no substitution is needed.
        def decode_ampersands(value)
          value.gsub("&", "&")
        end

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants