Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Comprehension/fix plagiarism nil error #7976

Merged
merged 2 commits into from Jul 28, 2021
Merged

Comprehension/fix plagiarism nil error #7976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
083806b
Refactor plagiarism highlighting to handle edge cases
anathomical Jul 23, 2021
a49ef32
Add tests for Plagiarism edge case
anathomical Jul 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 10 additions & 1 deletion services/QuillLMS/engines/comprehension/lib/comprehension/comprehension/plagiarism_check.rb
View file
Open in desktop
Expand Up @@ -120,8 +120,17 @@ def passage_word_arrays
end

private def get_highlight(text, cleaned_text)
start_index = cleaned_text.index(matched_slice)
extra_space_indexes = []
cleaned_text.each_char.with_index { |c, i| extra_space_indexes.push(i) if c == ' ' && cleaned_text[i+1] == ' ' }

space_normalized_text = cleaned_text.split.join(' ')

start_index = space_normalized_text.index(matched_slice)
extra_space_indexes.each { |i| start_index += 1 if i <= start_index }

end_index = start_index + matched_slice.size - 1
extra_space_indexes.each { |i| end_index += 1 if i > start_index && i <= end_index }

char_positions = text.enum_for(:scan, /[A-Za-z0-9\s]/).map { |c| Regexp.last_match.begin(0) }
text[char_positions[start_index]..char_positions[end_index]]
end
Expand Down
48 changes: 48 additions & 0 deletions services/QuillLMS/engines/comprehension/spec/lib/plagiarism_check_spec.rb
View file
Open in desktop
Expand Up @@ -52,6 +52,54 @@ module Comprehension
expect(feedback[:rule_uid]).to(be_truthy)
expect(feedback[:concept_uid]).to(be_truthy)
end

context 'space normalization handling' do
let(:feedback) { "this is some standard plagiarism feedback" }

it 'should successfully highlight even when the user entry has multiple consecutive spaces but the passage does not' do
entry = "This phrase plagiarises from the passage even though it has a ton of spaces in it."
passage = "From the passage even though it has a ton of spaces."

plagiarism_check = Comprehension::PlagiarismCheck.new(entry, passage, feedback, rule)
feedback = plagiarism_check.feedback_object

expect(feedback[:highlight][0][:text]).to eq("from the passage even though it has a ton of spaces")
expect(feedback[:highlight][1][:text]).to eq("From the passage even though it has a ton of spaces")
end

it 'should successfully highlight even when the passage has multiple consecutive spaces but the user entry does not' do
entry = "This phrase plagiarises from the passage even though it has a ton of spaces in it."
passage = "From the passage even though it has a ton of spaces."
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it's better to remove extra whitespace in passages at publish time, so that downstream algorithms like this can assume clean passages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but I also don't think there's any way to guarantee this without taking away some control from Curriculum. It's possible, though I can't imagine off-hand why, that they might intentionally want something like this in some edge case. It doesn't seem reasonable to try to take that away from them without good reason.


plagiarism_check = Comprehension::PlagiarismCheck.new(entry, passage, feedback, rule)
feedback = plagiarism_check.feedback_object

expect(feedback[:highlight][0][:text]).to eq("from the passage even though it has a ton of spaces")
expect(feedback[:highlight][1][:text]).to eq("From the passage even though it has a ton of spaces")
end

it 'should successfully highlight even when the user entry has isolated punctuation but the passage does not' do
entry = "This phrase plagiarises from - the -- passage even - - though it , has a ton of spaces in it."
passage = "From the passage even though it has a ton of spaces."

plagiarism_check = Comprehension::PlagiarismCheck.new(entry, passage, feedback, rule)
feedback = plagiarism_check.feedback_object

expect(feedback[:highlight][0][:text]).to eq("from - the -- passage even - - though it , has a ton of spaces")
expect(feedback[:highlight][1][:text]).to eq("From the passage even though it has a ton of spaces")
end

it 'should successfully highlight even when the passage has isolated punctuation but the user entry does not' do
entry = "This phrase plagiarises from the passage even though it has a ton of spaces in it."
passage = "From the - passage even - - - though it has -- a ton of spaces."

plagiarism_check = Comprehension::PlagiarismCheck.new(entry, passage, feedback, rule)
feedback = plagiarism_check.feedback_object

expect(feedback[:highlight][0][:text]).to eq("from the passage even though it has a ton of spaces")
expect(feedback[:highlight][1][:text]).to eq("From the - passage even - - - though it has -- a ton of spaces")
end
end
end
end
end